Re: [PATCH 2/2] selftests/sched_ext: Add lazy preemption tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing PR_SET_PDEATHSIG in spawned victim tasks leads to orphaned tasks spinning at 100% CPU.
- [Medium] The wait condition for `nr_lazy_victim_running` hardcodes a target value without accounting for intermediate preemptions, causing test flakes.
--

commit 027a94072433d755727d58cbb128013210dcb223
Author: Andrea Righi <arighi@nvidia.com>

selftests/sched_ext: Add lazy preemption tests

This commit adds trace-based coverage for immediate and lazy preemption. It
tests kick requests individually and combined, and extends the NO_HZ_FULL
test with infinite-slice victims.

> diff --git a/tools/testing/selftests/sched_ext/kick.c b/tools/testing/selftests/sched_ext/kick.c
> new file mode 100644
> index 0000000000000..3fa112a1de607
> --- /dev/null
> +++ b/tools/testing/selftests/sched_ext/kick.c
[ ... ]
> +static struct victim spawn_victim(int cpu)
> +{
[ ... ]
> +	victim.pid = fork();
> +	if (!victim.pid) {
> +		cpu_set_t mask;
> +
> +		close(ready[0]);
> +		close(start[1]);
> +		CPU_ZERO(&mask);
> +		CPU_SET(cpu, &mask);
> +		if (sched_setaffinity(0, sizeof(mask), &mask))
> +			_exit(1);
> +		if (write(ready[1], &byte, 1) != 1)
> +			_exit(1);
> +		close(ready[1]);
> +		if (read(start[0], &byte, 1) != 1)
> +			_exit(1);
> +		close(start[0]);
> +		for (;;)
> +			asm volatile("" ::: "memory");

[Severity: Medium]
Will this lead to orphaned tasks spinning at 100% CPU if the test runner
crashes?

If the test is forcefully terminated or crashes before stop_victim() is
called, the child processes will be orphaned and continue their infinite loop
indefinitely.

Should prctl(PR_SET_PDEATHSIG, SIGKILL) be used here, similar to how it is
used in spawn_gated_worker() elsewhere in this patchset?

> +	}
[ ... ]
> diff --git a/tools/testing/selftests/sched_ext/nohz_tick.c b/tools/testing/selftests/sched_ext/nohz_tick.c
> index 028f54391c2ca..a5266a5412935 100644
> --- a/tools/testing/selftests/sched_ext/nohz_tick.c
> +++ b/tools/testing/selftests/sched_ext/nohz_tick.c
[ ... ]
> +	/* Repeat with a lazy kick delivered from a housekeeping CPU. */
> +	__atomic_store_n(&skel->bss->phase, NOHZ_PHASE_LAZY_KICK,
> +			 __ATOMIC_RELEASE);
> +	victim = spawn_gated_worker(ctx->test_cpu);
> +	challenger = spawn_gated_worker(ctx->test_cpu);
> +	trigger = spawn_gated_worker(ctx->housekeeping_cpu);
> +	if (victim.pid < 0 || challenger.pid < 0 || trigger.pid < 0) {
> +		SCX_ERR("Failed to spawn lazy-kick workers");
> +		goto out;
> +	}
> +	skel->bss->victim_pid = victim.pid;
> +	skel->bss->challenger_pid = challenger.pid;
> +	skel->bss->trigger_pid = trigger.pid;
> +	if (!start_gated_worker(&victim) ||
> +	    !wait_for_counter(&skel->bss->nr_lazy_victim_running, 2,
> +			      PHASE_TIMEOUT_MS)) {

[Severity: Medium]
Could this wait condition cause the test to flake by relying on an absolute
counter value?

During the teardown of the previous phase (NOHZ_PHASE_LAZY_ENQ), stopping the
challenger can allow the original victim to be rescheduled, incrementing
nr_lazy_victim_running to 2 before this new phase begins.

If that happens, wait_for_counter() will return immediately here, and the test
will proceed to launch the new challenger before the new victim is running. If
the new challenger is scheduled before the new victim, the subsequent check on
nr_lazy_kick_running can fail.

Should this wait condition account for intermediate preemptions instead of
hardcoding 2?

> +		SCX_ERR("Lazy-kick victim was not scheduled");
> +		goto out;
> +	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914084955.1798562-1-arighi@nvidia.com?part=2