* [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default
@ 2021-11-20 21:09 Stafford Horne via Libc-alpha
2021-11-22 13:00 ` Adhemerval Zanella via Libc-alpha
0 siblings, 1 reply; 6+ messages in thread
From: Stafford Horne via Libc-alpha @ 2021-11-20 21:09 UTC (permalink / raw)
To: GLIBC patches
In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the
default for STAT64_IS_KERNEL_STAT64 was removed. This patch adds it
back.
For architectures that want to used the default kernel_stat.h and do not
have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined. Set
the default as 1 as modern port's stat64 struct should match the kernel
stat64 layout.
I tested this on the OpenRISC port and it seems to work fine. Currently,
all archs that use the default kernel_stat.h define __NR_newfstatat so
they will not use the STAT64_IS_KERNEL_STAT64 macro. However, arc seems
to be an outlier it uses the default kernel_stat.h, but does not define
__NR_newfstatat or __NR_fstatat64 I am not clear how arc works here.
---
sysdeps/unix/sysv/linux/kernel_stat.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/sysdeps/unix/sysv/linux/kernel_stat.h b/sysdeps/unix/sysv/linux/kernel_stat.h
index 6c1b175cb4..b8c376f41e 100644
--- a/sysdeps/unix/sysv/linux/kernel_stat.h
+++ b/sysdeps/unix/sysv/linux/kernel_stat.h
@@ -20,3 +20,4 @@
#define XSTAT_IS_XSTAT64 1
#define STATFS_IS_STATFS64 __STATFS_MATCHES_STATFS64
#define STAT_IS_KERNEL_STAT 1
+#define STAT64_IS_KERNEL_STAT64 1
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default 2021-11-20 21:09 [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default Stafford Horne via Libc-alpha @ 2021-11-22 13:00 ` Adhemerval Zanella via Libc-alpha 2021-11-23 21:39 ` Stafford Horne via Libc-alpha 0 siblings, 1 reply; 6+ messages in thread From: Adhemerval Zanella via Libc-alpha @ 2021-11-22 13:00 UTC (permalink / raw) To: Stafford Horne, GLIBC patches On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote: > In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the > default for STAT64_IS_KERNEL_STAT64 was removed. This patch adds it > back. > > For architectures that want to used the default kernel_stat.h and do not > have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined. Set > the default as 1 as modern port's stat64 struct should match the kernel > stat64 layout. > > I tested this on the OpenRISC port and it seems to work fine. Currently, > all archs that use the default kernel_stat.h define __NR_newfstatat so > they will not use the STAT64_IS_KERNEL_STAT64 macro. However, arc seems > to be an outlier it uses the default kernel_stat.h, but does not define > __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here. arc and usually newer 32-bit ports will only use __NR_statx: 138 #if (__WORDSIZE == 32 \ 139 && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ 140 || defined STAT_HAS_TIME32 141 # define FSTATAT_USE_STATX 1 142 #else 143 # define FSTATAT_USE_STATX 0 144 #endif The patch looks ok, but it seems strange that ork1 requires it since it setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX would be defined and only fstatat64_time64_statx would be used. > --- > sysdeps/unix/sysv/linux/kernel_stat.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/sysdeps/unix/sysv/linux/kernel_stat.h b/sysdeps/unix/sysv/linux/kernel_stat.h > index 6c1b175cb4..b8c376f41e 100644 > --- a/sysdeps/unix/sysv/linux/kernel_stat.h > +++ b/sysdeps/unix/sysv/linux/kernel_stat.h > @@ -20,3 +20,4 @@ > #define XSTAT_IS_XSTAT64 1 > #define STATFS_IS_STATFS64 __STATFS_MATCHES_STATFS64 > #define STAT_IS_KERNEL_STAT 1 > +#define STAT64_IS_KERNEL_STAT64 1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default 2021-11-22 13:00 ` Adhemerval Zanella via Libc-alpha @ 2021-11-23 21:39 ` Stafford Horne via Libc-alpha 2021-11-24 15:50 ` Adhemerval Zanella via Libc-alpha 0 siblings, 1 reply; 6+ messages in thread From: Stafford Horne via Libc-alpha @ 2021-11-23 21:39 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: GLIBC patches On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote: > > > On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote: > > In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the > > default for STAT64_IS_KERNEL_STAT64 was removed. This patch adds it > > back. > > > > For architectures that want to used the default kernel_stat.h and do not > > have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined. Set > > the default as 1 as modern port's stat64 struct should match the kernel > > stat64 layout. > > > > I tested this on the OpenRISC port and it seems to work fine. Currently, > > all archs that use the default kernel_stat.h define __NR_newfstatat so > > they will not use the STAT64_IS_KERNEL_STAT64 macro. However, arc seems > > to be an outlier it uses the default kernel_stat.h, but does not define > > __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here. > > arc and usually newer 32-bit ports will only use __NR_statx: > > 138 #if (__WORDSIZE == 32 \ > 139 && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ > 140 || defined STAT_HAS_TIME32 > 141 # define FSTATAT_USE_STATX 1 > 142 #else > 143 # define FSTATAT_USE_STATX 0 > 144 #endif > > The patch looks ok, but it seems strange that ork1 requires it since it > setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX > would be defined and only fstatat64_time64_statx would be used. Right, In that case maybe another ifdef is needed in fstatat64.c? I don't see fstatat64_time64_stat is actually getting compiled into the libc.so binary. But if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error. I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in the compile unit and I get the below which looks right: * __ASSUME_STATX: 1 * FSTATAT_USE_STATX: 1 Error: or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -ftls-model=initial-exec -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc -I../sysdeps/unix/sysv/linux/or1k -I../sysdeps/or1k/nptl -I../sysdeps/unix/sysv/linux/generic/wordsize-32 -I../sysdeps/unix/sysv/linux/generic -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/or1k/nofpu -I../sysdeps/ieee754/soft-fp -I../sysdeps/or1k -I../sysdeps/wordsize-32 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’: ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef] 89 | # if STAT64_IS_KERNEL_STAT64 | ^~~~~~~~~~~~~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known 94 | struct kernel_stat64 kst64; | ^~~~~ ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration] 97 | __cp_stat64_kstat64 (buf, &kst64); | ^~~~~~~~~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable] 94 | struct kernel_stat64 kst64; | ^~~~~ ../sysdeps/unix/sysv/linux/fstatat64.c: At top level: ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’ 149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX) | ^~~~~~~ ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’ 150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX) | ^~~~~~~ Again I am not sure how arc avoids this error I shall try to compile it too. -Stafford > > --- > > sysdeps/unix/sysv/linux/kernel_stat.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/sysdeps/unix/sysv/linux/kernel_stat.h b/sysdeps/unix/sysv/linux/kernel_stat.h > > index 6c1b175cb4..b8c376f41e 100644 > > --- a/sysdeps/unix/sysv/linux/kernel_stat.h > > +++ b/sysdeps/unix/sysv/linux/kernel_stat.h > > @@ -20,3 +20,4 @@ > > #define XSTAT_IS_XSTAT64 1 > > #define STATFS_IS_STATFS64 __STATFS_MATCHES_STATFS64 > > #define STAT_IS_KERNEL_STAT 1 > > +#define STAT64_IS_KERNEL_STAT64 1 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default 2021-11-23 21:39 ` Stafford Horne via Libc-alpha @ 2021-11-24 15:50 ` Adhemerval Zanella via Libc-alpha 2021-11-24 21:29 ` Stafford Horne via Libc-alpha 0 siblings, 1 reply; 6+ messages in thread From: Adhemerval Zanella via Libc-alpha @ 2021-11-24 15:50 UTC (permalink / raw) To: Stafford Horne; +Cc: GLIBC patches On 23/11/2021 18:39, Stafford Horne wrote: > On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote: >> >> >> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote: >>> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the >>> default for STAT64_IS_KERNEL_STAT64 was removed. This patch adds it >>> back. >>> >>> For architectures that want to used the default kernel_stat.h and do not >>> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined. Set >>> the default as 1 as modern port's stat64 struct should match the kernel >>> stat64 layout. >>> >>> I tested this on the OpenRISC port and it seems to work fine. Currently, >>> all archs that use the default kernel_stat.h define __NR_newfstatat so >>> they will not use the STAT64_IS_KERNEL_STAT64 macro. However, arc seems >>> to be an outlier it uses the default kernel_stat.h, but does not define >>> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here. >> >> arc and usually newer 32-bit ports will only use __NR_statx: >> >> 138 #if (__WORDSIZE == 32 \ >> 139 && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ >> 140 || defined STAT_HAS_TIME32 >> 141 # define FSTATAT_USE_STATX 1 >> 142 #else >> 143 # define FSTATAT_USE_STATX 0 >> 144 #endif >> >> The patch looks ok, but it seems strange that ork1 requires it since it >> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX >> would be defined and only fstatat64_time64_statx would be used. > > Right, > > In that case maybe another ifdef is needed in fstatat64.c? I don't see > fstatat64_time64_stat is actually getting compiled into the libc.so binary. But > if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error. > > I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in > the compile unit and I get the below which looks right: > > * __ASSUME_STATX: 1 > * FSTATAT_USE_STATX: 1 > > Error: > > or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -ftls-model=initial-exec -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc -I../sysdeps/unix/sysv/linux/or1k -I../sysdeps/or1k/nptl -I../sysdeps/unix/sysv/linux/generic/wordsize-32 -I../sysdeps/unix/sysv/linux/generic -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/or1k/nofpu -I../sysdeps/ieee754/soft-fp -I../sysdeps/or1k -I../sysdeps/wordsize-32 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o > ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’: > ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef] > 89 | # if STAT64_IS_KERNEL_STAT64 > | ^~~~~~~~~~~~~~~~~~~~~~~ > ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known > 94 | struct kernel_stat64 kst64; > | ^~~~~ > ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration] > 97 | __cp_stat64_kstat64 (buf, &kst64); > | ^~~~~~~~~~~~~~~~~~~ > ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable] > 94 | struct kernel_stat64 kst64; > | ^~~~~ > ../sysdeps/unix/sysv/linux/fstatat64.c: At top level: > ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’ > 149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX) > | ^~~~~~~ > ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’ > 150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX) > | ^~~~~~~ > > Again I am not sure how arc avoids this error I shall try to compile it too. The problem is 'fstatat64_time64_stat' should not be build for 32-bit architectures with __ASSUME_STATX (since only statx provides 64-bit timestamps). Other architecture might get an improved codegen using older syscalls (specially 64-bit ones), so statx should be used only when really required. The patch below fixes it: diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c index f968e4ef05..50ae5ad748 100644 --- a/sysdeps/unix/sysv/linux/fstatat64.c +++ b/sysdeps/unix/sysv/linux/fstatat64.c @@ -74,6 +74,17 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf, return r; } +#if (__WORDSIZE == 32 \ + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ + || defined STAT_HAS_TIME32 +# define FSTATAT_USE_STATX 1 +#else +# define FSTATAT_USE_STATX 0 +#endif + +/* Only statx supports 64-bit timestamps for 32-bit architectures with + __ASSUME_STATX, so there is no point in building the fallback. */ +#if !FSTATAT_USE_STATX || (FSTATAT_USE_STATX && !defined __ASSUME_STATX) static inline int fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf, int flag) @@ -134,13 +145,6 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf, return r; } - -#if (__WORDSIZE == 32 \ - && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ - || defined STAT_HAS_TIME32 -# define FSTATAT_USE_STATX 1 -#else -# define FSTATAT_USE_STATX 0 #endif int ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default 2021-11-24 15:50 ` Adhemerval Zanella via Libc-alpha @ 2021-11-24 21:29 ` Stafford Horne via Libc-alpha 2021-11-25 12:53 ` Adhemerval Zanella via Libc-alpha 0 siblings, 1 reply; 6+ messages in thread From: Stafford Horne via Libc-alpha @ 2021-11-24 21:29 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: GLIBC patches On Wed, Nov 24, 2021 at 12:50:40PM -0300, Adhemerval Zanella wrote: > > > On 23/11/2021 18:39, Stafford Horne wrote: > > On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote: > >> > >> > >> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote: > >>> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the > >>> default for STAT64_IS_KERNEL_STAT64 was removed. This patch adds it > >>> back. > >>> > >>> For architectures that want to used the default kernel_stat.h and do not > >>> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined. Set > >>> the default as 1 as modern port's stat64 struct should match the kernel > >>> stat64 layout. > >>> > >>> I tested this on the OpenRISC port and it seems to work fine. Currently, > >>> all archs that use the default kernel_stat.h define __NR_newfstatat so > >>> they will not use the STAT64_IS_KERNEL_STAT64 macro. However, arc seems > >>> to be an outlier it uses the default kernel_stat.h, but does not define > >>> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here. > >> > >> arc and usually newer 32-bit ports will only use __NR_statx: > >> > >> 138 #if (__WORDSIZE == 32 \ > >> 139 && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ > >> 140 || defined STAT_HAS_TIME32 > >> 141 # define FSTATAT_USE_STATX 1 > >> 142 #else > >> 143 # define FSTATAT_USE_STATX 0 > >> 144 #endif > >> > >> The patch looks ok, but it seems strange that ork1 requires it since it > >> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX > >> would be defined and only fstatat64_time64_statx would be used. > > > > Right, > > > > In that case maybe another ifdef is needed in fstatat64.c? I don't see > > fstatat64_time64_stat is actually getting compiled into the libc.so binary. But > > if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error. > > > > I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in > > the compile unit and I get the below which looks right: > > > > * __ASSUME_STATX: 1 > > * FSTATAT_USE_STATX: 1 > > > > Error: > > > > or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -ftls-model=initial-exec -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc -I../sysdeps/unix/sysv/linux/or1k -I../sysdeps/or1k/nptl -I../sysdeps/unix/sysv/linux/generic/wordsize-32 -I../sysdeps/unix/sysv/linux/generic -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/or1k/nofpu -I../sysdeps/ieee754/soft-fp -I../sysdeps/or1k -I../sysdeps/wordsize-32 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o > > ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’: > > ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef] > > 89 | # if STAT64_IS_KERNEL_STAT64 > > | ^~~~~~~~~~~~~~~~~~~~~~~ > > ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known > > 94 | struct kernel_stat64 kst64; > > | ^~~~~ > > ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration] > > 97 | __cp_stat64_kstat64 (buf, &kst64); > > | ^~~~~~~~~~~~~~~~~~~ > > ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable] > > 94 | struct kernel_stat64 kst64; > > | ^~~~~ > > ../sysdeps/unix/sysv/linux/fstatat64.c: At top level: > > ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’ > > 149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX) > > | ^~~~~~~ > > ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’ > > 150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX) > > | ^~~~~~~ > > > > Again I am not sure how arc avoids this error I shall try to compile it too. > > The problem is 'fstatat64_time64_stat' should not be build for 32-bit architectures > with __ASSUME_STATX (since only statx provides 64-bit timestamps). Other architecture > might get an improved codegen using older syscalls (specially 64-bit ones), so statx > should be used only when really required. > > The patch below fixes it: Right, this was what I was thinking too. However, I am still not sure why or1k needs it and arc does not. I tested the arc build and confirmed with the same version of GCC the arc build passes but or1k fails with the above error. So as you said, maybe there is better codegen somewhere. > diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c > index f968e4ef05..50ae5ad748 100644 > --- a/sysdeps/unix/sysv/linux/fstatat64.c > +++ b/sysdeps/unix/sysv/linux/fstatat64.c > @@ -74,6 +74,17 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf, > return r; > } > > +#if (__WORDSIZE == 32 \ > + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ > + || defined STAT_HAS_TIME32 > +# define FSTATAT_USE_STATX 1 > +#else > +# define FSTATAT_USE_STATX 0 > +#endif > + > +/* Only statx supports 64-bit timestamps for 32-bit architectures with > + __ASSUME_STATX, so there is no point in building the fallback. */ > +#if !FSTATAT_USE_STATX || (FSTATAT_USE_STATX && !defined __ASSUME_STATX) > static inline int > fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf, > int flag) > @@ -134,13 +145,6 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf, > > return r; > } > - > -#if (__WORDSIZE == 32 \ > - && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ > - || defined STAT_HAS_TIME32 > -# define FSTATAT_USE_STATX 1 > -#else > -# define FSTATAT_USE_STATX 0 > #endif > > int I tested this patch and it works. -Stafford ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default 2021-11-24 21:29 ` Stafford Horne via Libc-alpha @ 2021-11-25 12:53 ` Adhemerval Zanella via Libc-alpha 0 siblings, 0 replies; 6+ messages in thread From: Adhemerval Zanella via Libc-alpha @ 2021-11-25 12:53 UTC (permalink / raw) To: Stafford Horne; +Cc: GLIBC patches On 24/11/2021 18:29, Stafford Horne wrote: > On Wed, Nov 24, 2021 at 12:50:40PM -0300, Adhemerval Zanella wrote: >> >> >> On 23/11/2021 18:39, Stafford Horne wrote: >>> On Mon, Nov 22, 2021 at 10:00:31AM -0300, Adhemerval Zanella wrote: >>>> >>>> >>>> On 20/11/2021 18:09, Stafford Horne via Libc-alpha wrote: >>>>> In commit 36260d5035 ("linux: Set default kernel_stat.h to LFS") the >>>>> default for STAT64_IS_KERNEL_STAT64 was removed. This patch adds it >>>>> back. >>>>> >>>>> For architectures that want to used the default kernel_stat.h and do not >>>>> have __NR_newfstatat, STAT64_IS_KERNEL_STAT64 needs to be defined. Set >>>>> the default as 1 as modern port's stat64 struct should match the kernel >>>>> stat64 layout. >>>>> >>>>> I tested this on the OpenRISC port and it seems to work fine. Currently, >>>>> all archs that use the default kernel_stat.h define __NR_newfstatat so >>>>> they will not use the STAT64_IS_KERNEL_STAT64 macro. However, arc seems >>>>> to be an outlier it uses the default kernel_stat.h, but does not define >>>>> __NR_newfstatat or __NR_fstatat64 I am not clear how arc works here. >>>> >>>> arc and usually newer 32-bit ports will only use __NR_statx: >>>> >>>> 138 #if (__WORDSIZE == 32 \ >>>> 139 && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ >>>> 140 || defined STAT_HAS_TIME32 >>>> 141 # define FSTATAT_USE_STATX 1 >>>> 142 #else >>>> 143 # define FSTATAT_USE_STATX 0 >>>> 144 #endif >>>> >>>> The patch looks ok, but it seems strange that ork1 requires it since it >>>> setting minimum required kernel to 5.10. I would expect that __ASSUME_STATX >>>> would be defined and only fstatat64_time64_statx would be used. >>> >>> Right, >>> >>> In that case maybe another ifdef is needed in fstatat64.c? I don't see >>> fstatat64_time64_stat is actually getting compiled into the libc.so binary. But >>> if I don't define STAT64_IS_KERNEL_STAT64 I get the below compile error. >>> >>> I added a pragma to output the value of FSTATAT_USE_STATX and __ASSUME_STATX in >>> the compile unit and I get the below which looks right: >>> >>> * __ASSUME_STATX: 1 >>> * FSTATAT_USE_STATX: 1 >>> >>> Error: >>> >>> or1k-glibc-linux-gnu-gcc ../sysdeps/unix/sysv/linux/fstatat64.c -c -std=gnu11 -fgnu89-inline -g -O2 -Wall -Wwrite-strings -Wundef -Werror -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common -Wstrict-prototypes -Wold-style-definition -fmath-errno -ftls-model=initial-exec -I../include -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io -I/home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc -I../sysdeps/unix/sysv/linux/or1k -I../sysdeps/or1k/nptl -I../sysdeps/unix/sysv/linux/generic/wordsize-32 -I../sysdeps/unix/sysv/linux/generic -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/or1k/nofpu -I../sysdeps/ieee754/soft-fp -I../sysdeps/or1k -I../sysdeps/wordsize-32 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/libc-modules.h -DMODULE_NAME=libc -include ../include/libc-symbols.h -DTOP_NAMESPACE=glibc -o /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o -MD -MP -MF /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o.dt -MT /home/shorne/work/gnu-toolchain/build-many/build/glibcs/or1k-linux-gnu-soft/glibc/io/fstatat64.o >>> ../sysdeps/unix/sysv/linux/fstatat64.c: In function ‘fstatat64_time64_stat’: >>> ../sysdeps/unix/sysv/linux/fstatat64.c:89:7: error: "STAT64_IS_KERNEL_STAT64" is not defined, evaluates to 0 [-Werror=undef] >>> 89 | # if STAT64_IS_KERNEL_STAT64 >>> | ^~~~~~~~~~~~~~~~~~~~~~~ >>> ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: storage size of ‘kst64’ isn’t known >>> 94 | struct kernel_stat64 kst64; >>> | ^~~~~ >>> ../sysdeps/unix/sysv/linux/fstatat64.c:97:5: error: implicit declaration of function ‘__cp_stat64_kstat64’ [-Werror=implicit-function-declaration] >>> 97 | __cp_stat64_kstat64 (buf, &kst64); >>> | ^~~~~~~~~~~~~~~~~~~ >>> ../sysdeps/unix/sysv/linux/fstatat64.c:94:24: error: unused variable ‘kst64’ [-Werror=unused-variable] >>> 94 | struct kernel_stat64 kst64; >>> | ^~~~~ >>> ../sysdeps/unix/sysv/linux/fstatat64.c: At top level: >>> ../sysdeps/unix/sysv/linux/fstatat64.c:149:9: note: ‘#pragma message: The value of FSTATAT_USE_STATX: 1’ >>> 149 | #pragma message "The value of FSTATAT_USE_STATX: " XSTR(FSTATAT_USE_STATX) >>> | ^~~~~~~ >>> ../sysdeps/unix/sysv/linux/fstatat64.c:150:9: note: ‘#pragma message: The value of __ASSUME_STATX: 1’ >>> 150 | #pragma message "The value of __ASSUME_STATX: " XSTR(__ASSUME_STATX) >>> | ^~~~~~~ >>> >>> Again I am not sure how arc avoids this error I shall try to compile it too. >> >> The problem is 'fstatat64_time64_stat' should not be build for 32-bit architectures >> with __ASSUME_STATX (since only statx provides 64-bit timestamps). Other architecture >> might get an improved codegen using older syscalls (specially 64-bit ones), so statx >> should be used only when really required. >> >> The patch below fixes it: > > Right, this was what I was thinking too. However, I am still not sure why or1k > needs it and arc does not. I tested the arc build and confirmed with the same > version of GCC the arc build passes but or1k fails with the above error. So as > you said, maybe there is better codegen somewhere. ARC has: sysdeps/unix/sysv/linux/arc/fixup-asm-unistd.h:24:#undef __NR_fstatat64 Which leads fstatat64_time64_stat to be an empty function: static inline int fstatat64_time64_stat (int fd, const char *file, struct stat64 *buf, int flag) { int r; # 135 "../sysdeps/unix/sysv/linux/fstatat64.c" return r; } > >> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c >> index f968e4ef05..50ae5ad748 100644 >> --- a/sysdeps/unix/sysv/linux/fstatat64.c >> +++ b/sysdeps/unix/sysv/linux/fstatat64.c >> @@ -74,6 +74,17 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf, >> return r; >> } >> >> +#if (__WORDSIZE == 32 \ >> + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ >> + || defined STAT_HAS_TIME32 >> +# define FSTATAT_USE_STATX 1 >> +#else >> +# define FSTATAT_USE_STATX 0 >> +#endif >> + >> +/* Only statx supports 64-bit timestamps for 32-bit architectures with >> + __ASSUME_STATX, so there is no point in building the fallback. */ >> +#if !FSTATAT_USE_STATX || (FSTATAT_USE_STATX && !defined __ASSUME_STATX) >> static inline int >> fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf, >> int flag) >> @@ -134,13 +145,6 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf, >> >> return r; >> } >> - >> -#if (__WORDSIZE == 32 \ >> - && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \ >> - || defined STAT_HAS_TIME32 >> -# define FSTATAT_USE_STATX 1 >> -#else >> -# define FSTATAT_USE_STATX 0 >> #endif >> >> int > > I tested this patch and it works. Thanks, I will push it upstream. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-25 12:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-20 21:09 [PATCH] linux: Define STAT64_IS_KERNEL_STAT64 by default Stafford Horne via Libc-alpha 2021-11-22 13:00 ` Adhemerval Zanella via Libc-alpha 2021-11-23 21:39 ` Stafford Horne via Libc-alpha 2021-11-24 15:50 ` Adhemerval Zanella via Libc-alpha 2021-11-24 21:29 ` Stafford Horne via Libc-alpha 2021-11-25 12:53 ` Adhemerval Zanella via Libc-alpha
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).