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: [PATCH v4 2/3] 32-bit ABIs: support stat syscall family


Hi,

On 2016/8/9 22:23, 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.

I suggested to declare new time64_t and struct timespec64 and use it
in union with 32-bit versions where possible but it was rejected too.
So with 32-bit time_t/timespec, this is the only option to treat time
fields as special case and introduce a bunch of paddings.

Let's take a step back here. We had a very long discussion about the
kernel ABI, and in particular the definition of 'struct stat64' for
aarch64-ilp32. To recap, there were three options (we probably had
an implementation for each one at some point):

a) use the generic definition for 'stat64' from the architecture
    independent headers: This has the downside of requiring extra
    syscall entry points in the kernel for the new ABI, as the layout
    is different from what 32-bit ARM uses, and our normal compat
    handlers provide those.

b) use the definition from 32-bit ARM, and share the compat stat64
    syscalls: This has the downside of using an awkward layout that
    has the STAT64_HAS_BROKEN_ST_INO set, and has two copies of
    st_ino.

c) Use the layout that is compatible with 64-bit ARM (aarch64) kernels:
    The downside here is that it does not use the standard types for
    the time stamps, so you end up having to handle it in an architecture
    specific way in user space.

We ended up with approach c), which seemed the most future-proof
ABI, but if the decision was to not use the generic ABI, why do you
try to add this to the generic implementation? The easiest way
would be to just have a aarch64 specific conversion function that
copies the data from the kernel structure into the generic user
space structure.

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.

Joseph or Andreas can probably say which of those ways for handling
this in aarch64 specific code is better (override bits/stat.h or
convert the structure), and we can also go back to approach b)
above (using the 32-bit ARM compatible structure) if that ends
up being simpler.
Compare with b and c. IIUC, the difference is where we convert the stat/statfs.
b convert in kernel while c convert in glibc. I wanted to test the performance
between these two approaches. But after read the code, I found that kernel
always convert stat/statfs, it means that b is faster than c. If we chose b, we
need to define the stat64 in aarch64/bit/stat.h. Any reason why we could not do
it?

	Arnd



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