From: Feiyang Chen <chenfeiyang@loongson.cn>
Neither __NR_newfstatat nor __NR_stat is defined on new architecture
like LoongArch, but we can use statx() to implement sys_stat().
Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
---
tools/include/nolibc/sys.h | 36 +++++++++++++++++++++++++---
tools/include/nolibc/types.h | 46 ++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index b5f8cd35c03b..7ff9b401008a 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -1054,6 +1054,7 @@ pid_t setsid(void)
* Warning: the struct stat's layout is arch-dependent.
*/
+#if defined(__NR_newfstatat) || defined(__NR_stat)
static __attribute__((unused))
int sys_stat(const char *path, struct stat *buf)
{
@@ -1063,10 +1064,8 @@ int sys_stat(const char *path, struct stat *buf)
#ifdef __NR_newfstatat
/* only solution for arm64 */
ret = my_syscall4(__NR_newfstatat, AT_FDCWD, path, &stat, 0);
-#elif defined(__NR_stat)
+#else /* __NR_stat */
ret = my_syscall2(__NR_stat, path, &stat);
-#else
-#error Neither __NR_newfstatat nor __NR_stat defined, cannot implement sys_stat()
#endif
buf->st_dev = stat.st_dev;
buf->st_ino = stat.st_ino;
@@ -1083,6 +1082,37 @@ int sys_stat(const char *path, struct stat *buf)
buf->st_ctime = stat.st_ctime;
return ret;
}
+#elif defined(__NR_statx)
+/* only solution for loongarch */
+static __attribute__((unused))
+int sys_stat(const char *path, struct stat *buf)
+{
+ struct statx statx;
+ long ret;
+
+ ret = my_syscall5(__NR_statx, AT_FDCWD, path, 0, STATX_BASIC_STATS, &statx);
+ buf->st_dev = ((statx.stx_dev_minor & 0xff)
+ | (statx.stx_dev_major << 8)
+ | ((statx.stx_dev_minor & ~0xff) << 12));
+ buf->st_ino = statx.stx_ino;
+ buf->st_mode = statx.stx_mode;
+ buf->st_nlink = statx.stx_nlink;
+ buf->st_uid = statx.stx_uid;
+ buf->st_gid = statx.stx_gid;
+ buf->st_rdev = ((statx.stx_rdev_minor & 0xff)
+ | (statx.stx_rdev_major << 8) |
+ ((statx.stx_rdev_minor & ~0xff) << 12));
+ buf->st_size = statx.stx_size;
+ buf->st_blksize = statx.stx_blksize;
+ buf->st_blocks = statx.stx_blocks;
+ buf->st_atime = statx.stx_atime.tv_sec;
+ buf->st_mtime = statx.stx_mtime.tv_sec;
+ buf->st_ctime = statx.stx_ctime.tv_sec;
+ return ret;
+}
+#else
+#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined, cannot implement sys_stat()
+#endif
static __attribute__((unused))
int stat(const char *path, struct stat *buf)
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index fbbc0e68c001..9b244af1ec2c 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -193,6 +193,52 @@ struct stat {
time_t st_ctime; /* time of last status change */
};
+/* for statx() */
+#ifndef STATX_BASIC_STATS
+#define STATX_BASIC_STATS (0x000007ffU)
+#endif
+
+struct statx_timestamp {
+ __s64 tv_sec;
+ __u32 tv_nsec;
+ __s32 __reserved;
+};
+
+struct statx {
+ /* 0x00 */
+ __u32 stx_mask; /* What results were written [uncond] */
+ __u32 stx_blksize; /* Preferred general I/O size [uncond] */
+ __u64 stx_attributes; /* Flags conveying information about the file [uncond] */
+ /* 0x10 */
+ __u32 stx_nlink; /* Number of hard links */
+ __u32 stx_uid; /* User ID of owner */
+ __u32 stx_gid; /* Group ID of owner */
+ __u16 stx_mode; /* File mode */
+ __u16 __spare0[1];
+ /* 0x20 */
+ __u64 stx_ino; /* Inode number */
+ __u64 stx_size; /* File size */
+ __u64 stx_blocks; /* Number of 512-byte blocks allocated */
+ __u64 stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
+ /* 0x40 */
+ struct statx_timestamp stx_atime; /* Last access time */
+ struct statx_timestamp stx_btime; /* File creation time */
+ struct statx_timestamp stx_ctime; /* Last attribute change time */
+ struct statx_timestamp stx_mtime; /* Last data modification time */
+ /* 0x80 */
+ __u32 stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
+ __u32 stx_rdev_minor;
+ __u32 stx_dev_major; /* ID of device containing file [uncond] */
+ __u32 stx_dev_minor;
+ /* 0x90 */
+ __u64 stx_mnt_id;
+ __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
+ __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
+ /* 0xa0 */
+ __u64 __spare3[12]; /* Spare space for future expansion */
+ /* 0x100 */
+};
+
/* WARNING, it only deals with the 4096 first majors and 256 first minors */
#define makedev(major, minor) ((dev_t)((((major) & 0xfff) << 8) | ((minor) & 0xff)))
#define major(dev) ((unsigned int)(((dev) >> 8) & 0xfff))
--
2.39.0On Tue, Feb 7, 2023, at 03:09, chris.chenfeiyang@gmail.com wrote:
> From: Feiyang Chen <chenfeiyang@loongson.cn>
>
> Neither __NR_newfstatat nor __NR_stat is defined on new architecture
> like LoongArch, but we can use statx() to implement sys_stat().
>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
This looks nice, it should also help on 32-bit architectures that
only have stat64 but not newfstatat or stat.
> +#if defined(__NR_newfstatat) || defined(__NR_stat)
> static __attribute__((unused))
> int sys_stat(const char *path, struct stat *buf)
...
> +#else
> +#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined,
> cannot implement sys_stat()
> +#endif
Given that all architectures implement statx the same way, I wonder
if we can't just kill off the old function here and always use statx.
That would also allow removing the architecture specific
sys_stat_struct definitions in all arch-*.h files.
> +struct statx_timestamp {
> + __s64 tv_sec;
> + __u32 tv_nsec;
> + __s32 __reserved;
> +};
> +
> +struct statx {
> + /* 0x00 */
> + __u32 stx_mask; /* What results were written [uncond] */
> + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> + __u64 stx_attributes; /* Flags conveying information about the file
> [uncond] */
> + /* 0x10 */
> + __u32 stx_nlink; /* Number of hard links */
> + __u32 stx_uid; /* User ID of owner */
> + __u32 stx_gid; /* Group ID of owner */
> + __u16 stx_mode; /* File mode */
> + __u16 __spare0[1];
> + /* 0x20 */
> + __u64 stx_ino; /* Inode number */
> + __u64 stx_size; /* File size */
> + __u64 stx_blocks; /* Number of 512-byte blocks allocated */
> + __u64 stx_attributes_mask; /* Mask to show what's supported in
> stx_attributes */
> + /* 0x40 */
> + struct statx_timestamp stx_atime; /* Last access time */
> + struct statx_timestamp stx_btime; /* File creation time */
> + struct statx_timestamp stx_ctime; /* Last attribute change time */
> + struct statx_timestamp stx_mtime; /* Last data modification time */
> + /* 0x80 */
> + __u32 stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
> + __u32 stx_rdev_minor;
> + __u32 stx_dev_major; /* ID of device containing file [uncond] */
> + __u32 stx_dev_minor;
> + /* 0x90 */
> + __u64 stx_mnt_id;
> + __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
> + __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
> + /* 0xa0 */
> + __u64 __spare3[12]; /* Spare space for future expansion */
> + /* 0x100 */
> +};
Can't we just #include <linux/stat.h> here to avoid having to maintain
a duplicate copy?
Arnd
On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 7, 2023, at 03:09, chris.chenfeiyang@gmail.com wrote:
> > From: Feiyang Chen <chenfeiyang@loongson.cn>
> >
> > Neither __NR_newfstatat nor __NR_stat is defined on new architecture
> > like LoongArch, but we can use statx() to implement sys_stat().
> >
> > Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
>
> This looks nice, it should also help on 32-bit architectures that
> only have stat64 but not newfstatat or stat.
>
> > +#if defined(__NR_newfstatat) || defined(__NR_stat)
> > static __attribute__((unused))
> > int sys_stat(const char *path, struct stat *buf)
> ...
> > +#else
> > +#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined,
> > cannot implement sys_stat()
> > +#endif
>
> Given that all architectures implement statx the same way, I wonder
> if we can't just kill off the old function here and always use statx.
>
> That would also allow removing the architecture specific
> sys_stat_struct definitions in all arch-*.h files.
>
Hi, Arnd,
I'd really like to make all architectures use sys_statx() instead
of sys_stat(). I just fear we might get dragged into a long discussion.
Can I send a patch series to do this later?
> > +struct statx_timestamp {
> > + __s64 tv_sec;
> > + __u32 tv_nsec;
> > + __s32 __reserved;
> > +};
> > +
> > +struct statx {
> > + /* 0x00 */
> > + __u32 stx_mask; /* What results were written [uncond] */
> > + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> > + __u64 stx_attributes; /* Flags conveying information about the file
> > [uncond] */
> > + /* 0x10 */
> > + __u32 stx_nlink; /* Number of hard links */
> > + __u32 stx_uid; /* User ID of owner */
> > + __u32 stx_gid; /* Group ID of owner */
> > + __u16 stx_mode; /* File mode */
> > + __u16 __spare0[1];
> > + /* 0x20 */
> > + __u64 stx_ino; /* Inode number */
> > + __u64 stx_size; /* File size */
> > + __u64 stx_blocks; /* Number of 512-byte blocks allocated */
> > + __u64 stx_attributes_mask; /* Mask to show what's supported in
> > stx_attributes */
> > + /* 0x40 */
> > + struct statx_timestamp stx_atime; /* Last access time */
> > + struct statx_timestamp stx_btime; /* File creation time */
> > + struct statx_timestamp stx_ctime; /* Last attribute change time */
> > + struct statx_timestamp stx_mtime; /* Last data modification time */
> > + /* 0x80 */
> > + __u32 stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
> > + __u32 stx_rdev_minor;
> > + __u32 stx_dev_major; /* ID of device containing file [uncond] */
> > + __u32 stx_dev_minor;
> > + /* 0x90 */
> > + __u64 stx_mnt_id;
> > + __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
> > + __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
> > + /* 0xa0 */
> > + __u64 __spare3[12]; /* Spare space for future expansion */
> > + /* 0x100 */
> > +};
>
> Can't we just #include <linux/stat.h> here to avoid having to maintain
> a duplicate copy?
>
Yes, I will #include <linux/stat.h> here.
Thanks,
Feiyang
> Arnd
Hi Feiyang,
Sorry for the delay.
On Wed, Feb 08, 2023 at 10:09:48AM +0800, Feiyang Chen wrote:
> On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <arnd@arndb.de> wrote:
(...)
> > Given that all architectures implement statx the same way, I wonder
> > if we can't just kill off the old function here and always use statx.
> >
> > That would also allow removing the architecture specific
> > sys_stat_struct definitions in all arch-*.h files.
> >
>
> Hi, Arnd,
>
> I'd really like to make all architectures use sys_statx() instead
> of sys_stat(). I just fear we might get dragged into a long discussion.
> Can I send a patch series to do this later?
I generally agree with the Arnd's points overall and I'm fine with the
rest of your series. On this specific point, I'm fine with your proposal,
let's just start with sys_statx() only on this arch, please add a comment
about this possibility in the commit message that brings statx(),
indicating that other archs are likely to benefit from it as well, and
let's see after this if we can migrate all archs to statx.
I'm having another comment below however:
> > > +struct statx_timestamp {
> > > + __s64 tv_sec;
> > > + __u32 tv_nsec;
> > > + __s32 __reserved;
> > > +};
> > > +
> > > +struct statx {
> > > + /* 0x00 */
> > > + __u32 stx_mask; /* What results were written [uncond] */
> > > + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> > > + __u64 stx_attributes; /* Flags conveying information about the file
(...)
For all these types exposed to userland that you have to define, I'd
prefer if we would avoid using kernel-inherited types like __u32, __u64
etc given that all other archs for now only use regular types. It's not
critical at all but I'd prefer that we stay consistent between all archs.
Also, based on the comments on the fields it seems to me that this file
was just copy-pasted from some uapi header which is not under the same
license, so that's another reason for just defining what is needed here
if you need to define it here. And of course, if including linux/stat.h
also works, that's by far the preferred solution which will also save
us from having to maintain a copy!
Thanks!
Willy
On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <w@1wt.eu> wrote:
>
> Hi Feiyang,
>
> Sorry for the delay.
>
> On Wed, Feb 08, 2023 at 10:09:48AM +0800, Feiyang Chen wrote:
> > On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <arnd@arndb.de> wrote:
> (...)
> > > Given that all architectures implement statx the same way, I wonder
> > > if we can't just kill off the old function here and always use statx.
> > >
> > > That would also allow removing the architecture specific
> > > sys_stat_struct definitions in all arch-*.h files.
> > >
> >
> > Hi, Arnd,
> >
> > I'd really like to make all architectures use sys_statx() instead
> > of sys_stat(). I just fear we might get dragged into a long discussion.
> > Can I send a patch series to do this later?
>
> I generally agree with the Arnd's points overall and I'm fine with the
> rest of your series. On this specific point, I'm fine with your proposal,
> let's just start with sys_statx() only on this arch, please add a comment
> about this possibility in the commit message that brings statx(),
> indicating that other archs are likely to benefit from it as well, and
> let's see after this if we can migrate all archs to statx.
>
Hi, Arnd, Willy,
We have a problem if we just start with sys_statx() only on this arch.
When struct stat is not defined, what should we do with stat() in the
nolibc selftest?
> I'm having another comment below however:
>
> > > > +struct statx_timestamp {
> > > > + __s64 tv_sec;
> > > > + __u32 tv_nsec;
> > > > + __s32 __reserved;
> > > > +};
> > > > +
> > > > +struct statx {
> > > > + /* 0x00 */
> > > > + __u32 stx_mask; /* What results were written [uncond] */
> > > > + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> > > > + __u64 stx_attributes; /* Flags conveying information about the file
> (...)
>
> For all these types exposed to userland that you have to define, I'd
> prefer if we would avoid using kernel-inherited types like __u32, __u64
> etc given that all other archs for now only use regular types. It's not
> critical at all but I'd prefer that we stay consistent between all archs.
> Also, based on the comments on the fields it seems to me that this file
> was just copy-pasted from some uapi header which is not under the same
> license, so that's another reason for just defining what is needed here
> if you need to define it here. And of course, if including linux/stat.h
> also works, that's by far the preferred solution which will also save
> us from having to maintain a copy!
>
I try to include linux/stat.h and it works.
Thanks,
Feiyang
> Thanks!
> Willy
On Wed, Feb 08, 2023 at 03:42:56PM +0800, Feiyang Chen wrote: > Hi, Arnd, Willy, > > We have a problem if we just start with sys_statx() only on this arch. > When struct stat is not defined, what should we do with stat() in the > nolibc selftest? You need to have a struct stat regardless of the syscalls you implement, because it is what userland will see and use. Even if your implementation of stat() is based on statx(), you'll need that struct stat that your stat() function will feed. Right now it is defined in types.h. As you can see in sys.h where sys_stat() is implemented, each arch's specific syscall is used with a local sys_stat_struct that is specific to that syscall, then the fields are stuffed into the userland-compatible stat struct. aarch64 is a good example, as its stat() implementation relies on newfstatat() that uses a different one, but all fields are properly mapped. Just let me know if anything is not clear and if you need more help. Thanks! Willy
On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <w@1wt.eu> wrote:
>>
>> I generally agree with the Arnd's points overall and I'm fine with the
>> rest of your series. On this specific point, I'm fine with your proposal,
>> let's just start with sys_statx() only on this arch, please add a comment
>> about this possibility in the commit message that brings statx(),
>> indicating that other archs are likely to benefit from it as well, and
>> let's see after this if we can migrate all archs to statx.
>>
>
> We have a problem if we just start with sys_statx() only on this arch.
> When struct stat is not defined, what should we do with stat() in the
> nolibc selftest?
To clarify: your proposed implementation of the stat() function that
fills the nolibc 'struct stat' based on information from 'struct statx'
is fine here. Just remove the 'struct sys_stat_struct' definition
loongarch but keep 'struct stat'.
Arnd
On Wed, Feb 08, 2023 at 09:06:24AM +0100, Arnd Bergmann wrote:
> On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> > On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <w@1wt.eu> wrote:
> >>
> >> I generally agree with the Arnd's points overall and I'm fine with the
> >> rest of your series. On this specific point, I'm fine with your proposal,
> >> let's just start with sys_statx() only on this arch, please add a comment
> >> about this possibility in the commit message that brings statx(),
> >> indicating that other archs are likely to benefit from it as well, and
> >> let's see after this if we can migrate all archs to statx.
> >>
> >
> > We have a problem if we just start with sys_statx() only on this arch.
> > When struct stat is not defined, what should we do with stat() in the
> > nolibc selftest?
>
> To clarify: your proposed implementation of the stat() function that
> fills the nolibc 'struct stat' based on information from 'struct statx'
> is fine here. Just remove the 'struct sys_stat_struct' definition
> loongarch but keep 'struct stat'.
Ah I think now I understand the problem Feiyang is facing. Since "struct
stat" is just between libc and userland, there's the "sys_stat_struct"
that we're using to abstract the syscalls in sys_stat() and that is
compatible with each variant of the stat() syscall on each arch. Here
there's simply no stat() syscall so it's best not to try to abstract
this function at all since types will not match between stat and statx,
so it will be better to just implement it like this:
#if defined(__NR_statx)
static __attribute__((unused))
int sys_stat(const char *path, struct stat *buf)
{
struct statx statx;
long ret;
ret = statx(...);
buf->xxx = statx.xxx;
...
return ret;
}
#else ...
// keep the regular sys_stat() here
#endif
Also looking at the man page I see that statx() only appeared in 4.11,
and here we're targetting userland so I'd rather keep a bit of margin
when it comes to backwards compatibility, thus not dropping stat() and
friends too early when not necessary. However using statx() by default
when available sounds fine to me!
Cheers,
Willy
On Wed, 8 Feb 2023 at 16:19, Willy Tarreau <w@1wt.eu> wrote:
>
> On Wed, Feb 08, 2023 at 09:06:24AM +0100, Arnd Bergmann wrote:
> > On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> > > On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <w@1wt.eu> wrote:
> > >>
> > >> I generally agree with the Arnd's points overall and I'm fine with the
> > >> rest of your series. On this specific point, I'm fine with your proposal,
> > >> let's just start with sys_statx() only on this arch, please add a comment
> > >> about this possibility in the commit message that brings statx(),
> > >> indicating that other archs are likely to benefit from it as well, and
> > >> let's see after this if we can migrate all archs to statx.
> > >>
> > >
> > > We have a problem if we just start with sys_statx() only on this arch.
> > > When struct stat is not defined, what should we do with stat() in the
> > > nolibc selftest?
> >
> > To clarify: your proposed implementation of the stat() function that
> > fills the nolibc 'struct stat' based on information from 'struct statx'
> > is fine here. Just remove the 'struct sys_stat_struct' definition
> > loongarch but keep 'struct stat'.
>
> Ah I think now I understand the problem Feiyang is facing. Since "struct
> stat" is just between libc and userland, there's the "sys_stat_struct"
> that we're using to abstract the syscalls in sys_stat() and that is
> compatible with each variant of the stat() syscall on each arch. Here
> there's simply no stat() syscall so it's best not to try to abstract
> this function at all since types will not match between stat and statx,
> so it will be better to just implement it like this:
>
> #if defined(__NR_statx)
> static __attribute__((unused))
> int sys_stat(const char *path, struct stat *buf)
> {
> struct statx statx;
> long ret;
>
> ret = statx(...);
> buf->xxx = statx.xxx;
> ...
> return ret;
> }
> #else ...
> // keep the regular sys_stat() here
> #endif
>
> Also looking at the man page I see that statx() only appeared in 4.11,
> and here we're targetting userland so I'd rather keep a bit of margin
> when it comes to backwards compatibility, thus not dropping stat() and
> friends too early when not necessary. However using statx() by default
> when available sounds fine to me!
>
Hi, Arnd, Willy,
I think I get it now, thank you!
Thanks,
Feiyang
> Cheers,
> Willy
On Wed, Feb 8, 2023, at 04:31, Willy Tarreau wrote: > On Wed, Feb 08, 2023 at 10:09:48AM +0800, Feiyang Chen wrote: >> On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <arnd@arndb.de> wrote: > (...) >> > Given that all architectures implement statx the same way, I wonder >> > if we can't just kill off the old function here and always use statx. >> > >> > That would also allow removing the architecture specific >> > sys_stat_struct definitions in all arch-*.h files. >> > >> >> Hi, Arnd, >> >> I'd really like to make all architectures use sys_statx() instead >> of sys_stat(). I just fear we might get dragged into a long discussion. >> Can I send a patch series to do this later? > > I generally agree with the Arnd's points overall and I'm fine with the > rest of your series. On this specific point, I'm fine with your proposal, > let's just start with sys_statx() only on this arch, please add a comment > about this possibility in the commit message that brings statx(), > indicating that other archs are likely to benefit from it as well, and > let's see after this if we can migrate all archs to statx. Ok, makes sense. Just change the description then to note that this will also fix riscv32 (which has only statx) as well as arc, hexagon, nios2 and openrisc (which have statx and stat64 but not stat or newstat). Arnd
© 2016 - 2025 Red Hat, Inc.