Re: [PATCH v2 6/6] selftests/sched_ext: Report reload_loop attach failures

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: flaky reload_loop test due to strict iteration limit
--

commit 8a9da64f2647d703dd026bef834ba09325868dbc
Author: Tao Cui <cuitao@kylinos.cn>

selftests/sched_ext: Report reload_loop attach failures

This commit updates the reload_loop test to record the number of EBUSY
attach failures per thread. It fails the test if an unexpected errno is
encountered or if any thread fails to attach the link on every iteration.

> diff --git a/tools/testing/selftests/sched_ext/reload_loop.c b/tools/testing/selftests/sched_ext/reload_loop.c
> index a00f3964dba05..7d8880fa1a032 100644
> --- a/tools/testing/selftests/sched_ext/reload_loop.c
> +++ b/tools/testing/selftests/sched_ext/reload_loop.c

[ ... ]

> @@ -31,22 +38,33 @@ static enum scx_test_status setup(void **ctx)
>  
>  static void *do_reload_loop(void *arg)
>  {
> -	u32 i;
> +	u32 i, failures = 0;
>  
> -	for (i = 0; i < 1024 && !force_exit; i++) {
> +	for (i = 0; i < RELOAD_ITERS && !force_exit; i++) {
>  		struct bpf_link *link;
>  
> +		errno = 0;
>  		link = bpf_map__attach_struct_ops(skel->maps.maximal_ops);
> -		if (link)
> -			bpf_link__destroy(link);
> +		if (!link) {
> +			int e = errno;
> +
> +			if (e != EBUSY)
> +				__atomic_store_n(&unexpected_errno, e,
> +						 __ATOMIC_RELAXED);
> +			failures++;
> +			continue;

[Severity: Medium]
Because this loop does not yield the CPU (e.g., via sched_yield() or usleep())
when receiving -EBUSY, can a thread rapidly spin through all 1024 iterations
while the other thread holds the link or is preempted?

> +		}
> +
> +		bpf_link__destroy(link);
>  	}
>  
> -	return NULL;
> +	return (void *)(unsigned long)failures;
>  }

[ ... ]

> @@ -60,7 +78,18 @@ static enum scx_test_status run(void *ctx)
>  	}
>  
>  	SCX_FAIL_IF(pthread_join(threads[0], &ret), "thread 0 failed");
> +	failures[0] = (unsigned long)ret;
> +
>  	SCX_FAIL_IF(pthread_join(threads[1], &ret), "thread 1 failed");
> +	failures[1] = (unsigned long)ret;
> +
> +	SCX_FAIL_IF(unexpected_errno, "unexpected attach errno %d",
> +		    unexpected_errno);
> +
> +	for (i = 0; i < RELOAD_THREADS; i++)
> +		SCX_FAIL_IF(failures[i] == RELOAD_ITERS,
> +			    "thread %d: every attach failed (%lu EBUSY)",
> +			    i, failures[i]);

[Severity: Medium]
Could this assertion cause spurious test failures on loaded systems?

Since 1024 syscalls returning -EBUSY execute extremely fast, if one thread
successfully attaches the link and is temporarily preempted before destroying
it, the other thread can easily exhaust its RELOAD_ITERS and hit this failure
condition.

>  
>  	return SCX_TEST_PASS;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014131325.793640-1-cui.tao@linux.dev?part=6