This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]