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

[Andrea Righi <arighi@nvidia.com>]

Hi Tejun,

On Thu, Sep 17, 2026 at 09:19:07AM -1000, Tejun Heo wrote:
> Hello, Andrea.
> 
> This is an AI review. The series was built and the tests pass under vng here
> in lazy and full modes, with nohz_full on 2-3 and on 1-3. Three things worth
> fixing and some leftovers.
> 
> On Thu, Sep 17, 2026 at 09:02:53AM +0200, Andrea Righi wrote:
> > +enum kick_scenario {
> 
> exit_test.h and hotplug_test.h share such enums between the BPF and user
> sides. The same would work here and for nohz_phase. run_invalid() and
> is_trace_scenario() depend on the two copies staying in order.
> 
> > +#ifdef HAVE_GENHDR
> > +#include "autoconf.h"
> > +#endif
> 
> Nothing uses CONFIG_ in kick.c anymore.
> 
> > +static struct victim spawn_victim(int cpu)
> 
> Same as spawn_gated_worker() in nohz_tick.c except for sched_setscheduler().
> util.c could hold one copy.
> 
> > +	file = fopen("/sys/kernel/debug/sched/preempt", "r");
> > +	if (!file)
> > +		return -1;
> 
> That file only exists with PREEMPT_DYNAMIC, and needs debugfs mounted. Every
> static kernel takes the INFO path and passes without comparing, including
> static PREEMPT_LAZY, which is the default on arm64, riscv, powerpc, s390 and
> loongarch where PREEMPT_DYNAMIC defaults off. uname can't tell either, as
> PREEMPT_LAZY selects PREEMPT_BUILD. /proc/config.gz covers static kernels
> when it's there. Otherwise skipping would be more honest than passing.
> 
> > +	SCX_EQ(both.resched_tif, immediate.resched_tif);
> > +	SCX_EQ(lazy_wait.resched_tif, immediate.resched_tif);
> 
> Outside lazy mode the lazy TIF is the immediate one, so these and the
> override checks in run_tick() hold whatever kick_one_cpu() decided. Maybe
> skip or note it when the mode isn't lazy.
> 
> > +	case NOHZ_PHASE_LAZY_ENQ:
> > +	case NOHZ_PHASE_LAZY_KICK:
> > +		dsq_id = SCX_DSQ_LOCAL;
> > +		slice = p->pid == victim_pid ? SCX_SLICE_INF : SCX_SLICE_DFL;
> 
> The victim gets an infinite slice on every enqueue, including the one after
> the lazy preemption puts it back. In the kick phase the challenger is stopped
> first; if its slice runs out before the SIGKILL lands, it's re-enqueued
> behind the victim and never runs again, waitpid() blocks, and the 5s watchdog
> aborts the scheduler, which then fails the test at check_exit. The enqueue
> phase is safe as the challenger re-inserts at the head. Stopping the victim
> first avoids it. ENQ_BOTH in kick.c has the same shape with the global DSQ
> and the default 30s watchdog, a stall rather than a failure there.
> 
> > +static bool wait_for_tick_stop(const u64 *counter, int timeout_ms)
> 
> Neither this nor wait_for_counter() checks uei.kind, so a watchdog abort
> mid-phase reports as no progress.
> 
> > +	if (!start_gated_worker(&challenger) ||
> > +	    !wait_for_counter(&skel->bss->nr_lazy_enq_running, 1,
> > +			      PHASE_TIMEOUT_MS)) {
> > +		SCX_ERR("Lazy enqueue made no progress on CPU %d", ctx->test_cpu);
> 
> This passes even without the tick dependency restore. sched_tick_remote()
> runs task_tick_scx() once a second on the tick-stopped CPU, and as
> nohz_tick_ops doesn't set SCX_OPS_LAZY_SLICE_EXPIRY that path does
> resched_curr() with an IPI, so the challenger runs within a second either
> way. Setting the flag on nohz_tick_ops keeps the remote tick lazy, so a
> missing restore shows up as the timeout. Both phases only test anything in
> lazy mode.

Agree to all of the above. I'll send a new version with all of these fixed.

Thanks,
-Andrea