From: Zhangjin Wu <falcon@tinylab.org> To: arnd@arndb.de, thomas@t-8ch.de, w@1wt.eu Cc: falcon@tinylab.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org, palmer@dabbelt.com, paul.walmsley@sifive.com Subject: Re: [PATCH 09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64 for rv32 Date: Sun, 28 May 2023 16:25:09 +0800 [thread overview] Message-ID: <20230528082509.293250-1-falcon@tinylab.org> (raw) In-Reply-To: <83ab9f47-e1ed-463c-a717-26aad6bf2b71@app.fastmail.com> Hi, Arnd, Thomas, Willy > On Fri, May 26, 2023, at 09:15, Thomas Wei=C3=9Fschuh wrote: > > On 2023-05-25 01:57:24+0800, Zhangjin Wu wrote: > >> > >> +/* needed by time64 syscalls */ > >> +struct timespec64 { > >> + time64_t tv_sec; /* seconds */ > >> + long tv_nsec; /* nanoseconds */ > >> +}; > > > > A question to you and Willy, as it's also done the same for other types: > > > > What is the advantage of custom definitions over using the one from the > > kernel (maybe via a typedef). > > > > From linux/time_types.h: > > > > struct __kernel_timespec { > > __kernel_time64_t tv_set; > > long long tv_nsec; > > }; > > I agree the __kernel_* types are what we should be using when > interacting with system calls directly, that is definitely what > they are intended for. > > I would go further here and completely drop support for 32-bit > time_t/off_t and derived types in nolibc. Unfortunately, the > kernel's include/uapi/linux/time.h header still defines the > old types, this is one of the last remnants the time32 syscalls > definitions in the kernel headers, and this already conflicts > with the glibc and musl definitions, so anything that includes > this header is broken on real systems. I think it makes most > sense for nolibc to just use the linux/time_types.h header > instead and use something like > > #define timespec __kernel_timespec > #define itimerspec __kernel_itimerspec > typedef __kernel_time64_t time_t; > /* timeval is only provided for users, not compatible with syscalls */ > struct timeval { __kernel_time64_t tv_sec; __s64 tv_nsec; }; > > so we can drop all the fallbacks for old 32-bit targets. This > also allows running with CONFIG_COMPAT_32BIT_TIME disabled. Just a status update ... I'm working on the pure time64 and 64bit off_t syscalls support, it almost worked (tested on rv32/64, arm32/64), thanks very much for your suggestions. It includes: * Based on linux/types.h and * Use 64bit off_t * Use 64bit time_t * the new std.h looks like this typedef uint32_t __kernel_dev_t; typedef __kernel_dev_t dev_t; typedef __kernel_ulong_t ino_t; typedef __kernel_mode_t mode_t; typedef __kernel_pid_t pid_t; typedef __kernel_uid32_t uid_t; typedef __kernel_gid32_t gid_t; typedef __kernel_loff_t off_t; typedef __kernel_time64_t time_t; typedef uint32_t nlink_t; typedef uint64_t blksize_t; typedef uint64_t blkcnt_t; * Use __kernel_timespec as timespec * Use 64bit time_t based struct timeval * Disable gettimeofday syscall completely for 32bit platforms * And disable the gettimeofday_bad1/2 test case too * Remove the oldselect and newslect path completely * The new types.h looks this: /* always use time64 structs in user-space even on 32bit platforms */ #define timespec __kernel_timespec #define itimerspec __kernel_itimerspec /* timeval is only provided for users, not compatible with syscalls */ struct __timeval64 { __kernel_time64_t tv_sec; /* seconds */ __s64 tv_usec; /* microseconds */ }; /* override the 32bit version of struct timeval in linux/time.h */ #define timeval __timeval64 /* itimerval is only provided for users, not compatible with syscalls */ struct __itimerval64 { struct __timeval64 it_interval; /* timer interval */ struct __timeval64 it_value; /* current value */ }; /* override the 32bit version of struct itimerval in linux/time.h */ #define itimerval __itimerval64 * Use __NR_*time64 for all 32bit platforms * Use __NR_pselect6/ppoll/clock_gettime only for 64bit platforms * New sizeof tests added to verify off_t, time_t, timespec, itimerspec... CASE_TEST(sizeof_time_t); EXPECT_EQ(1, 8, sizeof(time_t)); break; CASE_TEST(sizeof_timespec); EXPECT_EQ(1, 16, sizeof(struct timespec)); break; #ifdef NOLIBC CASE_TEST(sizeof_itimerspec); EXPECT_EQ(1, 32, sizeof(struct itimerspec)); break; #endif CASE_TEST(sizeof_timeval); EXPECT_EQ(1, 16, sizeof(struct timeval)); break; CASE_TEST(sizeof_itimerval); EXPECT_EQ(1, 32, sizeof(struct itimerval)); break; CASE_TEST(sizeof_off_t); EXPECT_EQ(1, 8, sizeof(off_t)); break; @Arnd, the above timeval/itimerval definitions are used to override the ones from linux/time.h to avoid such error: error: redefinition of ‘struct timeval’ nolibc/sysroot/riscv/include/types.h:225:8: error: redefinition of ‘struct timeval’ 225 | struct timeval { | ^~~~~~~ In file included from nolibc/sysroot/riscv/include/types.h:11, from nolibc/sysroot/riscv/include/nolibc.h:98, from nolibc/sysroot/riscv/include/errno.h:26, from nolibc/sysroot/riscv/include/stdio.h:14, from tools/testing/selftests/nolibc/nolibc-test.c:12: nolibc/sysroot/riscv/include/linux/time.h:16:8: note: originally defined here 16 | struct timeval { @Arnd, As you commented in another reply, is it time for us to update include/uapi/linux/time.h together and let it provide time64 timeval/itimerval instead of the old ones? perhaps some libc's are still using them. Or perhaps we can add a switch like __ARCH_WANT_TIME32_SYSCALLS, add a __ARCH_WANT_TIME32_STRUCTS and simply bind it with __ARCH_WANT_TIME32_SYSCALLS? About the above ugly override code, What's your suggestion in v2? ;-) Best regards, Zhangjin > > Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Zhangjin Wu <falcon@tinylab.org> To: arnd@arndb.de, thomas@t-8ch.de, w@1wt.eu Cc: falcon@tinylab.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org, palmer@dabbelt.com, paul.walmsley@sifive.com Subject: Re: [PATCH 09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64 for rv32 Date: Sun, 28 May 2023 16:25:09 +0800 [thread overview] Message-ID: <20230528082509.293250-1-falcon@tinylab.org> (raw) In-Reply-To: <83ab9f47-e1ed-463c-a717-26aad6bf2b71@app.fastmail.com> Hi, Arnd, Thomas, Willy > On Fri, May 26, 2023, at 09:15, Thomas Wei=C3=9Fschuh wrote: > > On 2023-05-25 01:57:24+0800, Zhangjin Wu wrote: > >> > >> +/* needed by time64 syscalls */ > >> +struct timespec64 { > >> + time64_t tv_sec; /* seconds */ > >> + long tv_nsec; /* nanoseconds */ > >> +}; > > > > A question to you and Willy, as it's also done the same for other types: > > > > What is the advantage of custom definitions over using the one from the > > kernel (maybe via a typedef). > > > > From linux/time_types.h: > > > > struct __kernel_timespec { > > __kernel_time64_t tv_set; > > long long tv_nsec; > > }; > > I agree the __kernel_* types are what we should be using when > interacting with system calls directly, that is definitely what > they are intended for. > > I would go further here and completely drop support for 32-bit > time_t/off_t and derived types in nolibc. Unfortunately, the > kernel's include/uapi/linux/time.h header still defines the > old types, this is one of the last remnants the time32 syscalls > definitions in the kernel headers, and this already conflicts > with the glibc and musl definitions, so anything that includes > this header is broken on real systems. I think it makes most > sense for nolibc to just use the linux/time_types.h header > instead and use something like > > #define timespec __kernel_timespec > #define itimerspec __kernel_itimerspec > typedef __kernel_time64_t time_t; > /* timeval is only provided for users, not compatible with syscalls */ > struct timeval { __kernel_time64_t tv_sec; __s64 tv_nsec; }; > > so we can drop all the fallbacks for old 32-bit targets. This > also allows running with CONFIG_COMPAT_32BIT_TIME disabled. Just a status update ... I'm working on the pure time64 and 64bit off_t syscalls support, it almost worked (tested on rv32/64, arm32/64), thanks very much for your suggestions. It includes: * Based on linux/types.h and * Use 64bit off_t * Use 64bit time_t * the new std.h looks like this typedef uint32_t __kernel_dev_t; typedef __kernel_dev_t dev_t; typedef __kernel_ulong_t ino_t; typedef __kernel_mode_t mode_t; typedef __kernel_pid_t pid_t; typedef __kernel_uid32_t uid_t; typedef __kernel_gid32_t gid_t; typedef __kernel_loff_t off_t; typedef __kernel_time64_t time_t; typedef uint32_t nlink_t; typedef uint64_t blksize_t; typedef uint64_t blkcnt_t; * Use __kernel_timespec as timespec * Use 64bit time_t based struct timeval * Disable gettimeofday syscall completely for 32bit platforms * And disable the gettimeofday_bad1/2 test case too * Remove the oldselect and newslect path completely * The new types.h looks this: /* always use time64 structs in user-space even on 32bit platforms */ #define timespec __kernel_timespec #define itimerspec __kernel_itimerspec /* timeval is only provided for users, not compatible with syscalls */ struct __timeval64 { __kernel_time64_t tv_sec; /* seconds */ __s64 tv_usec; /* microseconds */ }; /* override the 32bit version of struct timeval in linux/time.h */ #define timeval __timeval64 /* itimerval is only provided for users, not compatible with syscalls */ struct __itimerval64 { struct __timeval64 it_interval; /* timer interval */ struct __timeval64 it_value; /* current value */ }; /* override the 32bit version of struct itimerval in linux/time.h */ #define itimerval __itimerval64 * Use __NR_*time64 for all 32bit platforms * Use __NR_pselect6/ppoll/clock_gettime only for 64bit platforms * New sizeof tests added to verify off_t, time_t, timespec, itimerspec... CASE_TEST(sizeof_time_t); EXPECT_EQ(1, 8, sizeof(time_t)); break; CASE_TEST(sizeof_timespec); EXPECT_EQ(1, 16, sizeof(struct timespec)); break; #ifdef NOLIBC CASE_TEST(sizeof_itimerspec); EXPECT_EQ(1, 32, sizeof(struct itimerspec)); break; #endif CASE_TEST(sizeof_timeval); EXPECT_EQ(1, 16, sizeof(struct timeval)); break; CASE_TEST(sizeof_itimerval); EXPECT_EQ(1, 32, sizeof(struct itimerval)); break; CASE_TEST(sizeof_off_t); EXPECT_EQ(1, 8, sizeof(off_t)); break; @Arnd, the above timeval/itimerval definitions are used to override the ones from linux/time.h to avoid such error: error: redefinition of ‘struct timeval’ nolibc/sysroot/riscv/include/types.h:225:8: error: redefinition of ‘struct timeval’ 225 | struct timeval { | ^~~~~~~ In file included from nolibc/sysroot/riscv/include/types.h:11, from nolibc/sysroot/riscv/include/nolibc.h:98, from nolibc/sysroot/riscv/include/errno.h:26, from nolibc/sysroot/riscv/include/stdio.h:14, from tools/testing/selftests/nolibc/nolibc-test.c:12: nolibc/sysroot/riscv/include/linux/time.h:16:8: note: originally defined here 16 | struct timeval { @Arnd, As you commented in another reply, is it time for us to update include/uapi/linux/time.h together and let it provide time64 timeval/itimerval instead of the old ones? perhaps some libc's are still using them. Or perhaps we can add a switch like __ARCH_WANT_TIME32_SYSCALLS, add a __ARCH_WANT_TIME32_STRUCTS and simply bind it with __ARCH_WANT_TIME32_SYSCALLS? About the above ugly override code, What's your suggestion in v2? ;-) Best regards, Zhangjin > > Arnd _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-05-28 8:25 UTC|newest] Thread overview: 119+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-24 17:33 [PATCH 00/13] tools/nolibc: riscv: Add full rv32 support Zhangjin Wu 2023-05-24 17:33 ` Zhangjin Wu 2023-05-24 17:41 ` [PATCH 01/13] Revert "tools/nolibc: riscv: Support __NR_llseek for rv32" Zhangjin Wu 2023-05-24 17:41 ` Zhangjin Wu 2023-05-24 17:44 ` [PATCH 02/13] Revert "selftests/nolibc: Fix up compile error " Zhangjin Wu 2023-05-24 17:44 ` Zhangjin Wu 2023-05-24 17:46 ` [PATCH 03/13] selftests/nolibc: print name instead of number for EOVERFLOW Zhangjin Wu 2023-05-24 18:28 ` Zhangjin Wu 2023-05-24 17:46 ` Zhangjin Wu 2023-05-24 20:23 ` Thomas Weißschuh 2023-05-24 20:23 ` Thomas Weißschuh 2023-05-24 17:48 ` [PATCH 04/13] selftests/nolibc: syscall_args: use __NR_statx for rv32 Zhangjin Wu 2023-05-24 17:48 ` Zhangjin Wu 2023-05-24 19:49 ` Thomas Weißschuh 2023-05-24 19:49 ` Thomas Weißschuh 2023-05-25 7:20 ` Zhangjin Wu 2023-05-25 7:20 ` Zhangjin Wu 2023-05-26 9:21 ` Arnd Bergmann 2023-05-26 9:21 ` Arnd Bergmann 2023-05-26 10:06 ` Willy Tarreau 2023-05-26 10:06 ` Willy Tarreau 2023-05-27 0:58 ` Zhangjin Wu 2023-05-27 0:58 ` Zhangjin Wu 2023-05-24 17:50 ` [PATCH 05/13] selftests/nolibc: riscv: customize makefile " Zhangjin Wu 2023-05-24 17:50 ` Zhangjin Wu 2023-05-26 6:57 ` Thomas Weißschuh 2023-05-26 6:57 ` Thomas Weißschuh 2023-05-26 9:20 ` Zhangjin Wu 2023-05-26 9:20 ` Zhangjin Wu 2023-05-24 17:52 ` [PATCH 06/13] selftests/nolibc: allow specify a bios for qemu Zhangjin Wu 2023-05-24 17:52 ` Zhangjin Wu 2023-05-26 7:00 ` Thomas Weißschuh 2023-05-26 7:00 ` Thomas Weißschuh 2023-05-26 10:25 ` Zhangjin Wu 2023-05-26 10:25 ` Zhangjin Wu 2023-05-26 10:36 ` Conor Dooley 2023-05-26 10:36 ` Conor Dooley 2023-05-26 13:38 ` Zhangjin Wu 2023-05-26 13:38 ` Zhangjin Wu 2023-05-26 15:08 ` Conor Dooley 2023-05-26 15:08 ` Conor Dooley 2023-05-28 7:52 ` Willy Tarreau 2023-05-28 7:52 ` Willy Tarreau 2023-05-24 17:54 ` [PATCH 07/13] selftests/nolibc: remove the duplicated gettimeofday_bad2 Zhangjin Wu 2023-05-24 17:54 ` Zhangjin Wu 2023-05-24 17:55 ` [PATCH 08/13] tools/nolibc: sys_lseek: riscv: use __NR_llseek for rv32 Zhangjin Wu 2023-05-24 17:55 ` Zhangjin Wu 2023-05-24 17:57 ` [PATCH 09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64 " Zhangjin Wu 2023-05-24 17:57 ` Zhangjin Wu 2023-05-26 7:15 ` Thomas Weißschuh 2023-05-26 7:15 ` Thomas Weißschuh 2023-05-26 9:34 ` Arnd Bergmann 2023-05-26 9:34 ` Arnd Bergmann 2023-05-28 8:25 ` Zhangjin Wu [this message] 2023-05-28 8:25 ` Zhangjin Wu 2023-05-28 8:48 ` Arnd Bergmann 2023-05-28 8:48 ` Arnd Bergmann 2023-05-28 10:29 ` Willy Tarreau 2023-05-28 10:29 ` Willy Tarreau 2023-05-28 10:55 ` Arnd Bergmann 2023-05-28 10:55 ` Arnd Bergmann 2023-05-28 11:03 ` Willy Tarreau 2023-05-28 11:03 ` Willy Tarreau 2023-05-24 17:58 ` [PATCH 10/13] tools/nolibc: ppoll/ppoll_time64: add a missing argument Zhangjin Wu 2023-05-24 17:58 ` Zhangjin Wu 2023-05-24 17:59 ` [PATCH 11/13] tools/nolibc: sys_select: riscv: use __NR_pselect6_time64 for rv32 Zhangjin Wu 2023-05-24 17:59 ` Zhangjin Wu 2023-05-24 20:22 ` Thomas Weißschuh 2023-05-24 20:22 ` Thomas Weißschuh 2023-05-25 7:10 ` Zhangjin Wu 2023-05-25 7:10 ` Zhangjin Wu 2023-05-25 7:22 ` Thomas Weißschuh 2023-05-25 7:22 ` Thomas Weißschuh 2023-05-26 1:50 ` Zhangjin Wu 2023-05-26 1:50 ` Zhangjin Wu 2023-05-26 9:19 ` Arnd Bergmann 2023-05-26 9:19 ` Arnd Bergmann 2023-05-26 11:00 ` [PATCH 00/13] tools/nolibc: riscv: Add full rv32 support Zhangjin Wu 2023-05-26 11:00 ` Zhangjin Wu 2023-05-26 11:13 ` Arnd Bergmann 2023-05-26 11:13 ` Arnd Bergmann 2023-05-24 18:02 ` [PATCH 12/13] tools/nolibc: sys_wait4: riscv: use __NR_waitid for rv32 Zhangjin Wu 2023-05-24 18:02 ` Zhangjin Wu 2023-05-24 18:03 ` [PATCH 13/13] tools/nolibc: sys_gettimeofday: riscv: use __NR_clock_gettime64 " Zhangjin Wu 2023-05-24 18:03 ` Zhangjin Wu 2023-05-26 7:38 ` Thomas Weißschuh 2023-05-26 7:38 ` Thomas Weißschuh 2023-05-27 1:26 ` Zhangjin Wu 2023-05-27 1:26 ` Zhangjin Wu 2023-05-27 3:39 ` Zhangjin Wu 2023-05-27 3:39 ` Zhangjin Wu 2023-05-27 5:12 ` Willy Tarreau 2023-05-27 5:12 ` Willy Tarreau 2023-05-24 18:24 ` [PATCH 00/13] tools/nolibc: riscv: Add full rv32 support Zhangjin Wu 2023-05-24 18:24 ` Zhangjin Wu 2023-05-28 7:59 ` Willy Tarreau 2023-05-28 7:59 ` Willy Tarreau 2023-05-28 8:42 ` Thomas Weißschuh 2023-05-28 8:42 ` Thomas Weißschuh 2023-05-28 9:41 ` Thomas Weißschuh 2023-05-28 9:41 ` Thomas Weißschuh 2023-05-28 10:17 ` Willy Tarreau 2023-05-28 10:17 ` Willy Tarreau 2023-05-28 10:39 ` Zhangjin Wu 2023-05-28 10:39 ` Zhangjin Wu 2023-05-28 11:33 ` Willy Tarreau 2023-05-28 11:33 ` Willy Tarreau 2023-05-28 12:52 ` Zhangjin Wu 2023-05-28 12:52 ` Zhangjin Wu 2023-05-28 13:45 ` Thomas Weißschuh 2023-05-28 13:45 ` Thomas Weißschuh 2023-05-28 18:39 ` Zhangjin Wu 2023-05-28 18:39 ` Zhangjin Wu 2023-05-29 8:45 ` Thomas Weißschuh 2023-05-29 8:45 ` Thomas Weißschuh 2023-05-29 11:31 ` Willy Tarreau 2023-05-29 11:31 ` Willy Tarreau 2023-05-30 10:06 ` Zhangjin Wu 2023-05-30 10:06 ` Zhangjin Wu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230528082509.293250-1-falcon@tinylab.org \ --to=falcon@tinylab.org \ --cc=arnd@arndb.de \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-kselftest@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=thomas@t-8ch.de \ --cc=w@1wt.eu \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.