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

[Yury Norov <ynorov at caviumnetworks dot com>]

On Tue, Aug 09, 2016 at 05:59:24PM +0200, Arnd Bergmann wrote:
> sYcomyljbmeKCvvhv5G/JU0dNbNWxw==
> MIME-Version: 1.0
> Status: O
> Content-Length: 5693
> Lines: 114
> 
> On Tuesday, August 9, 2016 6:15:43 PM CEST Yury Norov wrote:
> > On Tue, Aug 09, 2016 at 04:23:27PM +0200, Arnd Bergmann wrote:
> > > On Tuesday, August 9, 2016 4:26:45 PM CEST Yury Norov wrote:
> > > > > As for struct timespec inside struct stat: are you sure that special 
> > > > > handling in the struct stat declaration is the right thing to do, rather 
> > > > > than arranging for this port to define struct timespec to contain the 
> > > > > padding members and be appropriately aligned (with the kernel made to 
> > > > > ignore the high parts when struct timespec is passed to the kernel from an 
> > > > > ILP32 process, since those high parts are considered padding in 
> > > > > userspace)?  Putting the padding in struct timespec so the layout is 
> > > > > genuinely compatible between userspace and the kernel would seem a lot 
> > > > > less intrusive in glibc.  (Because of the pre-__USE_XOPEN2K8 case in 
> > > > > bits/stat.h, I suppose you'd need an implementation-namespace macro or two 
> > > > > somewhere for defining time_t and nanoseconds fields, that macro 
> > > > > automatically declaring the padding field as needed, and that macro then 
> > > > > being used in bits/stat.h as well as in the original definition of struct 
> > > > > timespec.)
> > > > 
> > > > It was my initial idea, but Arnd told that we cannot modify time_t and
> > > > struct timespec as it is used in some ioctls and this change may harm
> > > > compatibility.
> > > 
> > > Right. The discussion for the y2038 glibc port has progressed a bit,
> > > and we will in fact see a way to use 64-bit glibc time_t with a kernel
> > > that has 32-bit time_t for backwards compatibility reasons, but this
> > > is very far from being at the point where a glibc port can make that
> > > the only option.
> > >  
> > 
> > Could you share this discussion to me?
> 
> See the thread "Fourth draft of the Y2038 design document"


Thanks

> > > Alternatively you can add a sysdeps/unix/sysv/linux/aarch64/bits/stat.h
> > > file that defines the structure the same way that the kernel does, but
> > > you don't need to come up with a generic way of doing this, as (almost
> > > certainly) no other architecture will have this problem, and we will have
> > > to solve the more general 64-bit time_t problem independently.
> > 
> > This is how it was implemented initially. But Joseph requested to
> > re-use generic glibc code as much as possible. In fact, if we use
> > 64-bit off_t etc, the only difference is struct timespec, and there
> > are 3 options we should consider to handle it:
> >  1. Add pads to generic structure (current).
> >  2. Declare stat structures in arch code (initial).
> >  3. Introduce riscv generic ABI - what I'm asking about here.
> 
> Ok, I see. I would not change anything for riscv *here*, because
> riscv does not have the issue of having to be compatible with
> a nonstandard 'struct stat64' in one of two 32-bit user space
> ABIs.
> 
> > struct stat{,fs} layout for aarch64/ilp32 is going to be the new
> > standard for 32-bit ports, so I think we'd end up with 1 or 3, and
> > that's what Joseph tells about, if I understand him right.
> 
> For statfs certainly: we want to use a single structure definition
> for any new architectures, including aarch64-ilp32 and riscv32.
> 
> 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?

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?


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.

> 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.

> 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.