From: Zhangjin Wu <falcon@tinylab.org> To: arnd@arndb.de Cc: falcon@tinylab.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org, thomas@t-8ch.de, w@1wt.eu Subject: Re: [PATCH v2 07/13] tools/nolibc: sys_lseek: add pure 64bit lseek Date: Tue, 30 May 2023 21:54:33 +0800 [thread overview] Message-ID: <20230530135433.405051-1-falcon@tinylab.org> (raw) In-Reply-To: <5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com> Hi, Arnd, Willy > On Mon, May 29, 2023, at 21:54, Zhangjin Wu wrote: > > use sys_llseek instead of sys_lseek to add 64bit seek even in 32bit > > platforms. > > > > This code is based on sysdeps/unix/sysv/linux/lseek.c of glibc and > > src/unistd/lseek.c of musl. > > > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org> > > Signed-off-by: Willy Tarreau <w@1wt.eu> > > --- > > tools/include/nolibc/sys.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h > > index 98cfa2f6d021..d0720af84b6d 100644 > > --- a/tools/include/nolibc/sys.h > > +++ b/tools/include/nolibc/sys.h > > @@ -672,7 +672,17 @@ int link(const char *old, const char *new) > > static __attribute__((unused)) > > off_t sys_lseek(int fd, off_t offset, int whence) > > { > > +#if defined(__NR_llseek) || defined(__NR__llseek) > > +#ifndef __NR__llseek > > +#define __NR__llseek __NR_llseek > > +#endif > > + off_t result; > > + return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, > > whence) ?: result; > > +#elif defined(__NR_lseek) > > return my_syscall3(__NR_lseek, fd, offset, whence); > > +#else > > +#error None of __NR_lseek, __NR_llseek nor __NR__llseek defined, > > cannot implement sys_lseek() > > +#endif > > } > > This is not technically wrong, but I think a different approach > would be clearer: Instead of having a sys_lseek() that works > differently depending on the macros, why not define the low-level > helpers to match the kernel arguments like > > static inline __attribute__((unused)) > __kernel_loff_t sys_lseek(int fd, __kernel_loff_t offset, int whence) > { > #ifdef __NR__llseek > __kernel_loff_t result; > return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, whence) ?: result; > #else > > #endif > } > > static inline __attribute__((unused)) > __kernel_off_t sys_lseek(int fd, __kernel_off_t offset, int whence) > { > #ifdef __NR_lseek > return my_syscall3(__NR_lseek, fd, offset, whence); > #else > return -ENOSYS; > #endif > } > > And then do the selection inside of the actual lseek, > something like > > static __attribute__((unused)) > off_t lseek(int fd, off_t offset, int whence) > { > off_t ret = -ENOSYS; > > if (BITS_PER_LONG == 32) > ret = sys_llseek(fd, offset, whence); > > if (ret == -ENOSYS) > ret = sys_lseek(fd, offset, whence); > > if (ret < 0) { > SET_ERRNO(-ret); > ret = -1; > } > return ret; > > } Yes, It is clearer, thanks. will learn carefully about the kernel types. > > For the loff_t selection, there is no real need to handle the > fallback, so this could just be an if()/else to select 32-bit > or 64-bit, but for the time_t ones the fallback is required > for pre-5.6 kernels. > Ok, will test it on the pre-5.6 versions too. Hi, Willy, what's your suggestion about the oldest kernel versions we plan to support? ;-) Best regards, Zhangjin > Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Zhangjin Wu <falcon@tinylab.org> To: arnd@arndb.de Cc: falcon@tinylab.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org, thomas@t-8ch.de, w@1wt.eu Subject: Re: [PATCH v2 07/13] tools/nolibc: sys_lseek: add pure 64bit lseek Date: Tue, 30 May 2023 21:54:33 +0800 [thread overview] Message-ID: <20230530135433.405051-1-falcon@tinylab.org> (raw) In-Reply-To: <5e7d2adf-e96f-41ca-a4c6-5c87a25d4c9c@app.fastmail.com> Hi, Arnd, Willy > On Mon, May 29, 2023, at 21:54, Zhangjin Wu wrote: > > use sys_llseek instead of sys_lseek to add 64bit seek even in 32bit > > platforms. > > > > This code is based on sysdeps/unix/sysv/linux/lseek.c of glibc and > > src/unistd/lseek.c of musl. > > > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org> > > Signed-off-by: Willy Tarreau <w@1wt.eu> > > --- > > tools/include/nolibc/sys.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h > > index 98cfa2f6d021..d0720af84b6d 100644 > > --- a/tools/include/nolibc/sys.h > > +++ b/tools/include/nolibc/sys.h > > @@ -672,7 +672,17 @@ int link(const char *old, const char *new) > > static __attribute__((unused)) > > off_t sys_lseek(int fd, off_t offset, int whence) > > { > > +#if defined(__NR_llseek) || defined(__NR__llseek) > > +#ifndef __NR__llseek > > +#define __NR__llseek __NR_llseek > > +#endif > > + off_t result; > > + return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, > > whence) ?: result; > > +#elif defined(__NR_lseek) > > return my_syscall3(__NR_lseek, fd, offset, whence); > > +#else > > +#error None of __NR_lseek, __NR_llseek nor __NR__llseek defined, > > cannot implement sys_lseek() > > +#endif > > } > > This is not technically wrong, but I think a different approach > would be clearer: Instead of having a sys_lseek() that works > differently depending on the macros, why not define the low-level > helpers to match the kernel arguments like > > static inline __attribute__((unused)) > __kernel_loff_t sys_lseek(int fd, __kernel_loff_t offset, int whence) > { > #ifdef __NR__llseek > __kernel_loff_t result; > return my_syscall5(__NR__llseek, fd, offset >> 32, offset, &result, whence) ?: result; > #else > > #endif > } > > static inline __attribute__((unused)) > __kernel_off_t sys_lseek(int fd, __kernel_off_t offset, int whence) > { > #ifdef __NR_lseek > return my_syscall3(__NR_lseek, fd, offset, whence); > #else > return -ENOSYS; > #endif > } > > And then do the selection inside of the actual lseek, > something like > > static __attribute__((unused)) > off_t lseek(int fd, off_t offset, int whence) > { > off_t ret = -ENOSYS; > > if (BITS_PER_LONG == 32) > ret = sys_llseek(fd, offset, whence); > > if (ret == -ENOSYS) > ret = sys_lseek(fd, offset, whence); > > if (ret < 0) { > SET_ERRNO(-ret); > ret = -1; > } > return ret; > > } Yes, It is clearer, thanks. will learn carefully about the kernel types. > > For the loff_t selection, there is no real need to handle the > fallback, so this could just be an if()/else to select 32-bit > or 64-bit, but for the time_t ones the fallback is required > for pre-5.6 kernels. > Ok, will test it on the pre-5.6 versions too. Hi, Willy, what's your suggestion about the oldest kernel versions we plan to support? ;-) 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-30 13:54 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-29 19:45 [PATCH v2 00/13] nolibc: add part2 of support for rv32 Zhangjin Wu 2023-05-29 19:45 ` Zhangjin Wu 2023-05-29 19:46 ` [PATCH v2 01/13] selftests/nolibc: remove gettimeofday_bad1/2 completely Zhangjin Wu 2023-05-29 19:46 ` Zhangjin Wu 2023-05-29 19:47 ` [PATCH v2 02/13] selftests/nolibc: support two errnos with EXPECT_SYSER2() Zhangjin Wu 2023-05-29 19:47 ` Zhangjin Wu 2023-05-29 19:49 ` [PATCH v2 03/13] selftests/nolibc: waitpid_min: add waitid syscall support Zhangjin Wu 2023-05-29 19:49 ` Zhangjin Wu 2023-05-29 19:50 ` [PATCH v2 04/13] tools/nolibc: add missing nanoseconds support for __NR_statx Zhangjin Wu 2023-05-29 19:50 ` Zhangjin Wu 2023-05-29 21:39 ` Thomas Weißschuh 2023-05-29 21:39 ` Thomas Weißschuh 2023-05-30 5:21 ` Zhangjin Wu 2023-05-30 5:21 ` Zhangjin Wu 2023-05-29 19:51 ` [PATCH v2 05/13] tools/nolibc: add more wait status related types Zhangjin Wu 2023-05-29 19:51 ` Zhangjin Wu 2023-05-29 19:53 ` [PATCH v2 06/13] tools/nolibc: add pure 64bit off_t, time_t and blkcnt_t Zhangjin Wu 2023-05-29 19:53 ` Zhangjin Wu 2023-05-29 19:54 ` [PATCH v2 07/13] tools/nolibc: sys_lseek: add pure 64bit lseek Zhangjin Wu 2023-05-29 19:54 ` Zhangjin Wu 2023-05-30 8:10 ` Arnd Bergmann 2023-05-30 8:10 ` Arnd Bergmann 2023-05-30 13:54 ` Zhangjin Wu [this message] 2023-05-30 13:54 ` Zhangjin Wu 2023-07-02 16:28 ` Willy Tarreau 2023-07-02 16:28 ` Willy Tarreau 2023-05-29 19:56 ` [PATCH v2 08/13] tools/nolibc: add pure 64bit time structs Zhangjin Wu 2023-05-29 19:56 ` Zhangjin Wu 2023-05-29 19:57 ` [PATCH v2 09/13] tools/nolibc: sys_select: add pure 64bit select Zhangjin Wu 2023-05-29 19:57 ` Zhangjin Wu 2023-05-29 19:58 ` [PATCH v2 10/13] tools/nolibc: sys_poll: add pure 64bit poll Zhangjin Wu 2023-05-29 19:58 ` Zhangjin Wu 2023-05-29 19:59 ` [PATCH v2 11/13] tools/nolibc: sys_gettimeofday: add pure 64bit gettimeofday Zhangjin Wu 2023-05-29 19:59 ` Zhangjin Wu 2023-05-29 20:01 ` [PATCH v2 12/13] tools/nolibc: sys_wait4: add waitid syscall support Zhangjin Wu 2023-05-29 20:01 ` Zhangjin Wu 2023-05-29 20:03 ` [PATCH v2 13/13] selftests/nolibc: riscv: customize makefile for rv32 Zhangjin Wu 2023-05-29 20:03 ` Zhangjin Wu 2023-06-02 4:06 ` Zhangjin Wu 2023-06-02 4:06 ` Zhangjin Wu 2023-06-02 10:33 ` Thomas Weißschuh 2023-06-02 10:33 ` Thomas Weißschuh 2023-06-02 11:56 ` Zhangjin Wu 2023-06-02 11:56 ` Zhangjin Wu 2023-05-30 6:33 ` [PATCH v2 0/2] nolibc: add part3 of support " Zhangjin Wu 2023-05-30 6:33 ` Zhangjin Wu 2023-05-30 6:37 ` [PATCH 1/2] selftests/nolibc: add new gettimeofday test cases Zhangjin Wu 2023-05-30 6:37 ` Zhangjin Wu 2023-05-30 10:59 ` Thomas Weißschuh 2023-05-30 10:59 ` Thomas Weißschuh 2023-05-30 11:28 ` Zhangjin Wu 2023-05-30 11:28 ` Zhangjin Wu 2023-05-30 11:54 ` Thomas Weißschuh 2023-05-30 11:54 ` Thomas Weißschuh 2023-05-30 12:05 ` Willy Tarreau 2023-05-30 12:05 ` Willy Tarreau 2023-05-30 12:31 ` Andreas Schwab 2023-05-30 12:31 ` Andreas Schwab 2023-05-30 12:35 ` Thomas Weißschuh 2023-05-30 12:35 ` Thomas Weißschuh 2023-05-30 6:42 ` [PATCH 2/2] selftests/nolibc: add sizeof test for the new 64bit data types Zhangjin Wu 2023-05-30 6:42 ` Zhangjin Wu 2023-05-30 9:18 ` Thomas Weißschuh 2023-05-30 9:18 ` Thomas Weißschuh 2023-05-30 11:17 ` Zhangjin Wu 2023-05-30 11:17 ` Zhangjin Wu 2023-06-02 19:44 ` [PATCH v2 00/13] nolibc: add part2 of support for rv32 Willy Tarreau 2023-06-02 19:44 ` Willy Tarreau
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=20230530135433.405051-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=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.