[PATCH v5 00/14] port C-SKY to glibc

[Mao Han]

On Wed, Nov 21, 2018 at 05:01:32PM +0000, Joseph Myers wrote:
> On Wed, 21 Nov 2018, Mao Han wrote:
> 
> > I'v modified patch 14/14 to a generic version with __NR3264_fstatat
> > conditionals (in the attachment). It is tested in the same environment
> > as patch V5 and got same result, but have't test fstat64 path yet.
> 
> Thanks, this is the sort of thing I'm looking for, but I'm concerned about 
> the use of __NR3264_fstatat.  As I understand, that's an internal 
> implementation detail of the asm-generic unistd.h; it shouldn't be 
> considered something stable that will reliably be there in future kernel 
> versions.  Rather, the stable interface from asm/unistd.h is the 
> __NR_<syscall> macros for each <syscall> provided by the kernel.  So I 
> think the #ifdef conditionals need to be on the actual syscalls used in 
> each case (e.g. you'd condition on __NR_fstat64 when using fstat64 in the 
> existing INLINE_SYSCALL use).  In statx_cp.c you'll need to find some stat 
> syscall to condition on that's present on all existing architectures, as 
> the aim is that no existing glibc architecture should get any code 
> compiled from that file (which you could check using 
> build-many-glibcs.py).

Yes. __NR_<syscall> macros should be used here. I'v change __NR3264_fstat*
to __NR_fstat*.
Current unistd.h alway have __NR_fstatat64 defined to __NR3264_fstatat
for 32-bit platform, so I used __NR3264_fstatat to make conditionals
work. As Arnd said that __NR3264_fstatat and the related macro will be
gone in 4.20 and the patch is not forwarded yet, C-SKY will get some
compile error with current kernel header, I added some conditionals
in unistd.h for test:

+#if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64)
#define __NR_fstatat64 __NR3264_fstatat
#define __NR_fstat64 __NR3264_fstat
+#endif

Current statx_cp.c will check __NR_fstat64, __NR_fstatat64, __WORDSIZE == 32
to see if it is a 32 bit platforms without fstat64; __NR_fstat and
__NR_newfstatat check is for mips64-*-n32 and x86_64-*-x32 they always use
64 bit system call but got __WORDSIZE == 32.
I'v add an #error in statx_cp.c while using build-many-glibcs.py to do
the build test. Seems no other arch compiled code inside statx_cp.c
 
> Also, it's a bad idea to duplicate the contents of 
> sysdeps/unix/sysv/linux/fxstat64.c in 
> sysdeps/unix/sysv/linux/generic/wordsize-32/fxstat64.c - rather, keep the 
> existing #include, and modify sysdeps/unix/sysv/linux/fxstat64.c 
> appropriately to handle the statx case (which definitely requires using 
> __NR_fstat64 in the #ifdef, since this file is used in cases not using the 
> asm-generic syscall list at all).
>

OK. I've modified sysdeps/unix/sysv/linux/fxstat64.c directly. I was quite
hesitate to change that file as it will affect more arch.

> You're using AT_NO_AUTOMOUNT in these statx calls - does that match what 
> the old stat syscalls do on other architectures?  (I think the aim there 
> should be to match the old stat syscalls, so if those don't automount for 
> stat of an automount point, then using AT_NO_AUTOMOUNT is correct.)

AT_NO_AUTOMOUNT is from ./include/linux/fs.h in the kernel. fstatat64
will call vfs_fstatat and fstat64 will call vfs_stat; they both got
AT_NO_AUTOMOUNT inside in call.
static inline int vfs_stat(const char __user *filename, struct kstat *stat)
{
        return vfs_statx(AT_FDCWD, filename, AT_NO_AUTOMOUNT,
                         stat, STATX_BASIC_STATS);
}
static inline int vfs_fstatat(int dfd, const char __user *filename,
                              struct kstat *stat, int flags)
{
        return vfs_statx(dfd, filename, flags | AT_NO_AUTOMOUNT,
                         stat, STATX_BASIC_STATS);
}


Best Regards,
Mao Han
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-statx-conditionals-for-wordsize-32-xstat.c.patch
Type: text/x-diff
Size: 15516 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20181123/3c37fdcb/attachment.bin>