* rcutorture initrd/nolibc build on ARMv8? @ 2021-01-19 15:31 Paul E. McKenney 2021-01-19 16:19 ` Willy Tarreau 2021-02-12 12:37 ` [tip: core/rcu] tools/nolibc: Remove incorrect definitions of __ARCH_WANT_* tip-bot2 for Willy Tarreau 0 siblings, 2 replies; 14+ messages in thread From: Paul E. McKenney @ 2021-01-19 15:31 UTC (permalink / raw) To: w; +Cc: linux-kernel, valentin.schneider, mark.rutland Hello, Willy, Some people are having trouble running rcutorture on ARMv8. They get things like this from the nolibc build of initrd: https://paste.debian.net/1181762/ The nolibc.h file says this: /* Some archs (at least aarch64) don't expose the regular syscalls anymore by * default, either because they have an "_at" replacement, or because there are * more modern alternatives. For now we'd rather still use them. */ Are these build failures expected behavior on ARMv8? Thanx, Paul ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-19 15:31 rcutorture initrd/nolibc build on ARMv8? Paul E. McKenney @ 2021-01-19 16:19 ` Willy Tarreau 2021-01-19 17:02 ` Mark Rutland 2021-02-12 12:37 ` [tip: core/rcu] tools/nolibc: Remove incorrect definitions of __ARCH_WANT_* tip-bot2 for Willy Tarreau 1 sibling, 1 reply; 14+ messages in thread From: Willy Tarreau @ 2021-01-19 16:19 UTC (permalink / raw) To: Paul E. McKenney; +Cc: linux-kernel, valentin.schneider, mark.rutland Hi Paul, On Tue, Jan 19, 2021 at 07:31:47AM -0800, Paul E. McKenney wrote: > Hello, Willy, > > Some people are having trouble running rcutorture on ARMv8. They > get things like this from the nolibc build of initrd: > > https://paste.debian.net/1181762/ > > The nolibc.h file says this: > > /* Some archs (at least aarch64) don't expose the regular syscalls anymore by > * default, either because they have an "_at" replacement, or because there are > * more modern alternatives. For now we'd rather still use them. > */ > > Are these build failures expected behavior on ARMv8? No, I don't think so. I'm regularly building my own init using this, and it works on various platforms including aarch64, while it makes use of a number of such syscalls. From what I'm seeing, this seems to happen each time in such a construct: #if defined(__NR_new_name) use __NR_new_name #else use __NR_old_name #endif with the error appearing on old_name. So I guess that we're rather facing a case where a number of such __NR_* entries are not defined because presumably one include file might be missing (probably from a recent change of headers dependency). I can't spot from the report above the original C file that was attempted to be built, it makes me think we tried to compile directly the .h file. Having it run through sh -x would help me try to locate the root cause or possibly even attempt to reproduce it. Thanks, Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-19 16:19 ` Willy Tarreau @ 2021-01-19 17:02 ` Mark Rutland 2021-01-19 17:16 ` Willy Tarreau 0 siblings, 1 reply; 14+ messages in thread From: Mark Rutland @ 2021-01-19 17:02 UTC (permalink / raw) To: Willy Tarreau; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider Hi Willy, On Tue, Jan 19, 2021 at 05:19:01PM +0100, Willy Tarreau wrote: > On Tue, Jan 19, 2021 at 07:31:47AM -0800, Paul E. McKenney wrote: > > Some people are having trouble running rcutorture on ARMv8. They > > get things like this from the nolibc build of initrd: > > > > https://paste.debian.net/1181762/ > > > > The nolibc.h file says this: > > > > /* Some archs (at least aarch64) don't expose the regular syscalls anymore by > > * default, either because they have an "_at" replacement, or because there are > > * more modern alternatives. For now we'd rather still use them. > > */ > > > > Are these build failures expected behavior on ARMv8? > > No, I don't think so. I'm regularly building my own init using this, > and it works on various platforms including aarch64, while it makes > use of a number of such syscalls. > > From what I'm seeing, this seems to happen each time in such a construct: > > #if defined(__NR_new_name) > use __NR_new_name > #else > use __NR_old_name > #endif > > with the error appearing on old_name. So I guess that we're rather facing a > case where a number of such __NR_* entries are not defined because presumably > one include file might be missing (probably from a recent change of headers > dependency). > > I can't spot from the report above the original C file that was attempted > to be built, it makes me think we tried to compile directly the .h file. That was the inline snippet in tools/testing/selftests/rcutorture/bin/mkinitrd.sh: | #ifndef NOLIBC | #include <unistd.h> | #include <sys/time.h> | #endif | | volatile unsigned long delaycount; | | int main(int argc, int argv[]) | { | int i; | struct timeval tv; | struct timeval tvb; | | for (;;) { | sleep(1); | /* Need some userspace time. */ | if (gettimeofday(&tvb, NULL)) | continue; | do { | for (i = 0; i < 1000 * 100; i++) | delaycount = i * i; | if (gettimeofday(&tv, NULL)) | break; | tv.tv_sec -= tvb.tv_sec; | if (tv.tv_sec > 1) | break; | tv.tv_usec += tv.tv_sec * 1000 * 1000; | tv.tv_usec -= tvb.tv_usec; | } while (tv.tv_usec < 1000); | } | return 0; | } ... which gets written to a file called init.c, and then built with: | ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \ | -nostdlib -include ../../../../include/nolibc/nolibc.h \ | -lgcc -s -static -Os -o init init.c I was building natively on an arm64 box: | ./tools/testing/selftests/rcutorture/bin/kvm.sh \ | --cpus 250 --trust-make --configs "TREE03" \ | --kmake-arg "CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64" > Having it run through sh -x would help me try to locate the root cause or > possibly even attempt to reproduce it. I ran with sh -x, but it didn't log the compiler invocation; hopefully the above is sufficient? Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-19 17:02 ` Mark Rutland @ 2021-01-19 17:16 ` Willy Tarreau 2021-01-19 17:43 ` Willy Tarreau 0 siblings, 1 reply; 14+ messages in thread From: Willy Tarreau @ 2021-01-19 17:16 UTC (permalink / raw) To: Mark Rutland; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider Hi Mark, On Tue, Jan 19, 2021 at 05:02:38PM +0000, Mark Rutland wrote: > > I can't spot from the report above the original C file that was attempted > > to be built, it makes me think we tried to compile directly the .h file. > > That was the inline snippet in > tools/testing/selftests/rcutorture/bin/mkinitrd.sh: > > | #ifndef NOLIBC > | #include <unistd.h> > | #include <sys/time.h> > | #endif > | > | volatile unsigned long delaycount; > | > | int main(int argc, int argv[]) > | { > | int i; > | struct timeval tv; > | struct timeval tvb; > | > | for (;;) { > | sleep(1); > | /* Need some userspace time. */ > | if (gettimeofday(&tvb, NULL)) > | continue; > | do { > | for (i = 0; i < 1000 * 100; i++) > | delaycount = i * i; > | if (gettimeofday(&tv, NULL)) > | break; > | tv.tv_sec -= tvb.tv_sec; > | if (tv.tv_sec > 1) > | break; > | tv.tv_usec += tv.tv_sec * 1000 * 1000; > | tv.tv_usec -= tvb.tv_usec; > | } while (tv.tv_usec < 1000); > | } > | return 0; > | } > > ... which gets written to a file called init.c, and then built with: > > | ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \ > | -nostdlib -include ../../../../include/nolibc/nolibc.h \ > | -lgcc -s -static -Os -o init init.c OK I'll retry this, thank you! > I was building natively on an arm64 box: > > | ./tools/testing/selftests/rcutorture/bin/kvm.sh \ > | --cpus 250 --trust-make --configs "TREE03" \ > | --kmake-arg "CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64" > > > Having it run through sh -x would help me try to locate the root cause or > > possibly even attempt to reproduce it. > > I ran with sh -x, but it didn't log the compiler invocation; hopefully > the above is sufficient? I guess so, yes. I'm pretty sure I'll come back with new questions soon :-) Thanks, Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-19 17:16 ` Willy Tarreau @ 2021-01-19 17:43 ` Willy Tarreau 2021-01-20 12:07 ` Mark Rutland 0 siblings, 1 reply; 14+ messages in thread From: Willy Tarreau @ 2021-01-19 17:43 UTC (permalink / raw) To: Mark Rutland; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Tue, Jan 19, 2021 at 06:16:37PM +0100, Willy Tarreau wrote: > > | ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \ > > | -nostdlib -include ../../../../include/nolibc/nolibc.h \ > > | -lgcc -s -static -Os -o init init.c > > OK I'll retry this, thank you! For me on my x86 PC it worked using a cross-compiler: $ /f/tc/gcc-linaro-6.4.1-2018.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include tools/include/nolibc/nolibc.h -lgcc -s -static -Os -o init-fail init-fail.c $ file init-fail init-fail: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, BuildID[sha1]=1088f8fad85b7b8a7f00eef4918d24c4c4fdaadf, stripped Given that you used a native compiler we can't suspect an issue with a bare-metal compiler possibly affecting how kernel headers are passed there. But nevertheless, I'd still not disregard the possibility that the headers found under "linux/" are picked from the libc which for whatever reason would be missing a lot of them. I've now tested natively on a local rpi4b onto which I copied the files (not enough I/O BW on the SD card to clone a kernel there). Tried: $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -Os -o init-fail init-fail.c specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/aarch64-linux-gnu/7/lto-wrapper Target: aarch64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 7.5.0-3ubuntu1~18.04' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-7 --program-prefix=aarch64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libquadmath --disable-libquadmath-support --enable-plugin --enable-default-pie --with-system-zlib --enable-multiarch --enable-fix-cortex-a53-843419 --disable-werror --enable-checking=release --build=aarch64-linux-gnu --host=aarch64-linux-gnu --target=aarch64-linux-gnu Thread model: posix gcc version 7.5.0 (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) Could you please check the output of this from your intermediary "init.c" file: $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include tools/include/nolibc/nolibc.h -lgcc -s -static -E init.c | grep '^#' It might give us a clue about where it's finding its includes. And possibly the list of __NR_ entries reported here as well: $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include tools/include/nolibc/nolibc.h -lgcc -s -static -E -dM init.c | grep __NR_ We've seen that __NR_fork or __NR_dup2 for example were missing in your output, on my native machine I can see them, so that could give us a clue about the root cause of the issue: $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(fork|dup2)' #define __NR_dup2 1041 #define __NR_syscalls (__NR_fork+1) #define __NR_fork 1079 I could easily imagine a missing "linux" link or entry somewhere in the default includes path, but here since there's no such error it rather looks like an incorrect file which is a bit more concerning. I purposely avoided to maintain my own definitions for __NR_* let's hope we won't need to do that :-/ Thanks, Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-19 17:43 ` Willy Tarreau @ 2021-01-20 12:07 ` Mark Rutland 2021-01-20 12:43 ` Willy Tarreau 0 siblings, 1 reply; 14+ messages in thread From: Mark Rutland @ 2021-01-20 12:07 UTC (permalink / raw) To: Willy Tarreau; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Tue, Jan 19, 2021 at 06:43:58PM +0100, Willy Tarreau wrote: > On Tue, Jan 19, 2021 at 06:16:37PM +0100, Willy Tarreau wrote: > Given that you used a native compiler we can't suspect an issue with a > bare-metal compiler possibly affecting how kernel headers are passed > there. But nevertheless, I'd still not disregard the possibility that > the headers found under "linux/" are picked from the libc which for > whatever reason would be missing a lot of them. I think the actual issue here is a misapprehension in nolibc.h, which started blowing up due to a refactoring in asm/unistd.h. In nolibc.h, we do: | /* Some archs (at least aarch64) don't expose the regular syscalls anymore by | * default, either because they have an "_at" replacement, or because there are | * more modern alternatives. For now we'd rather still use them. | */ | #define __ARCH_WANT_SYSCALL_NO_AT | #define __ARCH_WANT_SYSCALL_NO_FLAGS | #define __ARCH_WANT_SYSCALL_DEPRECATED ... but this isn't quite right -- it's not that the syscalls aren't exposed by default, but rather that these syscall numbers are not valid for architectures which do not define the corresponding __ARCH_WANT_* flags. Architectures without those have never implemented the syscalls, and would have returned -ENOSYS for the unrecognized syscall numbers, but the numbers could be allocated to (distinct) syscalls in future. Since commit: a0673fdbcd421052 ("asm-generic: clean up asm/unistd.h") ... those definitions got pulled out of <asm-generic/unistd.h>, and hence it's no longer possible to accidentally get those where a userspace header defines __ARCH_WANT_* in an architecture where they don't exist (e.g. arm64). It seems that the headers on my Debian 10.7 system were generated after that commit, whereas yours were generated before that. > We've seen that __NR_fork or __NR_dup2 for example were missing in your > output, on my native machine I can see them, so that could give us a clue > about the root cause of the issue: > > $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(fork|dup2)' > #define __NR_dup2 1041 > #define __NR_syscalls (__NR_fork+1) > #define __NR_fork 1079 As above, these are bogus for arm64. There is no syscall number for dup2 or fork, and __NR_syscalls is currently only 442. I think the right thing to do is to have nolibc.h detect which syscalls are implemented, and to not define __ARCH_WANT_*. Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 12:07 ` Mark Rutland @ 2021-01-20 12:43 ` Willy Tarreau 2021-01-20 13:45 ` Mark Rutland 0 siblings, 1 reply; 14+ messages in thread From: Willy Tarreau @ 2021-01-20 12:43 UTC (permalink / raw) To: Mark Rutland; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider Hi Mark, On Wed, Jan 20, 2021 at 12:07:25PM +0000, Mark Rutland wrote: > On Tue, Jan 19, 2021 at 06:43:58PM +0100, Willy Tarreau wrote: > > On Tue, Jan 19, 2021 at 06:16:37PM +0100, Willy Tarreau wrote: > > Given that you used a native compiler we can't suspect an issue with a > > bare-metal compiler possibly affecting how kernel headers are passed > > there. But nevertheless, I'd still not disregard the possibility that > > the headers found under "linux/" are picked from the libc which for > > whatever reason would be missing a lot of them. > > I think the actual issue here is a misapprehension in nolibc.h, which > started blowing up due to a refactoring in asm/unistd.h. > > In nolibc.h, we do: > > | /* Some archs (at least aarch64) don't expose the regular syscalls anymore by > | * default, either because they have an "_at" replacement, or because there are > | * more modern alternatives. For now we'd rather still use them. > | */ > | #define __ARCH_WANT_SYSCALL_NO_AT > | #define __ARCH_WANT_SYSCALL_NO_FLAGS > | #define __ARCH_WANT_SYSCALL_DEPRECATED > > ... but this isn't quite right -- it's not that the syscalls aren't > exposed by default, but rather that these syscall numbers are not valid > for architectures which do not define the corresponding __ARCH_WANT_* > flags. Architectures without those have never implemented the syscalls, > and would have returned -ENOSYS for the unrecognized syscall numbers, > but the numbers could be allocated to (distinct) syscalls in future. > > Since commit: > > a0673fdbcd421052 ("asm-generic: clean up asm/unistd.h") > > ... those definitions got pulled out of <asm-generic/unistd.h>, and > hence it's no longer possible to accidentally get those where a > userspace header defines __ARCH_WANT_* in an architecture where they > don't exist (e.g. arm64). I didn't remember the details behind these definitions, I've found this in the related commit message: Some syscall numbers are not exported by default unless a few macros are defined before including unistd.h : #define __ARCH_WANT_SYSCALL_NO_AT #define __ARCH_WANT_SYSCALL_NO_FLAGS #define __ARCH_WANT_SYSCALL_DEPRECATED So it seems I faced difficulties getting the __NR_* entries and only could get them using such definitions :-/ Did you figure the correct way to get __NR_* defined on your machine or should I search ? > It seems that the headers on my Debian 10.7 system were generated after > that commit, whereas yours were generated before that. It's very possible indeed. > > We've seen that __NR_fork or __NR_dup2 for example were missing in your > > output, on my native machine I can see them, so that could give us a clue > > about the root cause of the issue: > > > > $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(fork|dup2)' > > #define __NR_dup2 1041 > > #define __NR_syscalls (__NR_fork+1) > > #define __NR_fork 1079 > > As above, these are bogus for arm64. There is no syscall number for dup2 > or fork, and __NR_syscalls is currently only 442. Ah that's very interesting indeed because actually these ones should only be used when __NR_dup3 or __NR_clone are not defined. Thus I wanted to check the definitions that were reported in your error output but actually what was needed was to figure whether the correct ones were present, and they are, here on my machine (and yes I agree that in this case the dup2/fork above are bofus): ubuntu@ubuntu:~$ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)' #define __NR_clone 220 #define __NR_dup3 24 Do you have these ones with your more recent includes ? Or are these wrong again ? Again I'd be surprised given that I didn't invent them and that my systems boot fine using these. > I think the right thing to do is to have nolibc.h detect which syscalls > are implemented, and to not define __ARCH_WANT_*. Actually that's what was attempted by already focusing on ifdefs but without *any* __NR_* we can't even hope to call a syscall and check for ENOSYS for example. So the code really tries to figure which is the right __NR_ value for a given syscall, and a quick check at my userland code shows that I'm already using dup2() and fork() as defined from dup3() and clone(). Thanks! Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 12:43 ` Willy Tarreau @ 2021-01-20 13:45 ` Mark Rutland 2021-01-20 14:25 ` Willy Tarreau 0 siblings, 1 reply; 14+ messages in thread From: Mark Rutland @ 2021-01-20 13:45 UTC (permalink / raw) To: Willy Tarreau; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Wed, Jan 20, 2021 at 01:43:40PM +0100, Willy Tarreau wrote: > On Wed, Jan 20, 2021 at 12:07:25PM +0000, Mark Rutland wrote: > > On Tue, Jan 19, 2021 at 06:43:58PM +0100, Willy Tarreau wrote: > > > On Tue, Jan 19, 2021 at 06:16:37PM +0100, Willy Tarreau wrote: > Did you figure the correct way to get __NR_* defined on your machine or > should I search ? Please see below. There is no way to get a number for some syscalls, as these do not have a legitimate number on arm64, but this doesn't matter as we can avoid using the number entirely when it does not exist. [...] > > > $ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(fork|dup2)' > > > #define __NR_dup2 1041 > > > #define __NR_syscalls (__NR_fork+1) > > > #define __NR_fork 1079 > > > > As above, these are bogus for arm64. There is no syscall number for dup2 > > or fork, and __NR_syscalls is currently only 442. > > Ah that's very interesting indeed because actually these ones should > only be used when __NR_dup3 or __NR_clone are not defined. Thus I wanted > to check the definitions that were reported in your error output but > actually what was needed was to figure whether the correct ones were > present, and they are, here on my machine (and yes I agree that in this > case the dup2/fork above are bofus): The issue is that even if a function is unused, the compiler still has to parse and compile the code, so where __NR_dup2 is not defined, we'll get a build error for: static __attribute__((unused)) int sys_dup2(int old, int new) { return my_syscall2(__NR_dup2, old, new); } ... we can deal with that by always returning -ENOSYS for unimplemented syscalls, e.g. static __attribute__((unused)) int sys_dup2(int old, int new) { #ifdef __NR_dup2 return my_syscall2(__NR_dup2, old, new); #else return -ENOSYS; #endif } I can spin a patch fixing up all the relevant syscalls, if you'd like? [...] > ubuntu@ubuntu:~$ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)' > #define __NR_clone 220 > #define __NR_dup3 24 > > Do you have these ones with your more recent includes ? Or are these wrong > again ? Those are correct (and all the syscall numbers in unistd.h should be correct so long as you don't erroneously set the __ARCH_WANT_* flags): [mark@gravadlaks:~/src/linux]% gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include tools/include/nolibc/nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)' #define __NR_clone 220 #define __NR_dup3 24 > > I think the right thing to do is to have nolibc.h detect which syscalls > > are implemented, and to not define __ARCH_WANT_*. > > Actually that's what was attempted by already focusing on ifdefs but > without *any* __NR_* we can't even hope to call a syscall and check for > ENOSYS for example. So the code really tries to figure which is the right > __NR_ value for a given syscall, and a quick check at my userland code > shows that I'm already using dup2() and fork() as defined from dup3() > and clone(). Please see above for how to get the -ENOSYS behaviour without relying on bogus syscall numbers. I have a local patch for this, which I can send if you'd like? There's still some latent issue when using nolibc (compared to using glibc) where the init process never seems to exit, but that looks to be orthogonal to the syscall numbering issue -- I'm currently digging into that. Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 13:45 ` Mark Rutland @ 2021-01-20 14:25 ` Willy Tarreau 2021-01-20 14:37 ` Mark Rutland 2021-01-20 14:54 ` Mark Rutland 0 siblings, 2 replies; 14+ messages in thread From: Willy Tarreau @ 2021-01-20 14:25 UTC (permalink / raw) To: Mark Rutland; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Wed, Jan 20, 2021 at 01:45:11PM +0000, Mark Rutland wrote: > > Ah that's very interesting indeed because actually these ones should > > only be used when __NR_dup3 or __NR_clone are not defined. Thus I wanted > > to check the definitions that were reported in your error output but > > actually what was needed was to figure whether the correct ones were > > present, and they are, here on my machine (and yes I agree that in this > > case the dup2/fork above are bofus): > > The issue is that even if a function is unused, the compiler still has > to parse and compile the code, so where __NR_dup2 is not defined, we'll > get a build error for: > > static __attribute__((unused)) > int sys_dup2(int old, int new) > { > return my_syscall2(__NR_dup2, old, new); > } For sure but this is supposed to be used only when __NR_dup3 is not defined. Ah now I understand where my mistake is: after it built successfully for me I inspected the most recent tree which has these in place. Sorry for being stupid! In my local tree it's defined like this: static __attribute__((unused)) int sys_dup2(int old, int new) { #ifdef __NR_dup3 return my_syscall3(__NR_dup3, old, new, 0); #else return my_syscall2(__NR_dup2, old, new); #endif } I guess I fixed it in a hurry and forgot to upstream it. I hate doing that :-( I'm going to push an update then. On a quick glance I'm seeing that I addressed dup2() using dup3(), fork() using clone(), getpgrp() using getpgid(), and poll() using ppoll(). > ... we can deal with that by always returning -ENOSYS for unimplemented > syscalls, e.g. > > static __attribute__((unused)) > int sys_dup2(int old, int new) > { > #ifdef __NR_dup2 > return my_syscall2(__NR_dup2, old, new); > #else > return -ENOSYS; > #endif > } I didn't want to do that because that would break userland which needs dup2(), hence the mapping to dup3 instead: static __attribute__((unused)) int sys_dup2(int old, int new) { #ifdef __NR_dup3 return my_syscall3(__NR_dup3, old, new, 0); #else return my_syscall2(__NR_dup2, old, new); #endif } > I can spin a patch fixing up all the relevant syscalls, if you'd like? I shouldn't need since these are already fixed in my tree. At first glance the equivalent of the following commits is missing from the kernel version: https://github.com/wtarreau/nolibc/commit/2379f25073f906d0bad22c48566fcffee8fc9cde https://github.com/wtarreau/nolibc/commit/fd5272ec2c66e6f5b195d41c9c8978f58eb74668 https://github.com/wtarreau/nolibc/commit/47cc42a79c92305f4f8bc02fb28628a4fdd63eaa https://github.com/wtarreau/nolibc/commit/d2dc42fd614991c741dfdc8b984864fa3cf64c8e https://github.com/wtarreau/nolibc/commit/800f75c13ede49097325f82a4cca3515c44a7939 However I'm interested in knowing if the latest version fixes everything for you or not : https://raw.githubusercontent.com/wtarreau/nolibc/master/nolibc.h > [...] > > > ubuntu@ubuntu:~$ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)' > > #define __NR_clone 220 > > #define __NR_dup3 24 > > > > Do you have these ones with your more recent includes ? Or are these wrong > > again ? > > Those are correct (and all the syscall numbers in unistd.h should be > correct so long as you don't erroneously set the __ARCH_WANT_* flags): > > [mark@gravadlaks:~/src/linux]% gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include tools/include/nolibc/nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)' > #define __NR_clone 220 > #define __NR_dup3 24 OK thanks! I will retry here without setting those. I'm pretty sure I needed these ones to find the __NR_* values but it's possible that it was before I had the alternate ones and that these are finally not nedeed at all (which I would prefer as these are ugly). > There's still some latent issue when using nolibc (compared to using > glibc) where the init process never seems to exit, but that looks to be > orthogonal to the syscall numbering issue -- I'm currently digging into > that. OK! Usually for me it does as in my preinit (which uses nolibc), if I exit I instantly get a kernel panic. In addition if I launch it after boot, it immediately exits and shows no issue. But maybe you're observing an artefact related to something else (process session, opened FD or anything else maybe). I'll send an update ASAP, likely this evening. Many thanks for digging into this, and sorry for this mess, as I was absolutely certain it was up to date :-( Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 14:25 ` Willy Tarreau @ 2021-01-20 14:37 ` Mark Rutland 2021-01-20 14:54 ` Mark Rutland 1 sibling, 0 replies; 14+ messages in thread From: Mark Rutland @ 2021-01-20 14:37 UTC (permalink / raw) To: Willy Tarreau; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Wed, Jan 20, 2021 at 03:25:00PM +0100, Willy Tarreau wrote: > On Wed, Jan 20, 2021 at 01:45:11PM +0000, Mark Rutland wrote: > > There's still some latent issue when using nolibc (compared to using > > glibc) where the init process never seems to exit, but that looks to be > > orthogonal to the syscall numbering issue -- I'm currently digging into > > that. > > OK! Usually for me it does as in my preinit (which uses nolibc), if I > exit I instantly get a kernel panic. In addition if I launch it after > boot, it immediately exits and shows no issue. But maybe you're observing > an artefact related to something else (process session, opened FD or > anything else maybe). Luckily this was nothing to do with nolibc -- the build system wasn't regenerating the initramfs to use the correctly-compiled init, and things were blowing up because the kernel couldn't find an init process. With that regenerated it worked as intended. I'll reply separately for the rest of the nolibc bits shortly. Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 14:25 ` Willy Tarreau 2021-01-20 14:37 ` Mark Rutland @ 2021-01-20 14:54 ` Mark Rutland 2021-01-20 15:02 ` Willy Tarreau 1 sibling, 1 reply; 14+ messages in thread From: Mark Rutland @ 2021-01-20 14:54 UTC (permalink / raw) To: Willy Tarreau; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Wed, Jan 20, 2021 at 03:25:00PM +0100, Willy Tarreau wrote: > On Wed, Jan 20, 2021 at 01:45:11PM +0000, Mark Rutland wrote: > > > Ah that's very interesting indeed because actually these ones should > > > only be used when __NR_dup3 or __NR_clone are not defined. Thus I wanted > > > to check the definitions that were reported in your error output but > > > actually what was needed was to figure whether the correct ones were > > > present, and they are, here on my machine (and yes I agree that in this > > > case the dup2/fork above are bofus): > > > > The issue is that even if a function is unused, the compiler still has > > to parse and compile the code, so where __NR_dup2 is not defined, we'll > > get a build error for: > > > > static __attribute__((unused)) > > int sys_dup2(int old, int new) > > { > > return my_syscall2(__NR_dup2, old, new); > > } > > For sure but this is supposed to be used only when __NR_dup3 is not > defined. Ah now I understand where my mistake is: after it built > successfully for me I inspected the most recent tree which has these > in place. Sorry for being stupid! > > In my local tree it's defined like this: > > static __attribute__((unused)) > int sys_dup2(int old, int new) > { > #ifdef __NR_dup3 > return my_syscall3(__NR_dup3, old, new, 0); > #else > return my_syscall2(__NR_dup2, old, new); > #endif > } Ah, much better! For robustness, I think it would be worth doing: static __attribute__((unused)) int sys_dup2(int old, int new) { #if defined(__NR_dup3) return my_syscall3(__NR_dup3, old, new, 0); #elif defined(__NR_dup2) return my_syscall2(__NR_dup2, old, new); #else #error Cannot implement dup2 #endif } ... and getting rid of the ARCH_WANT_* definitions (which are never legitimate for userspace to set). That way, there's no risk that we accidentally use a bogus syscall number in future. Where the kernel does implement a syscall, it will have done whatever is necessary to expose the corresponding __NR_<syscall> to userspace without userspace needing to define anything. > I didn't want to do that because that would break userland which needs > dup2(), hence the mapping to dup3 instead: > > static __attribute__((unused)) > int sys_dup2(int old, int new) > { > #ifdef __NR_dup3 > return my_syscall3(__NR_dup3, old, new, 0); > #else > return my_syscall2(__NR_dup2, old, new); > #endif > } Sure, makes sense, though as above it might be worth adding an explicit check for the fallback syscall number. > I shouldn't need since these are already fixed in my tree. At first glance > the equivalent of the following commits is missing from the kernel version: > > https://github.com/wtarreau/nolibc/commit/2379f25073f906d0bad22c48566fcffee8fc9cde > https://github.com/wtarreau/nolibc/commit/fd5272ec2c66e6f5b195d41c9c8978f58eb74668 > https://github.com/wtarreau/nolibc/commit/47cc42a79c92305f4f8bc02fb28628a4fdd63eaa > https://github.com/wtarreau/nolibc/commit/d2dc42fd614991c741dfdc8b984864fa3cf64c8e > https://github.com/wtarreau/nolibc/commit/800f75c13ede49097325f82a4cca3515c44a7939 > > However I'm interested in knowing if the latest version fixes everything > for you or not : > > https://raw.githubusercontent.com/wtarreau/nolibc/master/nolibc.h I replaced my in-tree copy with that, and it built without issues, and the tests ran correctly. > OK thanks! I will retry here without setting those. I'm pretty sure I > needed these ones to find the __NR_* values but it's possible that it > was before I had the alternate ones and that these are finally not > nedeed at all (which I would prefer as these are ugly). Great! I reckon they're not needed at all so long as usage of each __NR_* is suitably guarded (such as above). If you do spot issues when removing ARCH_WANT_*, I'm happy to take a look, and/or to test patches handling any fallout. Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 14:54 ` Mark Rutland @ 2021-01-20 15:02 ` Willy Tarreau 2021-01-21 3:50 ` Willy Tarreau 0 siblings, 1 reply; 14+ messages in thread From: Willy Tarreau @ 2021-01-20 15:02 UTC (permalink / raw) To: Mark Rutland; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Wed, Jan 20, 2021 at 02:54:47PM +0000, Mark Rutland wrote: > On Wed, Jan 20, 2021 at 03:25:00PM +0100, Willy Tarreau wrote: > > On Wed, Jan 20, 2021 at 01:45:11PM +0000, Mark Rutland wrote: > > > > Ah that's very interesting indeed because actually these ones should > > > > only be used when __NR_dup3 or __NR_clone are not defined. Thus I wanted > > > > to check the definitions that were reported in your error output but > > > > actually what was needed was to figure whether the correct ones were > > > > present, and they are, here on my machine (and yes I agree that in this > > > > case the dup2/fork above are bofus): > > > > > > The issue is that even if a function is unused, the compiler still has > > > to parse and compile the code, so where __NR_dup2 is not defined, we'll > > > get a build error for: > > > > > > static __attribute__((unused)) > > > int sys_dup2(int old, int new) > > > { > > > return my_syscall2(__NR_dup2, old, new); > > > } > > > > For sure but this is supposed to be used only when __NR_dup3 is not > > defined. Ah now I understand where my mistake is: after it built > > successfully for me I inspected the most recent tree which has these > > in place. Sorry for being stupid! > > > > In my local tree it's defined like this: > > > > static __attribute__((unused)) > > int sys_dup2(int old, int new) > > { > > #ifdef __NR_dup3 > > return my_syscall3(__NR_dup3, old, new, 0); > > #else > > return my_syscall2(__NR_dup2, old, new); > > #endif > > } > > Ah, much better! > > For robustness, I think it would be worth doing: > > static __attribute__((unused)) > int sys_dup2(int old, int new) > { > #if defined(__NR_dup3) > return my_syscall3(__NR_dup3, old, new, 0); > #elif defined(__NR_dup2) > return my_syscall2(__NR_dup2, old, new); > #else > #error Cannot implement dup2 > #endif > } Yep I'm fine with this. We'll probably adjust between return -ENOSYS or #error depending on the "criticity" of the syscalls. E.g. we can miss poll() for plenty of init stuff band be done with ENOSYS but missing dup is quickly going to cause trouble. > ... and getting rid of the ARCH_WANT_* definitions (which are never > legitimate for userspace to set). That way, there's no risk that we > accidentally use a bogus syscall number in future. Where the kernel does > implement a syscall, it will have done whatever is necessary to expose > the corresponding __NR_<syscall> to userspace without userspace needing > to define anything. I'm really willing to get rid of that crap, I hated it, I vaguely remember having gone through layers of glibc indirections when using the pre-processed asm/* and found this to be the only way to expose some of them. The fact that it's not needed for you is pretty much encouraging. I'll just retry on an older libc I've used a lot (2.18) to make sure I didn't overlook anything. > > However I'm interested in knowing if the latest version fixes everything > > for you or not : > > > > https://raw.githubusercontent.com/wtarreau/nolibc/master/nolibc.h > > I replaced my in-tree copy with that, and it built without issues, and > the tests ran correctly. Perfect, thanks for the quick feedback! > > OK thanks! I will retry here without setting those. I'm pretty sure I > > needed these ones to find the __NR_* values but it's possible that it > > was before I had the alternate ones and that these are finally not > > nedeed at all (which I would prefer as these are ugly). > > Great! I reckon they're not needed at all so long as usage of each > __NR_* is suitably guarded (such as above). > > If you do spot issues when removing ARCH_WANT_*, I'm happy to take a > look, and/or to test patches handling any fallout. Many thanks. I'm pretty confident that it's OK without them based on your feedback but if need I'll ask you for some advices. cheers, Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: rcutorture initrd/nolibc build on ARMv8? 2021-01-20 15:02 ` Willy Tarreau @ 2021-01-21 3:50 ` Willy Tarreau 0 siblings, 0 replies; 14+ messages in thread From: Willy Tarreau @ 2021-01-21 3:50 UTC (permalink / raw) To: Mark Rutland; +Cc: Paul E. McKenney, linux-kernel, valentin.schneider On Wed, Jan 20, 2021 at 04:02:23PM +0100, Willy Tarreau wrote: > > ... and getting rid of the ARCH_WANT_* definitions (which are never > > legitimate for userspace to set). That way, there's no risk that we > > accidentally use a bogus syscall number in future. Where the kernel does > > implement a syscall, it will have done whatever is necessary to expose > > the corresponding __NR_<syscall> to userspace without userspace needing > > to define anything. > > I'm really willing to get rid of that crap, I hated it, I vaguely > remember having gone through layers of glibc indirections when using > the pre-processed asm/* and found this to be the only way to expose > some of them. The fact that it's not needed for you is pretty much > encouraging. I'll just retry on an older libc I've used a lot (2.18) > to make sure I didn't overlook anything. I've now retested and can confirm that the only reason I included them by then was to have access to these (wrong) __NR_* definitions. Now with the fixed syscalls these ones are not needed anymore and I indeed only see the correct definitions, so I've removed these bogus definitions. I've completed my cleanup and tests, I'll send an updated patch set later today after I've carefully rediffed everything to synchronise the in-kernel version with the out-of-tree one. Cheers, Willy ^ permalink raw reply [flat|nested] 14+ messages in thread
* [tip: core/rcu] tools/nolibc: Remove incorrect definitions of __ARCH_WANT_* 2021-01-19 15:31 rcutorture initrd/nolibc build on ARMv8? Paul E. McKenney 2021-01-19 16:19 ` Willy Tarreau @ 2021-02-12 12:37 ` tip-bot2 for Willy Tarreau 1 sibling, 0 replies; 14+ messages in thread From: tip-bot2 for Willy Tarreau @ 2021-02-12 12:37 UTC (permalink / raw) To: linux-tip-commits Cc: Mark Rutland, Valentin Schneider, Willy Tarreau, Paul E. McKenney, x86, linux-kernel The following commit has been merged into the core/rcu branch of tip: Commit-ID: f65d7117785cb8ab04f1af55909807c7eb9ed30b Gitweb: https://git.kernel.org/tip/f65d7117785cb8ab04f1af55909807c7eb9ed30b Author: Willy Tarreau <w@1wt.eu> AuthorDate: Thu, 21 Jan 2021 08:20:29 +01:00 Committer: Paul E. McKenney <paulmck@kernel.org> CommitterDate: Thu, 21 Jan 2021 10:06:45 -08:00 tools/nolibc: Remove incorrect definitions of __ARCH_WANT_* The __ARCH_WANT_* definitions were added in order to support aarch64 when it was missing some syscall definitions (including __NR_dup2, __NR_fork, and __NR_getpgrp), but these __ARCH_WANT_* definitions were actually wrong because these syscalls do not exist on this platform. Defining these resulted in exposing invalid definitions, resulting in failures on aarch64. The missing syscalls were since implemented based on the newer ones (__NR_dup3, __NR_clone, __NR_getpgid) so these incorrect __ARCH_WANT_* definitions are no longer needed. Thanks to Mark Rutland for spotting this incorrect analysis and explaining why it was wrong. This is a port of nolibc's upstream commit 00b1b0d9b2a4 to the Linux kernel. Reported-by: Mark Rutland <mark.rutland@arm.com> Link: https://lore.kernel.org/lkml/20210119153147.GA5083@paulmck-ThinkPad-P72 Tested-by: Valentin Schneider <valentin.schneider@arm.com> Tested-by: Mark Rutland <mark.rutland@arm.com> [arm64] Signed-off-by: Willy Tarreau <w@1wt.eu> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> --- tools/include/nolibc/nolibc.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tools/include/nolibc/nolibc.h b/tools/include/nolibc/nolibc.h index 611d9d1..475d956 100644 --- a/tools/include/nolibc/nolibc.h +++ b/tools/include/nolibc/nolibc.h @@ -81,14 +81,6 @@ * */ -/* Some archs (at least aarch64) don't expose the regular syscalls anymore by - * default, either because they have an "_at" replacement, or because there are - * more modern alternatives. For now we'd rather still use them. - */ -#define __ARCH_WANT_SYSCALL_NO_AT -#define __ARCH_WANT_SYSCALL_NO_FLAGS -#define __ARCH_WANT_SYSCALL_DEPRECATED - #include <asm/unistd.h> #include <asm/ioctls.h> #include <asm/errno.h> ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-12 12:45 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-19 15:31 rcutorture initrd/nolibc build on ARMv8? Paul E. McKenney 2021-01-19 16:19 ` Willy Tarreau 2021-01-19 17:02 ` Mark Rutland 2021-01-19 17:16 ` Willy Tarreau 2021-01-19 17:43 ` Willy Tarreau 2021-01-20 12:07 ` Mark Rutland 2021-01-20 12:43 ` Willy Tarreau 2021-01-20 13:45 ` Mark Rutland 2021-01-20 14:25 ` Willy Tarreau 2021-01-20 14:37 ` Mark Rutland 2021-01-20 14:54 ` Mark Rutland 2021-01-20 15:02 ` Willy Tarreau 2021-01-21 3:50 ` Willy Tarreau 2021-02-12 12:37 ` [tip: core/rcu] tools/nolibc: Remove incorrect definitions of __ARCH_WANT_* tip-bot2 for 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).