Re: [PATCH 2/2] [AARCH64] ILP32: stat syscalls

[Arnd Bergmann <arnd at arndb dot de>]

On Monday, September 12, 2016 6:37:18 PM CEST Yury Norov wrote:
> On Mon, Sep 12, 2016 at 11:43:08AM +0200, Arnd Bergmann wrote:
> > On Monday, September 12, 2016 1:17:48 AM CEST Yury Norov wrote:
> > > By similar reasons I wire sys_getrlimit and sys_setrlimit to lp64.
> > 
> > I don't see that one making sense either.
> 
> It was suggested by Andreas:
> https://lkml.org/lkml/2016/7/5/77

Ah, I think we just missed that with the addition of prlimit64, the old
getrlimit/setrlimit syscall entry points got obsolete.

I think we should handle this the same way as renameat, which got
superceded by renameat2: not define the syscall entry point from
the kernel and let glibc handle it by calling the replacement
syscall.

Overriding getrlimit/setrlimit to the 64-bit types in the kernel
is wrong, it just makes the ABI different from all other
architectures, and we should be able to handle this without
any architecture specific code.

> > > I don't understand it. Some of this defines are needed because arm64
> > > takes syscalls from sysdeps/unix/sysv/linux, not from
> > > sysdeps/unix/sysv/linux/generic/worsize-32. I don't think it's
> > > generic. Though, if it's generic, could you explain it in details, and
> > > I'll change it.
> > 
> > I didn't realize we took syscalls from sysdeps/unix/sysv/linux instead
> > of sysdeps/unix/sysv/linux/generic/wordsize-32. Why is that? Changing
> > this seems like the easiest fix.
> 
> Because syscalls under generic/wordsize-32 use stat_overflow(), and it
> fails at compile time as struct stat has no pads:
> ../sysdeps/unix/sysv/linux/generic/wordsize-32/overflow.h:39:10: error: ‘struct stat’ has no member named      ‘__st_ino_pad’
>     if (buf->__st_ino_pad == 0 && buf->__st_size_pad == 0 &&
> 
> GCC complains on stat_overflow() even when I try to build
> sysdeps/unix/sysv/linux/generic/worsize-32/statfs.c

Hmm, but this just tells me that the generic/wordsize-32 syscalls
currently assume that you want different implementations for
32-bit off_t and 64-bit off_t, which we don't want for new
architectures any more.

> There's XSTAT_IS_XSTAT64 option to bypass stat_overflow() in stat
> syscalls but there's no such option for statfs syscalls. The other
> problem I see is that standard syscall files generate different
> function bodies for 32- and 64-bit syscalls. And this is the
> duplication I'd like to avoid.

But with your patch 1/2, you are fixing this already: the
broken __lxstat/__xstat implementations don't get compiled any
more and instead redirect to the correct __lxstat64/__xstat64
functions that don't have this problem.

statfs can simply do the same thing.

> So statfs syscalls under generic/wordsize-32:
>  - are broken, and should be fixed in kernel. 

I don't see anything broken in the kernel.

>  - create duplicated function bodies in binaries.

Easily fixed by doing the respective "don't duplicate statfs if..."
patch the same way that you did "don't duplicate lxstat, xstat".

>  - make me do the deep rework for owerflow.h, probably introduce new
>    option.

No, that file simply never gets included when we know we have 64-bit
off_t etc.

>  - limit __FSWORD_T_TYPE, to 32 bit, which is probably makes troubles
>    for big filesystems, which may be a case for servers.
>
> That's why I took sysdeps/unix/sysv/linux version. It looks more
> natural if I set __FSWORD_T_TYPE to 64-bit: I don't have to change
> anything to make things work.

If you think that __FSWORD_T_TYPE is not enough, we should change
it for all architectures and introduce a new statfs64 replacement.

Given the fields that use it, I don't see what could possibly
overflow though:

    __fsword_t f_type;
    __fsword_t f_bsize;
    __fsword_t f_namelen;
    __fsword_t f_frsize;
    __fsword_t f_flags;

I certainly don't want to see arm64 diverge from the generic syscall
ABI just because you think that one of those fields might overflow
in the future.

	Arnd