[RFC,v6,08/23] RISC-V: Define __NR_* as __NR_*_time64/64 for 32-bit

Message ID cfa2ae3aaba20454673c0bb6e77be535f41272fe.1578824547.git.alistair.francis@wdc.com
State New
Headers show
Series
  • RISC-V glibc port for the 32-bit
Related show

Commit Message

Alistair Francis Jan. 12, 2020, 10:33 a.m.
---
 sysdeps/unix/sysv/linux/riscv/sysdep.h | 73 ++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

-- 
2.24.1

Comments

Arnd Bergmann Jan. 13, 2020, 1:39 p.m. | #1
On Sun, Jan 12, 2020 at 11:40 AM Alistair Francis
<alistair.francis@wdc.com> wrote:

> +# ifndef __NR_timer_gettime

> +#  define __NR_timer_gettime __NR_timer_gettime64

> +# endif

> +

> +# ifndef __NR_timer_settime

> +#  define __NR_timer_settime __NR_timer_settime64

> +# endif

> +

> +# ifndef __NR_clock_getres

> +#  define __NR_clock_getres __NR_clock_getres_time64

> +# endif

> +

> +# ifndef __NR_clock_gettime

> +#  define __NR_clock_gettime __NR_clock_gettime64

> +# endif


What about clock_nanosleep and io_pgetevents?

       Arnd
Alistair Francis Jan. 14, 2020, 7:14 a.m. | #2
On Mon, Jan 13, 2020 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:
>

> On Sun, Jan 12, 2020 at 11:40 AM Alistair Francis

> <alistair.francis@wdc.com> wrote:

>

> > +# ifndef __NR_timer_gettime

> > +#  define __NR_timer_gettime __NR_timer_gettime64

> > +# endif

> > +

> > +# ifndef __NR_timer_settime

> > +#  define __NR_timer_settime __NR_timer_settime64

> > +# endif

> > +

> > +# ifndef __NR_clock_getres

> > +#  define __NR_clock_getres __NR_clock_getres_time64

> > +# endif

> > +

> > +# ifndef __NR_clock_gettime

> > +#  define __NR_clock_gettime __NR_clock_gettime64

> > +# endif

>

> What about clock_nanosleep and io_pgetevents?


I don't think we need clock_nanosleep as it has it's own C file and as
__ASSUME_TIME64_SYSCALLS is defined we can only ever call the *_time64
version. I think this actually applies to a few of the #defines in
this patch and that they can be removed.

My thoughts (maybe wrongly) with these #defines was to use this to
allow glibc to build and eventually we could remove these. The current
ones are what is required to allow the "old" calls in glibc to
transparently call the "new" ones and as we are using a 64-bit time_t
we can happily just change the syscall to the *_time64 version.

Alistair

>

>        Arnd
Arnd Bergmann Jan. 14, 2020, 12:47 p.m. | #3
On Tue, Jan 14, 2020 at 8:15 AM Alistair Francis <alistair23@gmail.com> wrote:
> On Mon, Jan 13, 2020 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:

> > On Sun, Jan 12, 2020 at 11:40 AM Alistair Francis

> > <alistair.francis@wdc.com> wrote:

> >

> > > +# ifndef __NR_timer_gettime

> > > +#  define __NR_timer_gettime __NR_timer_gettime64

> > > +# endif

> > > +

> > > +# ifndef __NR_timer_settime

> > > +#  define __NR_timer_settime __NR_timer_settime64

> > > +# endif

> > > +

> > > +# ifndef __NR_clock_getres

> > > +#  define __NR_clock_getres __NR_clock_getres_time64

> > > +# endif

> > > +

> > > +# ifndef __NR_clock_gettime

> > > +#  define __NR_clock_gettime __NR_clock_gettime64

> > > +# endif

> >

> > What about clock_nanosleep and io_pgetevents?

>

> I don't think we need clock_nanosleep as it has it's own C file and as

> __ASSUME_TIME64_SYSCALLS is defined we can only ever call the *_time64

> version. I think this actually applies to a few of the #defines in

> this patch and that they can be removed.


Ok.

> My thoughts (maybe wrongly) with these #defines was to use this to

> allow glibc to build and eventually we could remove these. The current

> ones are what is required to allow the "old" calls in glibc to

> transparently call the "new" ones and as we are using a 64-bit time_t

> we can happily just change the syscall to the *_time64 version.


I would certainly like that better, using the same names for the
system calls as the kernel, and across all 32-bit architectures
seems less confusing for readers, so the fewer redirects are needed,
the better.

       Arnd
Alistair Francis Jan. 15, 2020, 7:03 a.m. | #4
On Tue, Jan 14, 2020 at 10:48 PM Arnd Bergmann <arnd@arndb.de> wrote:
>

> On Tue, Jan 14, 2020 at 8:15 AM Alistair Francis <alistair23@gmail.com> wrote:

> > On Mon, Jan 13, 2020 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:

> > > On Sun, Jan 12, 2020 at 11:40 AM Alistair Francis

> > > <alistair.francis@wdc.com> wrote:

> > >

> > > > +# ifndef __NR_timer_gettime

> > > > +#  define __NR_timer_gettime __NR_timer_gettime64

> > > > +# endif

> > > > +

> > > > +# ifndef __NR_timer_settime

> > > > +#  define __NR_timer_settime __NR_timer_settime64

> > > > +# endif

> > > > +

> > > > +# ifndef __NR_clock_getres

> > > > +#  define __NR_clock_getres __NR_clock_getres_time64

> > > > +# endif

> > > > +

> > > > +# ifndef __NR_clock_gettime

> > > > +#  define __NR_clock_gettime __NR_clock_gettime64

> > > > +# endif

> > >

> > > What about clock_nanosleep and io_pgetevents?

> >

> > I don't think we need clock_nanosleep as it has it's own C file and as

> > __ASSUME_TIME64_SYSCALLS is defined we can only ever call the *_time64

> > version. I think this actually applies to a few of the #defines in

> > this patch and that they can be removed.

>

> Ok.

>

> > My thoughts (maybe wrongly) with these #defines was to use this to

> > allow glibc to build and eventually we could remove these. The current

> > ones are what is required to allow the "old" calls in glibc to

> > transparently call the "new" ones and as we are using a 64-bit time_t

> > we can happily just change the syscall to the *_time64 version.

>

> I would certainly like that better, using the same names for the

> system calls as the kernel, and across all 32-bit architectures

> seems less confusing for readers, so the fewer redirects are needed,

> the better.


Great! In which case I will keep this list as short as possible.

Alistair

>

>        Arnd
Khem Raj Jan. 25, 2020, 8:33 p.m. | #5
On 1/14/20 11:03 PM, Alistair Francis wrote:
> On Tue, Jan 14, 2020 at 10:48 PM Arnd Bergmann <arnd@arndb.de> wrote:

>>

>> On Tue, Jan 14, 2020 at 8:15 AM Alistair Francis <alistair23@gmail.com> wrote:

>>> On Mon, Jan 13, 2020 at 11:39 PM Arnd Bergmann <arnd@arndb.de> wrote:

>>>> On Sun, Jan 12, 2020 at 11:40 AM Alistair Francis

>>>> <alistair.francis@wdc.com> wrote:

>>>>

>>>>> +# ifndef __NR_timer_gettime

>>>>> +#  define __NR_timer_gettime __NR_timer_gettime64

>>>>> +# endif

>>>>> +

>>>>> +# ifndef __NR_timer_settime

>>>>> +#  define __NR_timer_settime __NR_timer_settime64

>>>>> +# endif

>>>>> +

>>>>> +# ifndef __NR_clock_getres

>>>>> +#  define __NR_clock_getres __NR_clock_getres_time64

>>>>> +# endif

>>>>> +

>>>>> +# ifndef __NR_clock_gettime

>>>>> +#  define __NR_clock_gettime __NR_clock_gettime64

>>>>> +# endif

>>>>

>>>> What about clock_nanosleep and io_pgetevents?

>>>

>>> I don't think we need clock_nanosleep as it has it's own C file and as

>>> __ASSUME_TIME64_SYSCALLS is defined we can only ever call the *_time64

>>> version. I think this actually applies to a few of the #defines in

>>> this patch and that they can be removed.

>>

>> Ok.

>>

>>> My thoughts (maybe wrongly) with these #defines was to use this to

>>> allow glibc to build and eventually we could remove these. The current

>>> ones are what is required to allow the "old" calls in glibc to

>>> transparently call the "new" ones and as we are using a 64-bit time_t

>>> we can happily just change the syscall to the *_time64 version.

>>

>> I would certainly like that better, using the same names for the

>> system calls as the kernel, and across all 32-bit architectures

>> seems less confusing for readers, so the fewer redirects are needed,

>> the better.

> 

> Great! In which case I will keep this list as short as possible.

> 


There is userspace code like [1] which expects SYS_futex
These overrides do not end up in usr/include/bits/syscall-32.h
so it fails to compile

src/xshmfence_futex.h:58:17: error: use of undeclared identifier 
'SYS_futex'; did you mean 'sys_futex'?
         return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);
                        ^~~~~~~~~

I wonder if __NR_futex and other aliases here should be exposed to 
userspace?

[1] 
https://github.com/freedesktop/libxshmfence/blob/master/src/xshmfence_futex.h#L55-L58

> Alistair

> 

>>

>>         Arnd
Arnd Bergmann Jan. 27, 2020, 8:39 a.m. | #6
On Sat, Jan 25, 2020 at 9:33 PM Khem Raj <raj.khem@gmail.com> wrote:
> On 1/14/20 11:03 PM, Alistair Francis wrote:

>

> There is userspace code like [1] which expects SYS_futex

> These overrides do not end up in usr/include/bits/syscall-32.h

> so it fails to compile

>

> src/xshmfence_futex.h:58:17: error: use of undeclared identifier

> 'SYS_futex'; did you mean 'sys_futex'?

>          return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);

>                         ^~~~~~~~~

>

> I wonder if __NR_futex and other aliases here should be exposed to

> userspace?


Please don't, that just makes it harder for the other ports to find
these instances and fix them in a portable way.

Any source code reference to a low-level system call that passes
a time32 type needs to be handled roughly like this:

typedef unsigned int __u32;
#if defined(__x86_64__) && defined(__ILP32__)
typedef long long __kernel_long_t;
#else
typedef long __kernel_long_t;
#endif

typedef __kernel_long_t __kernel_old_time_t;
struct __kernel_old_timespec {
        __kernel_long_t tv_sec;
        __kernel_long_t tv_nsec;
};
typedef long long __kernel_time64_t;
struct __kernel_timespec {
        __kernel_time64_t tv_sec;
        long long tv_nsec;
};

static inline long __kernel_futex_time64(__u32 * uaddr, int op, __u32 val,
                                         struct __kernel_timespec *utime,
                                         __u32 * uaddr2, __u32 val3)
{
#ifdef __NR_futex_time64
        return syscall(__NR_futex_time64, uaddr, op, val, utime, uaddr2, val3);
#else
        errno = -ENOSYS;
        return -1;
#endif
}

static inline long __kernel_futex_old(__u32 * uaddr, int op, __u32 val,
                                      struct __kernel_old_timespec *utime,
                                      __u32 * uaddr2, __u32 val3)
{
#ifdef __NR_futex
        return syscall(__NR_futex, uaddr, op, val, utime, uaddr2, val3);
#else
        errno = -ENOSYS;
        return -1;
#endif

}

static inline long __kernel_futex(__u32 *uaddr, int op, __u32 val,
                                      struct timespec *utime,
                                      __u32 *uaddr2, __u32 val3)
{
        long ret;

        if (sizeof(time_t) > sizeof(__kernel_long_t)) {
                if (utime) {
                        struct __kernel_timespec ts = {
                                .tv_sec = utime->tv_sec,.tv_nsec =
                                    utime->tv_nsec,
                        };
                        ret = __kernel_futex_time64(uaddr, op, val, &ts,
                                                    uaddr, val3);
                } else {
                        ret = __kernel_futex_time64(uaddr, op, val, NULL,
                                                    uaddr, val3);
                }
                if (ret != -1 || errno != -ENOSYS)
                        return ret;
        }
        if (utime) {
                struct __kernel_timespec ts = {
                        .tv_sec = utime->tv_sec,.tv_nsec = utime->tv_nsec,
                };

                return __kernel_futex_old(uaddr, op, val, &ts, uaddr,
                                          val3);
        }

        return __kernel_futex_old(uaddr, op, val, NULL, uaddr, val3);
}

I'm not sure if this covers all corner cases (or even works correctly
at all), but I'm fairly sure that anything simpler would break on some
configurations.

The code above uses the identifiers from the kernel namespace,
and this is how we might decide to distribute it along with the kernel
headers. If applications want to ship their own copy, they might
need to define their own types or require a very recent version of the
kernel headers, as __kernel_old_timespec was only added in linux-5.5
(the older headers just define 'timespec' which conflicts with the libc
type of the same name and cannot be included here at all).

I would like to actually provide automatically generated stubs like
the above from kernel headers, but that clearly requires more
discussion and some more work on turning
include/uapi/asm-generic/unistd.h into machine-readable format
first.

An alternative would be for all C libraries to start providing a futex()
wrapper and a way to identify whether it exists, then the applications
can largely start using that one on all architectures, and fall back to
the time32 version when building against an older libc.

      Arnd
Florian Weimer Jan. 27, 2020, 10:02 a.m. | #7
* Arnd Bergmann:

> On Sat, Jan 25, 2020 at 9:33 PM Khem Raj <raj.khem@gmail.com> wrote:

>> On 1/14/20 11:03 PM, Alistair Francis wrote:

>>

>> There is userspace code like [1] which expects SYS_futex

>> These overrides do not end up in usr/include/bits/syscall-32.h

>> so it fails to compile

>>

>> src/xshmfence_futex.h:58:17: error: use of undeclared identifier

>> 'SYS_futex'; did you mean 'sys_futex'?

>>          return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);

>>                         ^~~~~~~~~

>>

>> I wonder if __NR_futex and other aliases here should be exposed to

>> userspace?

>

> Please don't, that just makes it harder for the other ports to find

> these instances and fix them in a portable way.

>

> Any source code reference to a low-level system call that passes

> a time32 type needs to be handled roughly like this:

>

> typedef unsigned int __u32;

> #if defined(__x86_64__) && defined(__ILP32__)

> typedef long long __kernel_long_t;

> #else

> typedef long __kernel_long_t;

> #endif


Is this the type that glibc and some others call register_t?  I think
some MIPS targets also need a special definition for it.

> typedef __kernel_long_t __kernel_old_time_t;

> struct __kernel_old_timespec {

>         __kernel_long_t tv_sec;

>         __kernel_long_t tv_nsec;

> };

> typedef long long __kernel_time64_t;

> struct __kernel_timespec {

>         __kernel_time64_t tv_sec;

>         long long tv_nsec;

> };


Who is going to provide these types?

If we put it into glibc headers, we should not use the __kernel_*
prefix.

> static inline long __kernel_futex_time64(__u32 * uaddr, int op, __u32 val,

>                                          struct __kernel_timespec *utime,

>                                          __u32 * uaddr2, __u32 val3)

> {

> #ifdef __NR_futex_time64

>         return syscall(__NR_futex_time64, uaddr, op, val, utime, uaddr2, val3);

> #else

>         errno = -ENOSYS;

>         return -1;

> #endif

> }

>

> static inline long __kernel_futex_old(__u32 * uaddr, int op, __u32 val,

>                                       struct __kernel_old_timespec *utime,

>                                       __u32 * uaddr2, __u32 val3)

> {

> #ifdef __NR_futex

>         return syscall(__NR_futex, uaddr, op, val, utime, uaddr2, val3);

> #else

>         errno = -ENOSYS;

>         return -1;

> #endif

>

> }


Ah better design would split the operations that need timeouts from
those which do not, so that only code that actually uses timeouts needs
porting.  This avoids the type-polymorphic val2/utime argument.

> The code above uses the identifiers from the kernel namespace,

> and this is how we might decide to distribute it along with the kernel

> headers. If applications want to ship their own copy, they might

> need to define their own types or require a very recent version of the

> kernel headers, as __kernel_old_timespec was only added in linux-5.5

> (the older headers just define 'timespec' which conflicts with the libc

> type of the same name and cannot be included here at all).


Even for the kernel, __kernel_* prefixes are awkward because the old
type variants are almost exclusively userspace constructs.

> An alternative would be for all C libraries to start providing a futex()

> wrapper and a way to identify whether it exists, then the applications

> can largely start using that one on all architectures, and fall back to

> the time32 version when building against an older libc.


Yes, I think this is the right direction.  In theory, it is possible to
use GCC extensions to implement overloads.  But I tried to do that for
fcntl once, and the macros are rather unwieldy.  Furthermore, C++ neds a
different implementation.

Thanks,
Florian
Arnd Bergmann Jan. 27, 2020, 1:11 p.m. | #8
On Mon, Jan 27, 2020 at 11:02 AM Florian Weimer <fweimer@redhat.com> wrote:
>

> * Arnd Bergmann:

>

> > On Sat, Jan 25, 2020 at 9:33 PM Khem Raj <raj.khem@gmail.com> wrote:

> >> On 1/14/20 11:03 PM, Alistair Francis wrote:

> >>

> >> There is userspace code like [1] which expects SYS_futex

> >> These overrides do not end up in usr/include/bits/syscall-32.h

> >> so it fails to compile

> >>

> >> src/xshmfence_futex.h:58:17: error: use of undeclared identifier

> >> 'SYS_futex'; did you mean 'sys_futex'?

> >>          return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);

> >>                         ^~~~~~~~~

> >>

> >> I wonder if __NR_futex and other aliases here should be exposed to

> >> userspace?

> >

> > Please don't, that just makes it harder for the other ports to find

> > these instances and fix them in a portable way.

> >

> > Any source code reference to a low-level system call that passes

> > a time32 type needs to be handled roughly like this:

> >

> > typedef unsigned int __u32;

> > #if defined(__x86_64__) && defined(__ILP32__)

> > typedef long long __kernel_long_t;

> > #else

> > typedef long __kernel_long_t;

> > #endif

>

> Is this the type that glibc and some others call register_t?  I think

> some MIPS targets also need a special definition for it.


No, this is the type that is only different on x32, mips n32
also uses the regular syscall ABI.

> > typedef __kernel_long_t __kernel_old_time_t;

> > struct __kernel_old_timespec {

> >         __kernel_long_t tv_sec;

> >         __kernel_long_t tv_nsec;

> > };

> > typedef long long __kernel_time64_t;

> > struct __kernel_timespec {

> >         __kernel_time64_t tv_sec;

> >         long long tv_nsec;

> > };

>

> Who is going to provide these types?

>

> If we put it into glibc headers, we should not use the __kernel_*

> prefix.


I used the types from the existing kernel headers here to not have
to come up with a new naming scheme. If we end up providing the
wrappers from the kernel, then these are the types that should be
used. If glibc ends up providing a futex implementation, that
would clearly use the glibc provided types on the interface and
hide the implementation details. If we make the inline version a
standalone header that can be included in applications under
a permissive license, then all the identifiers should probably be
renamed to something that doesn't conflict with either kernel or
glibc.

> > static inline long __kernel_futex_time64(__u32 * uaddr, int op, __u32 val,

> >                                          struct __kernel_timespec *utime,

> >                                          __u32 * uaddr2, __u32 val3)

> > {

> > #ifdef __NR_futex_time64

> >         return syscall(__NR_futex_time64, uaddr, op, val, utime, uaddr2, val3);

> > #else

> >         errno = -ENOSYS;

> >         return -1;

> > #endif

> > }

> >

> > static inline long __kernel_futex_old(__u32 * uaddr, int op, __u32 val,

> >                                       struct __kernel_old_timespec *utime,

> >                                       __u32 * uaddr2, __u32 val3)

> > {

> > #ifdef __NR_futex

> >         return syscall(__NR_futex, uaddr, op, val, utime, uaddr2, val3);

> > #else

> >         errno = -ENOSYS;

> >         return -1;

> > #endif

> >

> > }

>

> Ah better design would split the operations that need timeouts from

> those which do not, so that only code that actually uses timeouts needs

> porting.  This avoids the type-polymorphic val2/utime argument.


I just realized that my version above is completely broken for the
'op' values that pass an integer instead of a pointer, so I agree
this has to be changed in some form that understands the difference
between timeout and val2 passing. :(

My hope was that we can stay close to the in-kernel types for the
prototypes. If we actually autogenerate a header file from the kernel
for all system calls that would help ensure that the prototype that user
space sees is exactly the same one that the kernel sees.

It would almost work by just defining a futex function locally as either
__kernel_futex_time64() or __kernel_futex_old() above based on
sizeof(time_t), but it breaks if anyone ever tries to support the resulting
binary with time64 support on an old kernel.

Having either individual wrappers per futex-op, or a wrapper that
does the conversion based on a switch(op) {} would solve that, but
both have the downside of not working with additional values of
'op' that might be added later.

> > The code above uses the identifiers from the kernel namespace,

> > and this is how we might decide to distribute it along with the kernel

> > headers. If applications want to ship their own copy, they might

> > need to define their own types or require a very recent version of the

> > kernel headers, as __kernel_old_timespec was only added in linux-5.5

> > (the older headers just define 'timespec' which conflicts with the libc

> > type of the same name and cannot be included here at all).

>

> Even for the kernel, __kernel_* prefixes are awkward because the old

> type variants are almost exclusively userspace constructs.


The __kernel_ prefix was always just intended to provide a namespace
for things that may be defined differently from user space and is unlikely
to cause conflicts. It was first added in linux-1.3.79 for basic types
[https://lkml.org/lkml/1996/3/26/68].

> > An alternative would be for all C libraries to start providing a futex()

> > wrapper and a way to identify whether it exists, then the applications

> > can largely start using that one on all architectures, and fall back to

> > the time32 version when building against an older libc.

>

> Yes, I think this is the right direction.  In theory, it is possible to

> use GCC extensions to implement overloads.  But I tried to do that for

> fcntl once, and the macros are rather unwieldy.  Furthermore, C++ neds a

> different implementation.


What is the problem with using a plain uintptr_t or struct timespec* and
requiring callers to add a cast? Is this just a matter of ugly calling
conventions, or are there cases where it causes bigger problems?

Would it help to have the libc futex wrapper take seven arguments with
separate timeout and val2 arguments, and requiring at least one of them
to be 0 or NULL? I think the only reason for the awkward interface in the
kernel is that a syscall with seven arguments would require passing
a structure on most architectures.

      Arnd
Florian Weimer Jan. 27, 2020, 1:17 p.m. | #9
* Arnd Bergmann:

> What is the problem with using a plain uintptr_t or struct timespec* and

> requiring callers to add a cast? Is this just a matter of ugly calling

> conventions, or are there cases where it causes bigger problems?


The problem with casts is that they make code compile which uses the
wrong timespec type.  Old 32-bit ports will have at least three of them,
and we should provide as much guidance as possible to programmers to get
this right.  If we require them to write casts, I don't think we can
achieve that.

> Would it help to have the libc futex wrapper take seven arguments with

> separate timeout and val2 arguments, and requiring at least one of them

> to be 0 or NULL? I think the only reason for the awkward interface in the

> kernel is that a syscall with seven arguments would require passing

> a structure on most architectures.


I hope that nowadays, we'd just add separate system calls instead of
multiplexers (even though futex_time64 doesn't give much hope).  Then
the different operations can use whatever argument types they need.

Thanks,
Florian
Arnd Bergmann Jan. 27, 2020, 2:08 p.m. | #10
On Mon, Jan 27, 2020 at 2:17 PM Florian Weimer <fweimer@redhat.com> wrote:
>

> * Arnd Bergmann:

>

> > What is the problem with using a plain uintptr_t or struct timespec* and

> > requiring callers to add a cast? Is this just a matter of ugly calling

> > conventions, or are there cases where it causes bigger problems?

>

> The problem with casts is that they make code compile which uses the

> wrong timespec type.  Old 32-bit ports will have at least three of them,

> and we should provide as much guidance as possible to programmers to get

> this right.  If we require them to write casts, I don't think we can

> achieve that.


I see, that is definitely a problem if callers mix the kernel types with the
glibc types, even if glibc only provides a single (conditional) definition of
struct timespec in its user-facing headers.

> > Would it help to have the libc futex wrapper take seven arguments with

> > separate timeout and val2 arguments, and requiring at least one of them

> > to be 0 or NULL? I think the only reason for the awkward interface in the

> > kernel is that a syscall with seven arguments would require passing

> > a structure on most architectures.

>

> I hope that nowadays, we'd just add separate system calls instead of

> multiplexers (even though futex_time64 doesn't give much hope).  Then

> the different operations can use whatever argument types they need.


I think that is the general trend, but for futex_time64() the goal was to
stay close to the existing futex() despite its problems. In case of
io_uring() and fs{mount,config,mount,pick}() we fortunately avoided
adding new multiplexers.

Coming back to futex, do you think this would work in a libc header?

/* plain kernel syscalls */
extern inline int __futex_time64(int *uaddr, int futex_op, int val,
                 const struct __timespec64 *timeout,
                 int *uaddr2, int val3);
extern inline int __futex_old(int *uaddr, int futex_op, int val,
                 const struct __timespec_old *timeout,
                 int *uaddr2, int val3);

/* typesafe wrapper, could be inline or out of line */
static __inline int
futex(int *uaddr, int futex_op, int val,
                 int val2, int *uaddr2, int val3,
                 const struct timespec *timeout)
{
         if (sizeof(time_t) > sizeof(long)) {
                  int ret;
                  if (!timeout)
                         return futex_time64(uaddr, futex_op, val,
val2, uaddr2, val3);
                 ret = futex_time64(uaddr, futex_op, val, val2, uaddr2, val3);
                 if (ret == -1 && errno = -ENOSYS) {
                           int ts[2] = { timeout.tv_sec, timeout.tv_nsec };
                           return futex_old(uaddr, futex_op, (void
*)ts, val2, uaddr2, val3);
                 }
       }

       if (!timeout)
                 return futex_old(uaddr, futex_op, val, val2, uaddr2, val3);

       return futex_old(uaddr, futex_op, (void *)timeout, val2, uaddr2, val3);
}

That would be roughly as efficient as the call to syscall() but do the
right thing in all cases, and not require adding an unsafe dummy
__NR_futex definition.

     Arnd

Patch

diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h
index 201bf9a91b..d030bf0db0 100644
--- a/sysdeps/unix/sysv/linux/riscv/sysdep.h
+++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h
@@ -116,6 +116,79 @@ 
 
 #include <sysdeps/unix/sysdep.h>
 
+#if __riscv_xlen == 32
+/* Define the __NR_futex as __NR_futex64 as RV32 doesn't have a
+ * __NR_futex syscall.
+ */
+# ifndef __NR_futex
+#  define __NR_futex __NR_futex_time64
+# endif
+
+# ifndef __NR_rt_sigtimedwait
+#  define __NR_rt_sigtimedwait __NR_rt_sigtimedwait_time64
+# endif
+
+# ifndef __NR_ppoll
+#  define __NR_ppoll __NR_ppoll_time64
+# endif
+
+# ifndef __NR_utimensat
+#  define __NR_utimensat __NR_utimensat_time64
+# endif
+
+# ifndef __NR_pselect6
+#  define __NR_pselect6 __NR_pselect6_time64
+# endif
+
+# ifndef __NR_recvmmsg
+#  define __NR_recvmmsg __NR_recvmmsg_time64
+# endif
+
+# ifndef __NR_semtimedop
+#  define __NR_semtimedop __NR_semtimedop_time64
+# endif
+
+# ifndef __NR_mq_timedreceive
+#  define __NR_mq_timedreceive __NR_mq_timedreceive_time64
+# endif
+
+# ifndef __NR_mq_timedsend
+#  define __NR_mq_timedsend __NR_mq_timedsend_time64
+# endif
+
+# ifndef __NR_timer_gettime
+#  define __NR_timer_gettime __NR_timer_gettime64
+# endif
+
+# ifndef __NR_timer_settime
+#  define __NR_timer_settime __NR_timer_settime64
+# endif
+
+# ifndef __NR_clock_getres
+#  define __NR_clock_getres __NR_clock_getres_time64
+# endif
+
+# ifndef __NR_clock_gettime
+#  define __NR_clock_gettime __NR_clock_gettime64
+# endif
+
+# ifndef __NR_timerfd_settime
+#  define __NR_timerfd_settime __NR_timerfd_settime64
+# endif
+
+# ifndef __NR_timerfd_gettime
+#  define __NR_timerfd_gettime __NR_timerfd_gettime64
+# endif
+
+# ifndef __NR_sched_rr_get_interval
+#  define __NR_sched_rr_get_interval __NR_sched_rr_get_interval_time64
+# endif
+
+# ifndef __NR_clock_adjtime
+#  define __NR_clock_adjtime __NR_clock_adjtime64
+# endif
+#endif /* __riscv_xlen == 32 */
+
 #undef SYS_ify
 #define SYS_ify(syscall_name)	__NR_##syscall_name