linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] tools/nolibc: cleanups for syscall fallbacks
@ 2023-09-14 16:01 Thomas Weißschuh
  2023-09-14 16:01 ` [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler Thomas Weißschuh
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-14 16:01 UTC (permalink / raw)
  To: Willy Tarreau, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan
  Cc: linux-kernel, linux-riscv, linux-kselftest, Thomas Weißschuh

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (4):
      selftests/nolibc: allow building i386 with multiarch compiler
      tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
      tools/nolibc: don't define new syscall number
      tools/nolibc: automatically detect necessity to use pselect6

 tools/include/nolibc/arch-aarch64.h     |  3 ---
 tools/include/nolibc/arch-loongarch.h   |  4 +---
 tools/include/nolibc/arch-riscv.h       |  3 ---
 tools/include/nolibc/sys.h              | 42 ++++++++++++++++++++++++++++-----
 tools/testing/selftests/nolibc/Makefile |  1 +
 5 files changed, 38 insertions(+), 15 deletions(-)
---
base-commit: 3f79a57865b33f49fdae6655510bd27c8e6610e0
change-id: 20230914-nolibc-syscall-nr-9a5f544c8494

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler
  2023-09-14 16:01 [PATCH 0/4] tools/nolibc: cleanups for syscall fallbacks Thomas Weißschuh
@ 2023-09-14 16:01 ` Thomas Weißschuh
  2023-09-17  2:49   ` Willy Tarreau
  2023-09-14 16:01 ` [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks Thomas Weißschuh
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-14 16:01 UTC (permalink / raw)
  To: Willy Tarreau, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan
  Cc: linux-kernel, linux-riscv, linux-kselftest, Thomas Weißschuh

When building with a multiarch-capable compiler, like those provided by
common distributions the -m32 argument is required to build 32bit code.

Wrap it in cc-option in case the compiler is not multiarch-capable.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 689658f81a19..19096a9ded55 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -113,6 +113,7 @@ else
 Q=@
 endif
 
+CFLAGS_i386 = $(call cc-option,-m32)
 CFLAGS_ppc = -m32 -mbig-endian -mno-vsx $(call cc-option,-mmultiple)
 CFLAGS_ppc64 = -m64 -mbig-endian -mno-vsx $(call cc-option,-mmultiple)
 CFLAGS_ppc64le = -m64 -mlittle-endian -mno-vsx $(call cc-option,-mabi=elfv2)

-- 
2.42.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
  2023-09-14 16:01 [PATCH 0/4] tools/nolibc: cleanups for syscall fallbacks Thomas Weißschuh
  2023-09-14 16:01 ` [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler Thomas Weißschuh
@ 2023-09-14 16:01 ` Thomas Weißschuh
  2023-09-17  2:58   ` Willy Tarreau
  2023-09-14 16:01 ` [PATCH 3/4] tools/nolibc: don't define new syscall number Thomas Weißschuh
  2023-09-14 16:01 ` [PATCH 4/4] tools/nolibc: automatically detect necessity to use pselect6 Thomas Weißschuh
  3 siblings, 1 reply; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-14 16:01 UTC (permalink / raw)
  To: Willy Tarreau, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan
  Cc: linux-kernel, linux-riscv, linux-kselftest, Thomas Weißschuh

The ENOSYS fallback code does not use its functions parameters.
This can lead to compiler warnings about unused parameters.

Explicitly avoid these warnings.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/sys.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index b478750c9004..bc56310c6bdf 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -133,6 +133,8 @@ int sys_chmod(const char *path, mode_t mode)
 #elif defined(__NR_chmod)
 	return my_syscall2(__NR_chmod, path, mode);
 #else
+	(void)path;
+	(void)mode;
 	return -ENOSYS;
 #endif
 }
@@ -156,6 +158,9 @@ int sys_chown(const char *path, uid_t owner, gid_t group)
 #elif defined(__NR_chown)
 	return my_syscall3(__NR_chown, path, owner, group);
 #else
+	(void)path;
+	(void)owner;
+	(void)group;
 	return -ENOSYS;
 #endif
 }
@@ -230,6 +235,8 @@ int sys_dup2(int old, int new)
 #elif defined(__NR_dup2)
 	return my_syscall2(__NR_dup2, old, new);
 #else
+	(void)old;
+	(void)new;
 	return -ENOSYS;
 #endif
 }
@@ -486,6 +493,8 @@ int sys_gettimeofday(struct timeval *tv, struct timezone *tz)
 #ifdef __NR_gettimeofday
 	return my_syscall2(__NR_gettimeofday, tv, tz);
 #else
+	(void)tv;
+	(void)tz;
 	return -ENOSYS;
 #endif
 }
@@ -563,6 +572,8 @@ int sys_link(const char *old, const char *new)
 #elif defined(__NR_link)
 	return my_syscall2(__NR_link, old, new);
 #else
+	(void)old;
+	(void)new;
 	return -ENOSYS;
 #endif
 }
@@ -584,6 +595,9 @@ off_t sys_lseek(int fd, off_t offset, int whence)
 #ifdef __NR_lseek
 	return my_syscall3(__NR_lseek, fd, offset, whence);
 #else
+	(void)fd;
+	(void)offset;
+	(void)whence;
 	return -ENOSYS;
 #endif
 }
@@ -607,6 +621,8 @@ int sys_mkdir(const char *path, mode_t mode)
 #elif defined(__NR_mkdir)
 	return my_syscall2(__NR_mkdir, path, mode);
 #else
+	(void)path;
+	(void)mode;
 	return -ENOSYS;
 #endif
 }
@@ -629,6 +645,7 @@ int sys_rmdir(const char *path)
 #elif defined(__NR_unlinkat)
 	return my_syscall3(__NR_unlinkat, AT_FDCWD, path, AT_REMOVEDIR);
 #else
+	(void)path;
 	return -ENOSYS;
 #endif
 }
@@ -652,6 +669,9 @@ long sys_mknod(const char *path, mode_t mode, dev_t dev)
 #elif defined(__NR_mknod)
 	return my_syscall3(__NR_mknod, path, mode, dev);
 #else
+	(void)path;
+	(void)mode;
+	(void)dev;
 	return -ENOSYS;
 #endif
 }
@@ -742,6 +762,9 @@ int sys_open(const char *path, int flags, mode_t mode)
 #elif defined(__NR_open)
 	return my_syscall3(__NR_open, path, flags, mode);
 #else
+	(void)path;
+	(void)flags;
+	(void)mode;
 	return -ENOSYS;
 #endif
 }
@@ -842,6 +865,9 @@ int sys_poll(struct pollfd *fds, int nfds, int timeout)
 #elif defined(__NR_poll)
 	return my_syscall3(__NR_poll, fds, nfds, timeout);
 #else
+	(void)fds;
+	(void)nfds;
+	(void)timeout;
 	return -ENOSYS;
 #endif
 }
@@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
 #endif
 	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
 #else
+	(void)nfds;
+	(void)rfds;
+	(void)wfds;
+	(void)efds;
+	(void)timeout;
 	return -ENOSYS;
 #endif
 }

-- 
2.42.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] tools/nolibc: don't define new syscall number
  2023-09-14 16:01 [PATCH 0/4] tools/nolibc: cleanups for syscall fallbacks Thomas Weißschuh
  2023-09-14 16:01 ` [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler Thomas Weißschuh
  2023-09-14 16:01 ` [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks Thomas Weißschuh
@ 2023-09-14 16:01 ` Thomas Weißschuh
  2023-09-17  2:59   ` Willy Tarreau
  2023-09-14 16:01 ` [PATCH 4/4] tools/nolibc: automatically detect necessity to use pselect6 Thomas Weißschuh
  3 siblings, 1 reply; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-14 16:01 UTC (permalink / raw)
  To: Willy Tarreau, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan
  Cc: linux-kernel, linux-riscv, linux-kselftest, Thomas Weißschuh

All symbols created by nolibc are also visible to user code.
Syscall constants are expected to come from the kernel headers and
should not be made up by nolibc.

Refactor the logic to avoid defining syscall numbers.
Also the new code is easier to understand.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/sys.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index bc56310c6bdf..d96f2aa7d987 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -954,11 +954,10 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
 		t.tv_nsec = timeout->tv_usec * 1000;
 	}
 	return my_syscall6(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
-#elif defined(__NR__newselect) || defined(__NR_select)
-#ifndef __NR__newselect
-#define __NR__newselect __NR_select
-#endif
+#elif defined(__NR__newselect)
 	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
+#elif defined(__NR_select)
+	return my_syscall5(__NR_select, nfds, rfds, wfds, efds, timeout);
 #else
 	(void)nfds;
 	(void)rfds;

-- 
2.42.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] tools/nolibc: automatically detect necessity to use pselect6
  2023-09-14 16:01 [PATCH 0/4] tools/nolibc: cleanups for syscall fallbacks Thomas Weißschuh
                   ` (2 preceding siblings ...)
  2023-09-14 16:01 ` [PATCH 3/4] tools/nolibc: don't define new syscall number Thomas Weißschuh
@ 2023-09-14 16:01 ` Thomas Weißschuh
  2023-09-17  3:07   ` Willy Tarreau
  3 siblings, 1 reply; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-14 16:01 UTC (permalink / raw)
  To: Willy Tarreau, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Shuah Khan
  Cc: linux-kernel, linux-riscv, linux-kselftest, Thomas Weißschuh

We can automatically detect if pselect6 is needed or not from the kernel
headers. This removes the need to manually specify it.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/arch-aarch64.h   |  3 ---
 tools/include/nolibc/arch-loongarch.h |  4 +---
 tools/include/nolibc/arch-riscv.h     |  3 ---
 tools/include/nolibc/sys.h            | 10 +++++-----
 4 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index 6c33c46848e3..b23ac1f04035 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -20,10 +20,7 @@
  *   - the arguments are cast to long and assigned into the target registers
  *     which are then simply passed as registers to the asm code, so that we
  *     don't have to experience issues with register constraints.
- *
- * On aarch64, select() is not implemented so we have to use pselect6().
  */
-#define __ARCH_WANT_SYS_PSELECT6
 
 #define my_syscall0(num)                                                      \
 ({                                                                            \
diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
index bf98f6220195..3f8ef8f86c0f 100644
--- a/tools/include/nolibc/arch-loongarch.h
+++ b/tools/include/nolibc/arch-loongarch.h
@@ -19,10 +19,8 @@
  *   - the arguments are cast to long and assigned into the target
  *     registers which are then simply passed as registers to the asm code,
  *     so that we don't have to experience issues with register constraints.
- *
- * On LoongArch, select() is not implemented so we have to use pselect6().
  */
-#define __ARCH_WANT_SYS_PSELECT6
+
 #define _NOLIBC_SYSCALL_CLOBBERLIST \
 	"memory", "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8"
 
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index 950cc2283fd7..1927c643c739 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -19,10 +19,7 @@
  *   - the arguments are cast to long and assigned into the target
  *     registers which are then simply passed as registers to the asm code,
  *     so that we don't have to experience issues with register constraints.
- *
- * On riscv, select() is not implemented so we have to use pselect6().
  */
-#define __ARCH_WANT_SYS_PSELECT6
 
 #define my_syscall0(num)                                                      \
 ({                                                                            \
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index d96f2aa7d987..dbf5cc92181b 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -946,7 +946,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
 		struct timeval *t;
 	} arg = { .n = nfds, .r = rfds, .w = wfds, .e = efds, .t = timeout };
 	return my_syscall1(__NR_select, &arg);
-#elif defined(__ARCH_WANT_SYS_PSELECT6) && defined(__NR_pselect6)
+#elif defined(__NR__newselect)
+	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
+#elif defined(__NR_select)
+	return my_syscall5(__NR_select, nfds, rfds, wfds, efds, timeout);
+#elif defined(__NR_pselect6)
 	struct timespec t;
 
 	if (timeout) {
@@ -954,10 +958,6 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
 		t.tv_nsec = timeout->tv_usec * 1000;
 	}
 	return my_syscall6(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
-#elif defined(__NR__newselect)
-	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
-#elif defined(__NR_select)
-	return my_syscall5(__NR_select, nfds, rfds, wfds, efds, timeout);
 #else
 	(void)nfds;
 	(void)rfds;

-- 
2.42.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler
  2023-09-14 16:01 ` [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler Thomas Weißschuh
@ 2023-09-17  2:49   ` Willy Tarreau
  0 siblings, 0 replies; 13+ messages in thread
From: Willy Tarreau @ 2023-09-17  2:49 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On Thu, Sep 14, 2023 at 06:01:17PM +0200, Thomas Weißschuh wrote:
> When building with a multiarch-capable compiler, like those provided by
> common distributions the -m32 argument is required to build 32bit code.
> 
> Wrap it in cc-option in case the compiler is not multiarch-capable.

I think it's a good idea. Zhangjin was also using a single compiler for
both archs.

> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Acked-by: Willy Tarreau <w@1wt.eu>

Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
  2023-09-14 16:01 ` [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks Thomas Weißschuh
@ 2023-09-17  2:58   ` Willy Tarreau
  2023-09-17  5:49     ` Thomas Weißschuh
  0 siblings, 1 reply; 13+ messages in thread
From: Willy Tarreau @ 2023-09-17  2:58 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On Thu, Sep 14, 2023 at 06:01:18PM +0200, Thomas Weißschuh wrote:
> The ENOSYS fallback code does not use its functions parameters.
> This can lead to compiler warnings about unused parameters.
> 
> Explicitly avoid these warnings.

Just out of curiosity, did you find a valid case for enabling this
warning or were you trying various combinations ? I'm asking because
I've never seen it enabled anywhere given that it's probably the most 
useless and unusable warning: as soon as you're dealing with function
pointers, you start to have multiple functions with a similar
prototype, some of which just don't need certain arguments, and the
only way to shut the warning is to significantly uglify the code.

If really needed, I'm wondering if instead we shouldn't have an
"no_syscall*" set of macros, that would have the same signature
as my_syscall* to just consume all args in the same order and
return -ENOSYS. E.g, consider the following:

  @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
   #endif
   	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
   #else
  +	(void)nfds;
  +	(void)rfds;
  +	(void)wfds;
  +	(void)efds;
  +	(void)timeout;
   	return -ENOSYS;
   #endif

It would become:

  @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
   #endif
   	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
   #else
  +	return no_syscall5(nfds, rfds, wfds, efds, timeout);
  -	return -ENOSYS;
   #endif

What do you think ?

Thanks!
Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] tools/nolibc: don't define new syscall number
  2023-09-14 16:01 ` [PATCH 3/4] tools/nolibc: don't define new syscall number Thomas Weißschuh
@ 2023-09-17  2:59   ` Willy Tarreau
  0 siblings, 0 replies; 13+ messages in thread
From: Willy Tarreau @ 2023-09-17  2:59 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On Thu, Sep 14, 2023 at 06:01:19PM +0200, Thomas Weißschuh wrote:
> All symbols created by nolibc are also visible to user code.
> Syscall constants are expected to come from the kernel headers and
> should not be made up by nolibc.
> 
> Refactor the logic to avoid defining syscall numbers.
> Also the new code is easier to understand.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Yeah that's a good point!

Acked-by: Willy Tarreau <w@1wt.eu>
Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] tools/nolibc: automatically detect necessity to use pselect6
  2023-09-14 16:01 ` [PATCH 4/4] tools/nolibc: automatically detect necessity to use pselect6 Thomas Weißschuh
@ 2023-09-17  3:07   ` Willy Tarreau
  0 siblings, 0 replies; 13+ messages in thread
From: Willy Tarreau @ 2023-09-17  3:07 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On Thu, Sep 14, 2023 at 06:01:20PM +0200, Thomas Weißschuh wrote:
> We can automatically detect if pselect6 is needed or not from the kernel
> headers. This removes the need to manually specify it.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

That's indeed cleaner, I can't find the reason why we didn't do it
like this initially, I suspect that maybe we were having __NR_select
defined but not usable, I don't know. I've found it first introduced with
nolibc commit 28a7178 ("nolibc: fall back to pselect6() on aarch64"), and
the test was made before __NR_newselect, so maybe I wanted to be sure not
to use pselect6() in case another arch would define it.

Let's do as you propose, it's much cleaner and simpler. If we ever find
any breakage at least we'll know how to deal with it so I'm not worried.

Acked-by: Willy Tarreau <w@1wt.eu>

Thanks!
Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
  2023-09-17  2:58   ` Willy Tarreau
@ 2023-09-17  5:49     ` Thomas Weißschuh
  2023-09-17  9:48       ` Willy Tarreau
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-17  5:49 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On 2023-09-17 04:58:51+0200, Willy Tarreau wrote:
> On Thu, Sep 14, 2023 at 06:01:18PM +0200, Thomas Weißschuh wrote:
> > The ENOSYS fallback code does not use its functions parameters.
> > This can lead to compiler warnings about unused parameters.
> > 
> > Explicitly avoid these warnings.
> 
> Just out of curiosity, did you find a valid case for enabling this
> warning or were you trying various combinations ? I'm asking because
> I've never seen it enabled anywhere given that it's probably the most 
> useless and unusable warning: as soon as you're dealing with function
> pointers, you start to have multiple functions with a similar
> prototype, some of which just don't need certain arguments, and the
> only way to shut the warning is to significantly uglify the code.

nolibc-test uses it currently and I also used it in some projects.

> If really needed, I'm wondering if instead we shouldn't have an
> "no_syscall*" set of macros, that would have the same signature
> as my_syscall* to just consume all args in the same order and
> return -ENOSYS. E.g, consider the following:
> 
>   @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
>    #endif
>    	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
>    #else
>   +	(void)nfds;
>   +	(void)rfds;
>   +	(void)wfds;
>   +	(void)efds;
>   +	(void)timeout;
>    	return -ENOSYS;
>    #endif
> 
> It would become:
> 
>   @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
>    #endif
>    	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
>    #else
>   +	return no_syscall5(nfds, rfds, wfds, efds, timeout);
>   -	return -ENOSYS;
>    #endif
> 
> What do you think ?

The idea sounds good. But "no_syscall5" sounds a bit non-obvious to me.

Maybe the macro-equivalent of this?

static inline int __nolibc_enosys(...)
{
	return -ENOSYS;
}

The only-vararg function unfortunately needs C23 so we can't use it.

It's clear to the users that this is about ENOSYS and we don't need a
bunch of new macros similar.

I'll check if it is cleaner to implement a generic macro or a few
numbered ones.


Thomas

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
  2023-09-17  5:49     ` Thomas Weißschuh
@ 2023-09-17  9:48       ` Willy Tarreau
  2023-09-17 15:07         ` Thomas Weißschuh
  0 siblings, 1 reply; 13+ messages in thread
From: Willy Tarreau @ 2023-09-17  9:48 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On Sun, Sep 17, 2023 at 07:49:57AM +0200, Thomas Weißschuh wrote:
> On 2023-09-17 04:58:51+0200, Willy Tarreau wrote:
> > On Thu, Sep 14, 2023 at 06:01:18PM +0200, Thomas Weißschuh wrote:
> > > The ENOSYS fallback code does not use its functions parameters.
> > > This can lead to compiler warnings about unused parameters.
> > > 
> > > Explicitly avoid these warnings.
> > 
> > Just out of curiosity, did you find a valid case for enabling this
> > warning or were you trying various combinations ? I'm asking because
> > I've never seen it enabled anywhere given that it's probably the most 
> > useless and unusable warning: as soon as you're dealing with function
> > pointers, you start to have multiple functions with a similar
> > prototype, some of which just don't need certain arguments, and the
> > only way to shut the warning is to significantly uglify the code.
> 
> nolibc-test uses it currently and I also used it in some projects.

OK then let's handle it.

> >   @@ -934,6 +960,11 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
> >    #endif
> >    	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
> >    #else
> >   +	return no_syscall5(nfds, rfds, wfds, efds, timeout);
> >   -	return -ENOSYS;
> >    #endif
> > 
> > What do you think ?
> 
> The idea sounds good. But "no_syscall5" sounds a bit non-obvious to me.

Of course, I was just trying to illustrate. I'm never good at giving
names.

> Maybe the macro-equivalent of this?
> 
> static inline int __nolibc_enosys(...)
> {
> 	return -ENOSYS;
> }
> 
> The only-vararg function unfortunately needs C23 so we can't use it.
>
> It's clear to the users that this is about ENOSYS and we don't need a
> bunch of new macros similar.

I like it, I didn't think about varargs, it's an excellent idea! Let's
just do simpler, start with a first arg "syscall_num" that we may later
reuse for debugging, and just mark this one unused:

  static inline int __nolibc_enosys(int syscall_num, ...)
  {
	(void)syscall_num;
  	return -ENOSYS;
  }

Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
  2023-09-17  9:48       ` Willy Tarreau
@ 2023-09-17 15:07         ` Thomas Weißschuh
  2023-09-17 15:08           ` Willy Tarreau
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Weißschuh @ 2023-09-17 15:07 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On 2023-09-17 11:48:27+0200, Willy Tarreau wrote:
> [..]
> > Maybe the macro-equivalent of this?
> > 
> > static inline int __nolibc_enosys(...)
> > {
> > 	return -ENOSYS;
> > }
> > 
> > The only-vararg function unfortunately needs C23 so we can't use it.
> >
> > It's clear to the users that this is about ENOSYS and we don't need a
> > bunch of new macros similar.
> 
> I like it, I didn't think about varargs, it's an excellent idea! Let's
> just do simpler, start with a first arg "syscall_num" that we may later
> reuse for debugging, and just mark this one unused:
> 
>   static inline int __nolibc_enosys(int syscall_num, ...)
>   {
> 	(void)syscall_num;
>   	return -ENOSYS;
>   }

But which syscall_num to use, as the point of __nolibc_enosys() would be
that no syscall number is available and the defines are missing.

For debugging we could add a string argument, though.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks
  2023-09-17 15:07         ` Thomas Weißschuh
@ 2023-09-17 15:08           ` Willy Tarreau
  0 siblings, 0 replies; 13+ messages in thread
From: Willy Tarreau @ 2023-09-17 15:08 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Shuah Khan,
	linux-kernel, linux-riscv, linux-kselftest

On Sun, Sep 17, 2023 at 05:07:18PM +0200, Thomas Weißschuh wrote:
> On 2023-09-17 11:48:27+0200, Willy Tarreau wrote:
> > [..]
> > > Maybe the macro-equivalent of this?
> > > 
> > > static inline int __nolibc_enosys(...)
> > > {
> > > 	return -ENOSYS;
> > > }
> > > 
> > > The only-vararg function unfortunately needs C23 so we can't use it.
> > >
> > > It's clear to the users that this is about ENOSYS and we don't need a
> > > bunch of new macros similar.
> > 
> > I like it, I didn't think about varargs, it's an excellent idea! Let's
> > just do simpler, start with a first arg "syscall_num" that we may later
> > reuse for debugging, and just mark this one unused:
> > 
> >   static inline int __nolibc_enosys(int syscall_num, ...)
> >   {
> > 	(void)syscall_num;
> >   	return -ENOSYS;
> >   }
> 
> But which syscall_num to use, as the point of __nolibc_enosys() would be
> that no syscall number is available and the defines are missing.

good point :-)

> For debugging we could add a string argument, though.

That works for me.

Willy

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-09-17 15:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-14 16:01 [PATCH 0/4] tools/nolibc: cleanups for syscall fallbacks Thomas Weißschuh
2023-09-14 16:01 ` [PATCH 1/4] selftests/nolibc: allow building i386 with multiarch compiler Thomas Weißschuh
2023-09-17  2:49   ` Willy Tarreau
2023-09-14 16:01 ` [PATCH 2/4] tools/nolibc: avoid unused parameter warnings for ENOSYS fallbacks Thomas Weißschuh
2023-09-17  2:58   ` Willy Tarreau
2023-09-17  5:49     ` Thomas Weißschuh
2023-09-17  9:48       ` Willy Tarreau
2023-09-17 15:07         ` Thomas Weißschuh
2023-09-17 15:08           ` Willy Tarreau
2023-09-14 16:01 ` [PATCH 3/4] tools/nolibc: don't define new syscall number Thomas Weißschuh
2023-09-17  2:59   ` Willy Tarreau
2023-09-14 16:01 ` [PATCH 4/4] tools/nolibc: automatically detect necessity to use pselect6 Thomas Weißschuh
2023-09-17  3:07   ` Willy Tarreau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).