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

[Andrea Righi <arighi@nvidia.com>]

On Mon, Sep 14, 2026 at 09:01:55AM +0000, sashiko-bot@kernel.org wrote:
...
> > +	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?

Although this is a kselftest intended for a test environment, it may still run
on a persistent test host, and these workers spin indefinitely. Since I'm
already planning to send a v3, I'll fix this as well.

> 
> > +	}
> [ ... ]
> > 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?

We can snapshot nr_lazy_kick_running before starting the new victim and wait for
the counter to increase by one. I'll also fix this on v3.

-Andrea