On 11/24/21 1:49 AM, Arnd Bergmann wrote: > On Wed, Nov 24, 2021 at 8:31 AM Rob Landley <rob@xxxxxxxxxxx> wrote: >> On 11/23/21 5:38 AM, Naresh Kamboju wrote: >> >> diff --git a/arch/sh/kernel/syscalls/syscall.tbl >> b/arch/sh/kernel/syscalls/syscall.tbl >> index 208f131659c5..65c3a94bff48 100644 >> --- a/arch/sh/kernel/syscalls/syscall.tbl >> +++ b/arch/sh/kernel/syscalls/syscall.tbl >> @@ -437,7 +437,7 @@ >> 432 common fsmount sys_fsmount >> 433 common fspick sys_fspick >> 434 common pidfd_open sys_pidfd_open >> -# 435 reserved for clone3 >> +435 common clone3 sys_clone3 >> 436 common close_range sys_close_range >> 437 common openat2 sys_openat2 >> 438 common pidfd_getfd sys_pidfd_getfd > > Did you test clone3? Haven't got anything that's using it (musl-libc doesn't know about it yet) but it looked straightforward? (Unlike the #ifdef stack around the previous clone...) I can try building tools/testing/selftests/clone3 if you like, but for some reason the clone3 tests want -lcap which isn't in my cross compiler. (Because to test a clone system call, you need to manipulate capability bits. Of course.) Right, comment out the LDLIBS line in the makefile and the first 3 built, let's try those... Hmmm, it's saying the syscall isn't supported, because it's using syscall.h out of the cross compiler headers (not THIS kernel's #includes) which of course doesn't have it, and then clone3_selftests.h falls back to: #ifndef __NR_clone3 #define __NR_clone3 -1 #endif Right, stick a 435 in there and... it's still skipping it. Why is it still skipping it... because the RUNTIME syscall is returning ENOSYS. Ok, I have to go stick printk() calls into the kernel. (Do I have to #define those #YES_I_WANT_THIS_SYSCALL_WHY_WOULDNT_I macros? Hmmm...) But yeah, you're right: the naive patch doesn't look like it actually works. Just shuts up the #warnings. > This needs a custom wrapper on most architectures > to have sensible calling conventions. Define "sensible" in this context? It's a new 2 argument syscall? (Do you mean a libc wrapper?) > If sh doesn't need it, that should > be explained in the changelog text. I'm happy to try to fix stuff up, but I don't understand the objection. Does it do something other than what the old clone did, except without the need to pass more arguments than we necessarily have registers defined for? (Calls the same clone plumbing, which should call back into arch/sh/kernel/process_32.c already...?) The most recent clone3 arch addition was commit 59a4e0d5511b which also just pulled in the generic version. (Via #define NO_REALLY_I_WANT_THIS_SYSCALL rather than editing the tbl file? Looks like I've got some reading to do...) >> @@ -451,3 +451,4 @@ >> 446 common landlock_restrict_self sys_landlock_restrict_self >> # 447 reserved for memfd_secret >> 448 common process_mrelease sys_process_mrelease >> +449 common futex_waitv sys_futex_waitv > > I don't know what's going on with this one, I don't actually see > a reason why it isn't already wired up on all architectures. Me neither, I'm just trying to stay ahead of warnings due to the encroaching -Werror culture going on within the kernel clique. > If we add > this, it should probably be done for all architectures at once as a > bugfix, but it's possible that this is intentionally only used on > x86 and arm. > > André, can you comment on this? I see elsethread the second syscall got handled and is going in through another tree, I'll leave off on that part... Rob