| Message ID | 20200327093620.GB1223497@xps-13 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kselftest/runner: avoid using timeout when timeout is disabled | expand |
On Fri, Mar 27, 2020 at 10:36:20AM +0100, Andrea Righi wrote: > Avoid using /usr/bin/timeout unnecessarily if timeout is set to 0 > (disabled) in the "settings" file for a specific test. That seems to be a reasonable optimization, sure. > NOTE: without this change (and adding timeout=0 in the corresponding > settings file - tools/testing/selftests/seccomp/settings) the > seccomp_bpf selftest is always failing with a timeout event during the > syscall_restart step. This, however, is worrisome. I think there is something else wrong here. I will investigate why the output of seccomp_bpf is weird when running under the runner scripts. Hmmm. The output looks corrupted... -Kees > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > --- > tools/testing/selftests/kselftest/runner.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh > index e84d901f8567..2cd3c8def0f6 100644 > --- a/tools/testing/selftests/kselftest/runner.sh > +++ b/tools/testing/selftests/kselftest/runner.sh > @@ -32,7 +32,7 @@ tap_prefix() > tap_timeout() > { > # Make sure tests will time out if utility is available. > - if [ -x /usr/bin/timeout ] ; then > + if [ -x /usr/bin/timeout ] && [ $kselftest_timeout -gt 0 ] ; then > /usr/bin/timeout "$kselftest_timeout" "$1" > else > "$1" > -- > 2.25.1 >
On Fri, Mar 27, 2020 at 12:28:05PM -0700, Kees Cook wrote: > On Fri, Mar 27, 2020 at 10:36:20AM +0100, Andrea Righi wrote: > > Avoid using /usr/bin/timeout unnecessarily if timeout is set to 0 > > (disabled) in the "settings" file for a specific test. > > That seems to be a reasonable optimization, sure. > > > NOTE: without this change (and adding timeout=0 in the corresponding > > settings file - tools/testing/selftests/seccomp/settings) the > > seccomp_bpf selftest is always failing with a timeout event during the > > syscall_restart step. > > This, however, is worrisome. I think there is something else wrong here. > I will investigate why the output of seccomp_bpf is weird when running > under the runner scripts. Hmmm. The output looks corrupted... Running seccomp_bpf directly (without using runner.sh) shows this error: $ sudo ./tools/testing/selftests/seccomp/seccomp_bpf ... seccomp_bpf.c:2839:global.syscall_restart:Expected true (1) == WIFSTOPPED(status) (0) global.syscall_restart: Test terminated by assertion Instead, running it via /usr/bin/timeout (with timeout disabled): $ sudo /usr/bin/timeout 0 ./tools/testing/selftests/seccomp/seccomp_bpf ... [ RUN ] TSYNC.siblings_fail_prctl It gets stuck here forever, basically it's during the execution of syscall_restart(). I'll investigate more later. Thanks, -Andrea
On Fri, Mar 27, 2020 at 12:28:05PM -0700, Kees Cook wrote: > On Fri, Mar 27, 2020 at 10:36:20AM +0100, Andrea Righi wrote: > > Avoid using /usr/bin/timeout unnecessarily if timeout is set to 0 > > (disabled) in the "settings" file for a specific test. > > That seems to be a reasonable optimization, sure. > > > NOTE: without this change (and adding timeout=0 in the corresponding > > settings file - tools/testing/selftests/seccomp/settings) the > > seccomp_bpf selftest is always failing with a timeout event during the > > syscall_restart step. > > This, however, is worrisome. I think there is something else wrong here. > I will investigate why the output of seccomp_bpf is weird when running > under the runner scripts. Hmmm. The output looks corrupted... > > -Kees Hi Kees, a quick update on this. After further investigation Cascardo (added in cc) found that the culprit of this issue was the usage of nanosleep() vs clock_nanosleep() in glibc. He already sent a fix for this: https://lkml.org/lkml/2020/4/8/968 Without this we are getting the following error: seccomp_bpf.c:2839:global.syscall_restart:Expected true (1) == WIFSTOPPED(status) (0) # global.syscall_restart: Test terminated by assertion I still think my timeout optimization patch can be useful, but for this particular problem we should definitely apply Cascardo's fix. Thanks, -Andrea > > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > --- > > tools/testing/selftests/kselftest/runner.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh > > index e84d901f8567..2cd3c8def0f6 100644 > > --- a/tools/testing/selftests/kselftest/runner.sh > > +++ b/tools/testing/selftests/kselftest/runner.sh > > @@ -32,7 +32,7 @@ tap_prefix() > > tap_timeout() > > { > > # Make sure tests will time out if utility is available. > > - if [ -x /usr/bin/timeout ] ; then > > + if [ -x /usr/bin/timeout ] && [ $kselftest_timeout -gt 0 ] ; then > > /usr/bin/timeout "$kselftest_timeout" "$1" > > else > > "$1" > > -- > > 2.25.1 > > > > -- > Kees Cook
diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index e84d901f8567..2cd3c8def0f6 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -32,7 +32,7 @@ tap_prefix() tap_timeout() { # Make sure tests will time out if utility is available. - if [ -x /usr/bin/timeout ] ; then + if [ -x /usr/bin/timeout ] && [ $kselftest_timeout -gt 0 ] ; then /usr/bin/timeout "$kselftest_timeout" "$1" else "$1"
Avoid using /usr/bin/timeout unnecessarily if timeout is set to 0 (disabled) in the "settings" file for a specific test. NOTE: without this change (and adding timeout=0 in the corresponding settings file - tools/testing/selftests/seccomp/settings) the seccomp_bpf selftest is always failing with a timeout event during the syscall_restart step. Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- tools/testing/selftests/kselftest/runner.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)