Re: [PATCH v4 2/3] 32-bit ABIs: support stat syscall family

[Arnd Bergmann <arnd at arndb dot de>]

On Tuesday, August 9, 2016 8:40:44 PM CEST Yury Norov wrote:
> > 
> > For stat, there are two distinct issues, and it's possible that
> > I also forgot about the second one in previous emails:
> > 
> > 1. We want to have a generic structure that is used on riscv32
> >    and on future architectures that all share the common 'struct
> >    stat64' layout from the Linux kernel, and that should absolutely
> >    use 64-bit ino_t and off_t.
> > 
> > 2. Because of what we decided for the kernel ABI, aarch64/ilp32
> >    actually uses a nonstandard structure definition that is
> >    uses 64-bit time_t unlike any other architecture. This port
> >    is unique here because we have to deal with 32-bit ARM support
> >    in the kernel and we don't want to have yet another set of
> >    syscall handlers for the stat family.
> 
> Arnd, I don't understand it. Right now we use standard unistd.h in
> kernel which assumes we use standard structs stat and statfs. Correct?

We do? That would be a bug, because the kernel by default uses
the defintion of 'stat64' from arch/arm64/include/asm/stat.h,
which is what arch/arm/ uses for its native syscall, and which
is slightly different from the structure definition in 
include/uapi/asm-generic/stat.h.

For all I know, we had decided to use the binary layout of
'struct stat' that 64-bit machines use, and that is *not* what
the standard unistd.h expects.

> And in glibc we take stat{,fs} layouts from standard
> sysdeps/unix/sysv/linux/generic/bits (though we add few macros, but
> nobody complains). So what non-standard definitions are you meaning?

I looked up the details again now:

The binary layout is almost identical on big-endian builds,
except the ARM structure does not support 64-bit st_ino numbers
in the second 64-bit field, but rather has them in the last
64-bit field at the end of the structure, as indicated by the
'STAT64_HAS_BROKEN_ST_INO' macro.

On little-endian machines, st_ino is more broken, as only the
upper half of the 64-bit field is set by the kernel.

I was actually expecting larger differences, so I guess we could
decide to just set STAT64_HAS_BROKEN_ST_INO here and be done
with it.

> For time_t aarch64/ilp32 uses 32-bit type like any other 32-bit port.
> To avoid introducing another set of handlers/conversion layers, I
> added pads in userspace which lets us to pass ilp32 syscalls to
> lp64 handlers w/o compat conversions.

Ok, this matches with my understanding, in my previous email
I was just explaining why we use the lp64 handlers for 'stat',
as that normally makes no sense here.

> > The second point is where it gets interesting, and I agree it's
> > not as clear-cut as I hoped it would be. I can see a multitude
> > of ways to deal with this, and to make this worse, most of them
> > require agreement from both kernel and glibc folks regarding how
> > we want to handle this in the future.
> > 
> > Let me try to describe what I can see as possible ways forward:
> > 
> > a) use the normal 'struct stat' definition that we get with
> >    _FILE_OFFSET_BITS=64 and with 32-bit time_t on aarch64-ilp32,
> >    and convert the kernel structure to that in an aarch64
> >    specific helper in glibc. This is probably the easiest way.
> > 
> > b) revise 'struct stat64' in the kernel to use 64-bit timestamps
> >    for all future architectures, and get glibc to expose
> >    that to applications.
> 
> I don't expose 64-bit timestamps. I leave pads, so kernel fills it
> with 64-bit data. But then I use conv_timespec() to convert
> them to 32-bit representations. So 64-bit time exists in glibc
> internals only and not exposed to applications.
> 
> >    This is basically what you are doing
> >    now, except with buy-in from the kernel side. The main
> >    reason we haven't done this already is that there has
> >    been talk about a new 'xstat' syscall interface with
> >    a much-expanded ABI for multiple years now, and it still
> >    looks like that's coming any time soon.
> > 
> > c) finally get 'xstat' into the kernel, and add a new
> >    stat implementation based on that for glibc, obsoleting
> >    any architecture differences. This would also help with
> >    the y2038 effort (at least on the kernel side).
> 
> I didn't read xstat thread. For 64-bit time only, we don't need
> new interface I, think.

See https://lwn.net/Articles/685519/ for a description of
what xstat does, it is useful for a number of reasons, one
of them being 64-bit timestamps.

> > d) go back to your previous approach of defining 'struct stat'
> >    as aarch64-ilp32 specific, acknowledging that we made a
> >    mistake in the assumption that it would be the new generic
> >    format.
> > 
> > e) go further back to defining the aarch64-ilp32 stat structure
> >    to be identical with the 32-bit arm structure and change
> >    the kernel ABI to use that too. This would avoid the need
> >    for any conversion, and the need for defini
> 
> e) is what I really don't like. Aarch32 is something really
> non-standard. So if we choose between non-standard 32-bit ABI and
> standard 64-bit one, and both of them are already exist, I'd choose
> 64-bit of course.

Actually if STAT64_HAS_BROKEN_ST_INO is the only incompatibility,
I don't really see a problem with that. The only downside of
this is that we don't have any remaining 64-bit padding fields that
we could use for extending 'struct stat64', but that is true for
many other architectures, and 'xstat' will solve the problem.

	Arnd