Re: [PATCH bpf-next] selftests/bpf: Bump internal send_signal/send_signal_tracepoint timeout

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Jul 27, 2022 at 06:29:55PM +0000, Daniel Müller wrote:
> The send_signal/send_signal_tracepoint is pretty flaky, with at least
> one failure in every ten runs on a few attempts I've tried it:
>   > test_send_signal_common:PASS:pipe_c2p 0 nsec
>   > test_send_signal_common:PASS:pipe_p2c 0 nsec
>   > test_send_signal_common:PASS:fork 0 nsec
>   > test_send_signal_common:PASS:skel_open_and_load 0 nsec
>   > test_send_signal_common:PASS:skel_attach 0 nsec
>   > test_send_signal_common:PASS:pipe_read 0 nsec
>   > test_send_signal_common:PASS:pipe_write 0 nsec
>   > test_send_signal_common:PASS:reading pipe 0 nsec
>   > test_send_signal_common:PASS:reading pipe error: size 0 0 nsec
>   > test_send_signal_common:FAIL:incorrect result unexpected incorrect result: actual 48 != expected 50
>   > test_send_signal_common:PASS:pipe_write 0 nsec
>   > #139/1   send_signal/send_signal_tracepoint:FAIL
> 
> The reason does not appear to be a correctness issue in the strict
> sense. Rather, we merely do not receive the signal we are waiting for
> within the provided timeout.
> Let's bump the timeout by a factor of ten. With that change I have not
> been able to reproduce the failure in 150+ iterations. I am also sneaking
> in a small simplification to the test_progs test selection logic.
> 
> Signed-off-by: Daniel Müller <deso@posteo.net>

I reproduced the fail, can't reproduce anymore with the fix

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> ---
>  tools/testing/selftests/bpf/prog_tests/send_signal.c | 2 +-
>  tools/testing/selftests/bpf/test_progs.c             | 7 ++-----
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> index d71226e..d63a20 100644
> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> @@ -64,7 +64,7 @@ static void test_send_signal_common(struct perf_event_attr *attr,
>  		ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read");
>  
>  		/* wait a little for signal handler */
> -		for (int i = 0; i < 100000000 && !sigusr1_received; i++)
> +		for (int i = 0; i < 1000000000 && !sigusr1_received; i++)
>  			j /= i + j + 1;
>  
>  		buf[0] = sigusr1_received ? '2' : '0';
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index c639f2e..3561c9 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1604,11 +1604,8 @@ int main(int argc, char **argv)
>  		struct prog_test_def *test = &prog_test_defs[i];
>  
>  		test->test_num = i + 1;
> -		if (should_run(&env.test_selector,
> -				test->test_num, test->test_name))
> -			test->should_run = true;
> -		else
> -			test->should_run = false;
> +		test->should_run = should_run(&env.test_selector,
> +					      test->test_num, test->test_name);
>  
>  		if ((test->run_test == NULL && test->run_serial_test == NULL) ||
>  		    (test->run_test != NULL && test->run_serial_test != NULL)) {
> -- 
> 2.30.2
>