2023-07-12 09:35:14

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support

__NR_statx has been added from v4.10:

commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")

It has been supported by all of the platforms since at least from v4.20
and glibc 2.28.

Let's remove the old arch related and dependent sys_stat support
completely.

This is friendly to the future new architecture porting.

Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/include/nolibc/arch-aarch64.h | 28 -------------
tools/include/nolibc/arch-arm.h | 37 -----------------
tools/include/nolibc/arch-i386.h | 26 ------------
tools/include/nolibc/arch-mips.h | 28 -------------
tools/include/nolibc/arch-riscv.h | 23 -----------
tools/include/nolibc/arch-s390.h | 25 ------------
tools/include/nolibc/arch-x86_64.h | 27 -------------
tools/include/nolibc/sys.h | 63 +++++------------------------
tools/include/nolibc/types.h | 4 +-
9 files changed, 13 insertions(+), 248 deletions(-)

diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index 6227b77a4a09..1bf122cd5966 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -9,34 +9,6 @@

#include "compiler.h"

-/* The struct returned by the newfstatat() syscall. Differs slightly from the
- * x86_64's stat one by field ordering, so be careful.
- */
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned int st_mode;
- unsigned int st_nlink;
- unsigned int st_uid;
- unsigned int st_gid;
-
- unsigned long st_rdev;
- unsigned long __pad1;
- long st_size;
- int st_blksize;
- int __pad2;
-
- long st_blocks;
- long st_atime;
- unsigned long st_atime_nsec;
- long st_mtime;
-
- unsigned long st_mtime_nsec;
- long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned int __unused[2];
-};
-
/* Syscalls for AARCH64 :
* - registers are 64-bit
* - stack is 16-byte aligned
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index 4d4887a5f04b..ea723596ed23 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -9,43 +9,6 @@

#include "compiler.h"

-/* The struct returned by the stat() syscall, 32-bit only, the syscall returns
- * exactly 56 bytes (stops before the unused array). In big endian, the format
- * differs as devices are returned as short only.
- */
-struct sys_stat_struct {
-#if defined(__ARMEB__)
- unsigned short st_dev;
- unsigned short __pad1;
-#else
- unsigned long st_dev;
-#endif
- unsigned long st_ino;
- unsigned short st_mode;
- unsigned short st_nlink;
- unsigned short st_uid;
- unsigned short st_gid;
-
-#if defined(__ARMEB__)
- unsigned short st_rdev;
- unsigned short __pad2;
-#else
- unsigned long st_rdev;
-#endif
- unsigned long st_size;
- unsigned long st_blksize;
- unsigned long st_blocks;
-
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
- unsigned long st_mtime_nsec;
-
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned long __unused[2];
-};
-
/* Syscalls for ARM in ARM or Thumb modes :
* - registers are 32-bit
* - stack is 8-byte aligned
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index 4c6b7c04e2e7..1d4b683bc2cd 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -9,32 +9,6 @@

#include "compiler.h"

-/* The struct returned by the stat() syscall, 32-bit only, the syscall returns
- * exactly 56 bytes (stops before the unused array).
- */
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned short st_mode;
- unsigned short st_nlink;
- unsigned short st_uid;
- unsigned short st_gid;
-
- unsigned long st_rdev;
- unsigned long st_size;
- unsigned long st_blksize;
- unsigned long st_blocks;
-
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
- unsigned long st_mtime_nsec;
-
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned long __unused[2];
-};
-
/* Syscalls for i386 :
* - mostly similar to x86_64
* - registers are 32-bit
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index a2bfdf57b957..ae40d5268793 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -9,34 +9,6 @@

#include "compiler.h"

-/* The struct returned by the stat() syscall. 88 bytes are returned by the
- * syscall.
- */
-struct sys_stat_struct {
- unsigned int st_dev;
- long st_pad1[3];
- unsigned long st_ino;
- unsigned int st_mode;
- unsigned int st_nlink;
- unsigned int st_uid;
- unsigned int st_gid;
- unsigned int st_rdev;
- long st_pad2[2];
- long st_size;
- long st_pad3;
-
- long st_atime;
- long st_atime_nsec;
- long st_mtime;
- long st_mtime_nsec;
-
- long st_ctime;
- long st_ctime_nsec;
- long st_blksize;
- long st_blocks;
- long st_pad4[14];
-};
-
/* Syscalls for MIPS ABI O32 :
* - WARNING! there's always a delayed slot!
* - WARNING again, the syntax is different, registers take a '$' and numbers
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index cd958b2f4b1b..2b89ea59c5e4 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -9,29 +9,6 @@

#include "compiler.h"

-struct sys_stat_struct {
- unsigned long st_dev; /* Device. */
- unsigned long st_ino; /* File serial number. */
- unsigned int st_mode; /* File mode. */
- unsigned int st_nlink; /* Link count. */
- unsigned int st_uid; /* User ID of the file's owner. */
- unsigned int st_gid; /* Group ID of the file's group. */
- unsigned long st_rdev; /* Device number, if device. */
- unsigned long __pad1;
- long st_size; /* Size of file, in bytes. */
- int st_blksize; /* Optimal block size for I/O. */
- int __pad2;
- long st_blocks; /* Number 512-byte blocks allocated. */
- long st_atime; /* Time of last access. */
- unsigned long st_atime_nsec;
- long st_mtime; /* Time of last modification. */
- unsigned long st_mtime_nsec;
- long st_ctime; /* Time of last status change. */
- unsigned long st_ctime_nsec;
- unsigned int __unused4;
- unsigned int __unused5;
-};
-
#if __riscv_xlen == 64
#define PTRLOG "3"
#define SZREG "8"
diff --git a/tools/include/nolibc/arch-s390.h b/tools/include/nolibc/arch-s390.h
index a644ecd361c0..a40424ba043e 100644
--- a/tools/include/nolibc/arch-s390.h
+++ b/tools/include/nolibc/arch-s390.h
@@ -10,31 +10,6 @@

#include "compiler.h"

-/* The struct returned by the stat() syscall, equivalent to stat64(). The
- * syscall returns 116 bytes and stops in the middle of __unused.
- */
-
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned long st_nlink;
- unsigned int st_mode;
- unsigned int st_uid;
- unsigned int st_gid;
- unsigned int __pad1;
- unsigned long st_rdev;
- unsigned long st_size;
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
- unsigned long st_mtime_nsec;
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- unsigned long st_blksize;
- long st_blocks;
- unsigned long __unused[3];
-};
-
/* Syscalls for s390:
* - registers are 64-bit
* - syscall number is passed in r1
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index e69113742a99..e980ca5417d0 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -9,33 +9,6 @@

#include "compiler.h"

-/* The struct returned by the stat() syscall, equivalent to stat64(). The
- * syscall returns 116 bytes and stops in the middle of __unused.
- */
-struct sys_stat_struct {
- unsigned long st_dev;
- unsigned long st_ino;
- unsigned long st_nlink;
- unsigned int st_mode;
- unsigned int st_uid;
-
- unsigned int st_gid;
- unsigned int __pad0;
- unsigned long st_rdev;
- long st_size;
- long st_blksize;
-
- long st_blocks;
- unsigned long st_atime;
- unsigned long st_atime_nsec;
- unsigned long st_mtime;
-
- unsigned long st_mtime_nsec;
- unsigned long st_ctime;
- unsigned long st_ctime_nsec;
- long __unused[3];
-};
-
/* Syscalls for x86_64 :
* - registers are 64-bit
* - syscall number is passed in rax
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index dee56894a811..8bfe7db20b80 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -943,15 +943,19 @@ pid_t setsid(void)
return __sysret(sys_setsid());
}

-#if defined(__NR_statx)
/*
* int statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf);
+ * int stat(const char *path, struct stat *buf);
*/

static __attribute__((unused))
int sys_statx(int fd, const char *path, int flags, unsigned int mask, struct statx *buf)
{
+#ifdef __NR_statx
return my_syscall5(__NR_statx, fd, path, flags, mask, buf);
+#else
+ return -ENOSYS;
+#endif
}

static __attribute__((unused))
@@ -959,24 +963,18 @@ int statx(int fd, const char *path, int flags, unsigned int mask, struct statx *
{
return __sysret(sys_statx(fd, path, flags, mask, buf));
}
-#endif

-/*
- * int stat(const char *path, struct stat *buf);
- * Warning: the struct stat's layout is arch-dependent.
- */

-#if defined(__NR_statx) && !defined(__NR_newfstatat) && !defined(__NR_stat)
-/*
- * Maybe we can just use statx() when available for all architectures?
- */
static __attribute__((unused))
-int sys_stat(const char *path, struct stat *buf)
+int stat(const char *path, struct stat *buf)
{
struct statx statx;
long ret;

- ret = sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &statx);
+ ret = __sysret(sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, STATX_BASIC_STATS, &statx));
+ if (ret == -1)
+ return ret;
+
buf->st_dev = ((statx.stx_dev_minor & 0xff)
| (statx.stx_dev_major << 8)
| ((statx.stx_dev_minor & ~0xff) << 12));
@@ -997,47 +995,8 @@ int sys_stat(const char *path, struct stat *buf)
buf->st_mtim.tv_nsec = statx.stx_mtime.tv_nsec;
buf->st_ctim.tv_sec = statx.stx_ctime.tv_sec;
buf->st_ctim.tv_nsec = statx.stx_ctime.tv_nsec;
- return ret;
-}
-#else
-static __attribute__((unused))
-int sys_stat(const char *path, struct stat *buf)
-{
- struct sys_stat_struct stat;
- long ret;
-
-#ifdef __NR_newfstatat
- /* only solution for arm64 */
- ret = my_syscall4(__NR_newfstatat, AT_FDCWD, path, &stat, 0);
-#elif defined(__NR_stat)
- ret = my_syscall2(__NR_stat, path, &stat);
-#else
- return -ENOSYS;
-#endif
- buf->st_dev = stat.st_dev;
- buf->st_ino = stat.st_ino;
- buf->st_mode = stat.st_mode;
- buf->st_nlink = stat.st_nlink;
- buf->st_uid = stat.st_uid;
- buf->st_gid = stat.st_gid;
- buf->st_rdev = stat.st_rdev;
- buf->st_size = stat.st_size;
- buf->st_blksize = stat.st_blksize;
- buf->st_blocks = stat.st_blocks;
- buf->st_atim.tv_sec = stat.st_atime;
- buf->st_atim.tv_nsec = stat.st_atime_nsec;
- buf->st_mtim.tv_sec = stat.st_mtime;
- buf->st_mtim.tv_nsec = stat.st_mtime_nsec;
- buf->st_ctim.tv_sec = stat.st_ctime;
- buf->st_ctim.tv_nsec = stat.st_ctime_nsec;
- return ret;
-}
-#endif

-static __attribute__((unused))
-int stat(const char *path, struct stat *buf)
-{
- return __sysret(sys_stat(path, buf));
+ return 0;
}


diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 23963e48d8ee..8cfc4c860fa4 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -15,8 +15,8 @@


/* Only the generic macros and types may be defined here. The arch-specific
- * ones such as the O_RDONLY and related macros used by fcntl() and open(), or
- * the layout of sys_stat_struct must not be defined here.
+ * ones such as the O_RDONLY and related macros used by fcntl() and open()
+ * must not be defined here.
*/

/* stat flags (WARNING, octal here). We need to check for an existing
--
2.25.1



2023-07-15 09:19:27

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support

Hi Zhangjin,

On Wed, Jul 12, 2023 at 05:16:34PM +0800, Zhangjin Wu wrote:
> __NR_statx has been added from v4.10:
>
> commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")
>
> It has been supported by all of the platforms since at least from v4.20
> and glibc 2.28.
>
> Let's remove the old arch related and dependent sys_stat support
> completely.
>
> This is friendly to the future new architecture porting.

As I previously said, I'd like that we at least preserve compatibility
with supported stable branches. 4.14 and 4.19 are still supported, so
does this mean that if I rebuild my preinit against the updated nolibc
it will fail to boot on such older systems for the archs that we support?

Because if it means that in order to support all currently active
kernels, one must only build from an older copy of the lib, that becomes
a disservice to its development and usage. Thus if you checked that aarch64,
arm, i386, mips, riscv, s390 and x86_64 had already adopted statx by 4.14,
then I'm fine and we can drop stat(), but then it must be mentioned in
the commit message, because here it's not explicit.

Thanks!
Willy

2023-07-15 10:55:39

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support

> Hi Zhangjin,
>
> On Wed, Jul 12, 2023 at 05:16:34PM +0800, Zhangjin Wu wrote:
> > __NR_statx has been added from v4.10:
> >
> > commit a528d35e8bfc ("statx: Add a system call to make enhanced file info available")
> >
> > It has been supported by all of the platforms since at least from v4.20
> > and glibc 2.28.
> >
> > Let's remove the old arch related and dependent sys_stat support
> > completely.
> >
> > This is friendly to the future new architecture porting.
>
> As I previously said, I'd like that we at least preserve compatibility
> with supported stable branches. 4.14 and 4.19 are still supported, so
> does this mean that if I rebuild my preinit against the updated nolibc
> it will fail to boot on such older systems for the archs that we support?
>
> Because if it means that in order to support all currently active
> kernels, one must only build from an older copy of the lib, that becomes
> a disservice to its development and usage. Thus if you checked that aarch64,
> arm, i386, mips, riscv, s390 and x86_64 had already adopted statx by 4.14,
> then I'm fine and we can drop stat(), but then it must be mentioned in
> the commit message, because here it's not explicit.
>

Yeah, we used 'git blame' before and found the last related change is
v4.20, but it is not really the first change.

Just read the statx manpage again:

https://man7.org/linux/man-pages/man2/statx.2.html

It shows something about "Linux 4.11, glibc 2.28.

And 'git grep' shows it is true:

$ git grep -r statx v4.11 arch/ include/uapi/asm-generic/unistd.h | grep -E "aarch64|arm|mips|s390|x86|:include/uapi"
v4.11:arch/arm/tools/syscall.tbl:397 common statx sys_statx
v4.11:arch/arm64/include/asm/unistd32.h:#define __NR_statx 397
v4.11:arch/arm64/include/asm/unistd32.h:__SYSCALL(__NR_statx, sys_statx)
v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 366)
v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 326)
v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 330)
v4.11:arch/mips/kernel/scall32-o32.S: PTR sys_statx
v4.11:arch/mips/kernel/scall64-64.S: PTR sys_statx
v4.11:arch/mips/kernel/scall64-n32.S: PTR sys_statx /* 6330 */
v4.11:arch/mips/kernel/scall64-o32.S: PTR sys_statx
v4.11:arch/s390/include/uapi/asm/unistd.h:#define __NR_statx 379
v4.11:arch/s390/kernel/compat_wrapper.c:COMPAT_SYSCALL_WRAP5(statx, int, dfd, const char __user *, path, unsigned, flags, unsigned, mask, struct statx __user *, buffer);
v4.11:arch/s390/kernel/syscalls.S:SYSCALL(sys_statx,compat_sys_statx)
v4.11:arch/x86/entry/syscalls/syscall_32.tbl:383 i386 statx sys_statx
v4.11:arch/x86/entry/syscalls/syscall_64.tbl:332 common statx sys_statx
v4.11:include/uapi/asm-generic/unistd.h:#define __NR_statx 291
v4.11:include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_statx, sys_statx)

both riscv and loongarch use the generic unistd.h, so, all of our
supported archs should work as-is. riscv itself is added from v4.15,
loongarch itself is added from v5.19.

The powerpc we plan to support is from v4.11:

$ git grep -r statx v4.11 arch/powerpc/
v4.11:arch/powerpc/include/asm/systbl.h:SYSCALL(statx)
v4.11:arch/powerpc/include/uapi/asm/unistd.h:#define __NR_statx 383

So, let's simply correct the commit message (the old v4.10 has a wrong -1
offset, I wrongly used 'git show a528d35e8bfc:Makefile' before).

sys_statx has been supported from Linux v4.11 and glibc 2.28:

$ git grep -r statx v4.11 arch/ include/uapi/asm-generic/unistd.h

Both riscv (firstly added from v4.15) and loongarch (firstly added from
v5.19) use the generic unistd.h, the others are supported from arch/.

Let's remove the old arch related and dependent sys_stat support
completely.

Since the current oldest stable branch is v4.14, so, using statx
instead of sys_stat preserves compatibility with all of the
supported stable branches.

This is friendly to the future new architecture porting.

Best regards,
Zhangjin

> Thanks!
> Willy

2023-07-15 13:14:57

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 01/11] tools/nolibc: remove the old sys_stat support

On Sat, Jul 15, 2023 at 06:39:41PM +0800, Zhangjin Wu wrote:
> Just read the statx manpage again:
>
> https://man7.org/linux/man-pages/man2/statx.2.html
>
> It shows something about "Linux 4.11, glibc 2.28.
>
> And 'git grep' shows it is true:
>
> $ git grep -r statx v4.11 arch/ include/uapi/asm-generic/unistd.h | grep -E "aarch64|arm|mips|s390|x86|:include/uapi"
> v4.11:arch/arm/tools/syscall.tbl:397 common statx sys_statx
> v4.11:arch/arm64/include/asm/unistd32.h:#define __NR_statx 397
> v4.11:arch/arm64/include/asm/unistd32.h:__SYSCALL(__NR_statx, sys_statx)
> v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 366)
> v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 326)
> v4.11:arch/mips/include/uapi/asm/unistd.h:#define __NR_statx (__NR_Linux + 330)
> v4.11:arch/mips/kernel/scall32-o32.S: PTR sys_statx
> v4.11:arch/mips/kernel/scall64-64.S: PTR sys_statx
> v4.11:arch/mips/kernel/scall64-n32.S: PTR sys_statx /* 6330 */
> v4.11:arch/mips/kernel/scall64-o32.S: PTR sys_statx
> v4.11:arch/s390/include/uapi/asm/unistd.h:#define __NR_statx 379
> v4.11:arch/s390/kernel/compat_wrapper.c:COMPAT_SYSCALL_WRAP5(statx, int, dfd, const char __user *, path, unsigned, flags, unsigned, mask, struct statx __user *, buffer);
> v4.11:arch/s390/kernel/syscalls.S:SYSCALL(sys_statx,compat_sys_statx)
> v4.11:arch/x86/entry/syscalls/syscall_32.tbl:383 i386 statx sys_statx
> v4.11:arch/x86/entry/syscalls/syscall_64.tbl:332 common statx sys_statx
> v4.11:include/uapi/asm-generic/unistd.h:#define __NR_statx 291
> v4.11:include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_statx, sys_statx)
>
> both riscv and loongarch use the generic unistd.h, so, all of our
> supported archs should work as-is. riscv itself is added from v4.15,
> loongarch itself is added from v5.19.

So that's perfect, thank you!

Willy