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

[Tejun Heo <tj@kernel.org>]

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.

Thanks.

-- 
tejun