unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>
To: Sergei Trofimovich <slyfox@gentoo.org>,
	sparclinux@vger.kernel.org, libc-alpha@sourceware.org
Cc: sparc@gentoo.org, toolchain@gentoo.org,
	"David S. Miller" <davem@davemloft.net>,
	"Michał Górny" <mgorny@gentoo.org>
Subject: Re: sparc vs sparc64: O_NDELAY and O_NONBLOCK mismatch in kernel and in glibc
Date: Mon, 22 Jun 2020 16:08:28 -0300	[thread overview]
Message-ID: <a213fcf7-73e3-7727-dea2-f73e1032a307@linaro.org> (raw)
In-Reply-To: <20200529104019.72983ef9@sf>



On 29/05/2020 06:40, Sergei Trofimovich via Libc-alpha wrote:
> On most targets glibc defines O_NDELAY as O_NONBLOCK.
> 
> glibc's manual/llio.texi manual says they are supposed to be equal:
> 
> """
> @deftypevr Macro int O_NDELAY
> @standards{BSD, fcntl.h}
> This is an obsolete name for @code{O_NONBLOCK}, provided for
> compatibility with BSD.  It is not defined by the POSIX.1 standard.
> @end deftypevr
> """
> 
> A bunch of packages rely on it and find out that this assumption
> breaks on sparc in unusual ways. Recently it popped up as:
>     https://github.com/eventlet/eventlet/pull/615
> Older workarounds:
>     https://github.com/libuv/libuv/issues/1830
> 
> What is more confusing for me:
> 
> linux kernel's uapi definition of O_NDELAY is ABI-dependent:
>   arch/sparc/include/uapi/asm/fcntl.h
> """
> #if defined(__sparc__) && defined(__arch64__)
> #define O_NDELAY        0x0004
> #else
> #define O_NDELAY        (0x0004 | O_NONBLOCK)
> #endif
> """
> 
> while glibc's is not:
>   sysdeps/unix/sysv/linux/sparc/bits/fcntl.h
> """
> #define O_NONBLOCK      0x4000
> #define O_NDELAY        (0x0004 | O_NONBLOCK)
> """

Doing some archeology it seems that sparc32 originally defined
O_NDELAY as 0x0004, but it has changed it to 0x0004 | O_NONBLOCK
on 2.1.29.

> 
> Spot-checking preprocessor's output that seems to corroborate:
> 
> """
> $ printf "#include <sys/fcntl.h>'\n int o_ndelay = O_NDELAY; int o_nonblock = O_NONBLOCK;" | sparc-unknown-linux-gnu-gcc -E -x c - | fgrep -A3 o_
> int o_ndelay =
>                (0x0004 | 0x4000)
>                        ; int o_nonblock =
>                                           0x4000
> 
> $ printf "#include <sys/fcntl.h>'\n int o_ndelay = O_NDELAY; int o_nonblock = O_NONBLOCK;" | sparc64-unknown-linux-gnu-gcc -E -x c - | fgrep -A3 o_
> 
> int o_ndelay =
>                (0x0004 | 0x4000)
>                        ; int o_nonblock =
>                                           0x4000
> """
> 
> I think this skew causes strange effects when you run sparc32
> binary on sparc64 kernel (compared to sparc32 binary on sparc32
> kernel) as kernel disagrees with userspace on O_NDELAY definition.
> 
> https://github.com/libuv/libuv/issues/1830 has more details.
> 
> I tried to trace the O_NDELAY definition and stopped at linux-2.1.29:
>   https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/diff/include/asm-sparc/fcntl.h?id=b7b4d2d2c1809575374269e14d86ee1953bd168c
> which brought O_NDELAY to O_NONBLOCK but did not make them
> match exactly.
> 
> Question time:
> 
> 1. Why is sparc32 special? Does it have something to do with
>    compatibility to other OSes of that time? (Solaris? BSD?)
> 
>    fs/fcntl.c has kernel handling:
>         /* required for strict SunOS emulation */
>         if (O_NONBLOCK != O_NDELAY)
>                if (arg & O_NDELAY)
>                    arg |= O_NONBLOCK;
>    but why does it leak to to userspace header definition?
> 
>    I think it should not.

It seems to provide some compatibility with SunOS since on Solaris11
O_NDELAY is 0x4 on both 32 and 64 bits. 

> 
> 2. Should sparc64-glibc change it's definition? Say, from
>     #define O_NDELAY        (0x0004 | O_NONBLOCK)
>    to
>     #define O_NDELAY        O_NONBLOCK
> 
>     I think it should.

This will make:

  fcntl(fd, F_SETFL, flags | O_NONBLOCK);
  flags = fcntl(fd, F_GETFL);
  fcntl(fd, F_SETFL, flags & ~O_NDELAY);

Not clearing the flag.

> 
> 3. Should sparc32-linux (and glibc) change it's definition? Say, from
>    #if defined(__sparc__) && defined(__arch64__)
>    #define O_NDELAY        0x0004
>    #else
>    #define O_NDELAY        (0x0004 | O_NONBLOCK)
>    #endif
>   to
>    #define O_NDELAY        (0x0004 | O_NONBLOCK)
>   or even to 
>   #define O_NDELAY        O_NONBLOCK
>   and make sure kernel maps old O_NDELAY to O_NONBLOCK?
> 
>   I think '#define O_NDELAY O_NONBLOCK' would be most
>   consistent.
> 
> What do you think?

I think the main issue here is in fact FIONBIO historical inconsistency
over different system that Linux originally tried to accommodate it:

fs/ioctl.c:

545 static int ioctl_fionbio(struct file *filp, int __user *argp)
546 {
547         unsigned int flag;
548         int on, error;
549 
550         error = get_user(on, argp);
551         if (error)
552                 return error;
553         flag = O_NONBLOCK;
554 #ifdef __sparc__
555         /* SunOS compatibility item. */
556         if (O_NONBLOCK != O_NDELAY)
557                 flag |= O_NDELAY;
558 #endif
559         spin_lock(&filp->f_lock);
560         if (on)
561                 filp->f_flags |= flag;
562         else
563                 filp->f_flags &= ~flag;
564         spin_unlock(&filp->f_lock);
565         return error;
566 }

The issue on sparc is FIONBIO will always try to set/reset *both*
flags at the same time. I think what would be better would be to
either define O_NDELAY and O_NONBLOCK to be the same value of
0x4004 on both sparc32 and sparc64 (since the kernel does treat
them semantically as the same) or try to avoid use FIONBIO set the
socket as non-blocking in favor or fcntl(fd, F_SETFL, ... O_NONBLOCK).

  parent reply	other threads:[~2020-06-22 19:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29  9:40 sparc vs sparc64: O_NDELAY and O_NONBLOCK mismatch in kernel and in glibc Sergei Trofimovich via Libc-alpha
2020-05-29 10:48 ` John Paul Adrian Glaubitz
2020-06-22 19:08 ` Adhemerval Zanella via Libc-alpha [this message]
2020-08-11 23:42 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a213fcf7-73e3-7727-dea2-f73e1032a307@linaro.org \
    --to=libc-alpha@sourceware.org \
    --cc=adhemerval.zanella@linaro.org \
    --cc=davem@davemloft.net \
    --cc=mgorny@gentoo.org \
    --cc=slyfox@gentoo.org \
    --cc=sparc@gentoo.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=toolchain@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).