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

[Alistair Francis <alistair23 at gmail dot com>]

On Thu, Jun 27, 2019 at 8:24 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jun 27, 2019 at 1:12 PM Florian Weimer <fweimer@redhat.com> wrote:
> > * Arnd Bergmann:
> > >> 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.
> >
> > But futex is different because in userspace, futex_time64 behaves
> > exactly like futex on other architectures on rv32.
>
> Only as long as nobody tries to define their own __timespec structure to
> pass into futex, based on the traditional definition. Given the lack
> or a correct way to get to the definition, that would not be an
> unreasonable thing to do for getting it to work on arm32
> as a fallback.

I'm unclear what the decision here is. I currently have a patch that does this:

+#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
+#endif

for futex and other missing syscalls in my RV32 port. It seems to be
working fine for RV32 (limited testing only).

What is the consensus here? I would like to try and tidy up the patch
set next week so that others can use this for RV32 until the glibc
merge window opens again.

Alistair

>
> > > 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.
> >
> > There's only __kernel_old_timeval, no __kernel_old_timespec, so you
> > can't use the kernel headers to write the 32-bit fallback path.  Once we
> > get _TIME_BITS=64 (to change struct timespec to 64-bit, like
> > _FILE_OFFSET_BITS), you can't use the glibc header files either.  Not
> > good.
>
> Right, that is clearly part of the cleanup that needs to happen.
> I thought we had actually merged the __kernel_old_timespec
> patch at some point, but evidently we have not.
>
> > >> 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,
> >
> > I'm not sure.  glib could deprecate all the APIs that use time_t
> > externally.  Given that they don't want to bump ABI, they may not want
> > to compile with __TIMESIZE == 64 on old 32-bit systems.
>
> From a very brief look at glib, my impression is that this is again a
> rather special case: all the external interfaces in glib are already
> independent of sizeof(time_t), and internally it should be time64
> safe already when built against a C library with 64-bit time_t
> (all internal types use 64-bit seconds), except for the futex usage
> usage in g_cond_wait_until().
>
> [glib has a different problem with its deprecated GTimeVal type
>  that they will have to solve to actually run beyond 2038, but
>  let's not get into that]
>
> Many other libraries do have external interfaces based on time_t,
> and have to deal with those breaking between different values
> of _TIME_BITS. I suspect the only realistic way to deal with this
> on a large scale is to do a full distro rebuild with _TIME_BITS=64
> and then fix whatever broke, but not expect a solution at the ABI
> level like glibc is doing to support both versions in one binary.
>
> > >> 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.
> >
> > That's a different concern.  They didn't want a 32-bit-time_t ABI.  I
> > expect that they might have chosen differently if they realized that
> > they'd need to absorb the userspace 64-bit porting cost, just to get
> > rv32 going.
>
> I warned them that this would be extra work, but I also hope that
> we will help each other while bringing up time64 distro support
> on rv32, arm32 and other 32-bit architectures.
>
> > > 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.
> >
> > glibc isn't the problem.  It's source-level changes to applications
> > which use kernel headers.
>
> Right, I should have not pointed to glibc in particular. The problem
> is really anything in user space that makes assumptions about
> time_t and how it relates to other types and interfaces.
>
> Applications using __NR_futex (and I hope less commonly the
> other time64 syscalls) need to be fixed for arm32, and I hope
> to get some help on this if the rv32 developers get there first.
>
> Another example that is independent of the kernel is apparently
> the Python C extension API, which fundamentally assumes that
> 'time_t' is the same as 'long'. If we first address this on arm32,
> this will help rv32 as well.
>
>       Arnd