diff mbox series

[v3,02/12] tools/nolibc: add missing nanoseconds support for __NR_statx

Message ID bcb69a382bbb68826f974ef4f521c8f1c60e47bc.1685777982.git.falcon@tinylab.org (mailing list archive)
State Handled Elsewhere
Headers show
Series nolibc: add generic part1 of prepare for rv32 | expand

Checks

Context Check Description
conchuod/tree_selection fail Failed to apply to next/pending-fixes, riscv/for-next or riscv/master

Commit Message

Zhangjin Wu June 3, 2023, 8:02 a.m. UTC
Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
added nanoseconds for stat() but missed the statx case, this adds it.

The stx_atime, stx_mtime, stx_ctime are in type of 'struct
statx_timestamp', which is incompatible with 'struct timespec', should
convert explicitly.

    /* include/uapi/linux/stat.h */

    struct statx_timestamp {
    	__s64	tv_sec;
    	__u32	tv_nsec;
    	__s32	__reserved;
    };

    /* include/uapi/linux/time.h */
    struct timespec {
    	__kernel_old_time_t	tv_sec;		/* seconds */
    	long			tv_nsec;	/* nanoseconds */
    };

Without this patch, the stat_timestamps test case would fail when
__NR_statx defined.

Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8ch.de/
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/include/nolibc/sys.h | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

Comments

Willy Tarreau June 4, 2023, 11:18 a.m. UTC | #1
On Sat, Jun 03, 2023 at 04:02:04PM +0800, Zhangjin Wu wrote:
> Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
> added nanoseconds for stat() but missed the statx case, this adds it.
> 
> The stx_atime, stx_mtime, stx_ctime are in type of 'struct
> statx_timestamp', which is incompatible with 'struct timespec', should
> convert explicitly.
> 
>     /* include/uapi/linux/stat.h */
> 
>     struct statx_timestamp {
>     	__s64	tv_sec;
>     	__u32	tv_nsec;
>     	__s32	__reserved;
>     };
> 
>     /* include/uapi/linux/time.h */
>     struct timespec {
>     	__kernel_old_time_t	tv_sec;		/* seconds */
>     	long			tv_nsec;	/* nanoseconds */
>     };
> 
> Without this patch, the stat_timestamps test case would fail when
> __NR_statx defined.
> 
> Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
> Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
> Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8ch.de/
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>

Thank you. I've queued it immediately after Thomas' patch.
I'll let the two of you tell me if it's better to squash them
together to avoid breaking bisect and mark you co-authors.

Willy
Thomas Weißschuh June 4, 2023, noon UTC | #2
On 2023-06-04 13:18:35+0200, Willy Tarreau wrote:
> On Sat, Jun 03, 2023 at 04:02:04PM +0800, Zhangjin Wu wrote:
> > Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
> > added nanoseconds for stat() but missed the statx case, this adds it.
> > 
> > The stx_atime, stx_mtime, stx_ctime are in type of 'struct
> > statx_timestamp', which is incompatible with 'struct timespec', should
> > convert explicitly.
> > 
> >     /* include/uapi/linux/stat.h */
> > 
> >     struct statx_timestamp {
> >     	__s64	tv_sec;
> >     	__u32	tv_nsec;
> >     	__s32	__reserved;
> >     };
> > 
> >     /* include/uapi/linux/time.h */
> >     struct timespec {
> >     	__kernel_old_time_t	tv_sec;		/* seconds */
> >     	long			tv_nsec;	/* nanoseconds */
> >     };
> > 
> > Without this patch, the stat_timestamps test case would fail when
> > __NR_statx defined.
> > 
> > Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
> > Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
> > Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8ch.de/
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> 
> Thank you. I've queued it immediately after Thomas' patch.
> I'll let the two of you tell me if it's better to squash them
> together to avoid breaking bisect and mark you co-authors.

Squashing them sounds like the correct solution to me.

Thomas
Willy Tarreau June 4, 2023, 12:53 p.m. UTC | #3
On Sun, Jun 04, 2023 at 02:00:02PM +0200, Thomas Weißschuh wrote:
> On 2023-06-04 13:18:35+0200, Willy Tarreau wrote:
> > On Sat, Jun 03, 2023 at 04:02:04PM +0800, Zhangjin Wu wrote:
> > > Commit a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
> > > added nanoseconds for stat() but missed the statx case, this adds it.
> > > 
> > > The stx_atime, stx_mtime, stx_ctime are in type of 'struct
> > > statx_timestamp', which is incompatible with 'struct timespec', should
> > > convert explicitly.
> > > 
> > >     /* include/uapi/linux/stat.h */
> > > 
> > >     struct statx_timestamp {
> > >     	__s64	tv_sec;
> > >     	__u32	tv_nsec;
> > >     	__s32	__reserved;
> > >     };
> > > 
> > >     /* include/uapi/linux/time.h */
> > >     struct timespec {
> > >     	__kernel_old_time_t	tv_sec;		/* seconds */
> > >     	long			tv_nsec;	/* nanoseconds */
> > >     };
> > > 
> > > Without this patch, the stat_timestamps test case would fail when
> > > __NR_statx defined.
> > > 
> > > Fixes: a89c937d781a ("tools/nolibc: support nanoseconds in stat()")
> > > Suggested-by: Thomas Weißschuh <linux@weissschuh.net>
> > > Link: https://lore.kernel.org/linux-riscv/3a3edd48-1ace-4c89-89e8-9c594dd1b3c9@t-8ch.de/
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > 
> > Thank you. I've queued it immediately after Thomas' patch.
> > I'll let the two of you tell me if it's better to squash them
> > together to avoid breaking bisect and mark you co-authors.
> 
> Squashing them sounds like the correct solution to me.

OK I've done it for now in my branch. I'm going to push it as
20230604-nolibc-rv32+stkp6. All tests pass fine again for me now on
all supported archs. I'll pass this one to Paul, I think it's fine
for 6.5. I just don't know if he still has tests planned on his side
for 6.5 (Paul always re-runs the whole tests after integration and
often spots failures).

By the way, I'm still using my test-all script that's extremely
convenient to test the expected results from user-mode (it basically
does what run-user does, but for all archs and at -O0, -Os, -O3).

I'm sharing it attached since I think it can help you and Zhangjin in
your respective tests. That's how I'm cheating to spot build issues in
contributed changes. I have not committed it because it's ugly and I
don't know where to put it, but I think you'll find it convenient
nevertheless. I'm starting it like this:

   $ ./test-all-opts.sh | tee test16.out
   $ grep passed test16.out
   136 test(s) passed, 2 skipped, 0 failed. See all results in run-arm64.out
   135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv5t_-marm.out
   135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv5t_-mthumb.out
   135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv7-a_-marm.out
   135 test(s) passed, 3 skipped, 0 failed. See all results in run-arm-march=armv7-a_-mthumb.out
   136 test(s) passed, 2 skipped, 0 failed. See all results in run-i386.out
   136 test(s) passed, 2 skipped, 0 failed. See all results in run-i386-march=i586.out
   (...)
   $ grep ' [^0] failed' test16.out || echo OK
   OK

Hoping this helps,
Willy
diff mbox series

Patch

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 1d6f33f58629..0160605444e7 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -1161,23 +1161,26 @@  int sys_stat(const char *path, struct stat *buf)
 	long ret;
 
 	ret = sys_statx(AT_FDCWD, path, AT_NO_AUTOMOUNT, 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;
+	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_atim.tv_sec  = statx.stx_atime.tv_sec;
+	buf->st_atim.tv_nsec = statx.stx_atime.tv_nsec;
+	buf->st_mtim.tv_sec  = statx.stx_mtime.tv_sec;
+	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