Re: [RFC v2 08/20] sysdeps/wait: Use waitid if avaliable

[Arnd Bergmann <arnd at arndb dot de>]

On Thu, Jun 27, 2019 at 9:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> * Arnd Bergmann:
> > On Wed, Jun 26, 2019 at 5:48 PM Florian Weimer <fweimer@redhat.com> wrote:
> >> * Arnd Bergmann:

> > One problem specific to futex is the fact that glibc does not come with
> > a wrapper for it, so it's up to the applications to decide on which macro
> > to pass.
>
> That's hardly special to futex.  Even if there's a perfectly fine
> wrapper, some people don't want to use it.

Sure, but any application doing that is broken with when it calls into
the kernel with any mismatched types. Calling syscall(__NR_gettimeofday)
or syscall(__NR_futex) with the time64 version of timespec (from the C library)
can't work, just like calling syscall(__NR_ftruncate) with a 64-bit off_t.

Using syscall() is always going to be hard, what makes futex different
is that there is no easy way.

> > For most of the other syscalls, having rv32 not define the macro means
> > that we break applications at compile-time, and are hopefully able to
> > fix them in a way that works on all architectures. In case of futex,
> > I agree that it would be better not to break compilation when you
> > don't pass a timeout, but I see no good way to have it both ways.
>
> I'm more and more confused here.
>
> Does rv32 still have 32-bit struct timeval/struct timespec outside
> getrusage?

Only in a few drivers that we intend to fix before anyone can
use them, so no.

> If not, there is no risk at all that programmers confuse the types, and
> no need to rename the systen calls.
>
> If there's only a 64-bit time_t, calling futex futex_time64 makes as
> much sense as alpha calling getpid getxpid.

No, the rule for syscalls in the kernel is to give a new name if you
change the arguments. When we went through multiple revisions for
fadvise/fadvise64/fadvise64_64, we could have made it so that
each new architecture only has the latest version by the name of
'fadvise', but I think that would have been really confusing.

Admittedly, the ABI for timespec is messed up, since there is
no good way to get to the right argument type. We do have a
'struct timespec' definition in <linux/time.h>, and this is what one
would have to use in combination with __NR_clock_gettime or
__NR_futex, but it clearly conflicts with the definition from
the libc <time.h> when using 64-bit time_t, so you can't include
those two headers together.

I also really don't want to have a different definition of timespec
in the kernel for rv32 and get people to use that. Instead I think
anyone using the __NR_* macros should really use __kernel_timespec
and other __kernel_* types from the kernel headers in place of
the libc types in the long run. Properly cleaning up the kernel
headers is something we definitely need to do at some point
but have no specific plan for.

We have also debated adding all the __NR_*time64 macros for all
64-bit architectures, which would have been more consistent,
but so far have not done that for consistency with
llseek/stat64/fctntl64/truncate64/... that are also 32-bit only.

> > The version above will
> > - always use __NR_futex on 64-bit architectures and x32
> > - use __NR_futex on 32-bit architectures with 32-bit time_t, this will always
> >   work except on kernels that don't support time32 syscalls (as intended)
> > - use __NR_futex_time64 on 32-bit architectures with 64-bit time_t, which
> >   works when built with linux-5.1+ headers and running on linux-5.1+.
> >   This is probably good enough, as many other things are also broken
> >   when trying to use time64 with older kernels or headers.
>
> I think the quoted fix isn't the best we can do.  Portable binaries
> 32-bit binaries need to call futex_time64 once to see if it is
> available, and fall back to futex if it is not.  g_cond_wait_until in
> particular does not have a dependency on the size of struct timespec,
> but other parts of glib might, and it may not be possible to compile
> glib as a while with __TIMESIZE == 64.

Can you clarify this? Making glib work with __TIMESIZE == 64 is
clearly required anyway, and g_cond_wait_until() cannot work
unless the local timeout structure has the same type that the kernel
expects.

> But t his is really an aside.  I'm concerned that porting software to
> rv32 requires that applications have to deal with the time_t transition
> today, and I don't see a good reason for linking the two.

The risc-v kernel maintainers specifically wanted to wait for the time_t
transition with libc, in order for rv32 to avoid going through the transition
after binaries are widely distributed already.

As I said before, we could easily undo the one-line change in the kernel
and start out with the usual 32-bit off_t/time_t/... in glibc if it turns
out to be too hard to get this to work.

     Arnd