Re: [PATCH 1/2] sched_ext: Add lazy preemption support

[Tejun Heo <tj@kernel.org>]

Hello, Andrea.

This is an AI review. The series was built and the selftests pass under vng
here, and the kick and tick paths were traced and look correct. A few things
to fix up.

On Tue, Sep 15, 2026 at 09:45:12PM +0200, Andrea Righi wrote:
> +	 * modified afterwards with scx_bpf_task_set_slice_expiry().
> +	 */
> +	bool			slice_expires_lazy;

This adds 8 bytes to task_struct for a bool. Put it next to @disallow, there
are three bytes of padding after it.

> +	 * A task in user space still reschedules on the way back from the tick; a
> +	 * task in the kernel runs on to its next return to user space or to the
> +	 * next tick, which promotes the request. No effect on kernels without
> +	 * lazy preemption. Rescheduling while disabling stays immediate.

Lazy is a runtime mode under PREEMPT_DYNAMIC, so "kernels without lazy
preemption" isn't right. And what forces immediate rescheduling is bypassing,
not disabling.

> +static void scx_resched_curr_lazy(struct rq *rq)
> +{
> +	if (rq->scx.flags & SCX_RQ_CAN_STOP_TICK) {
> +		rq->scx.flags &= ~SCX_RQ_CAN_STOP_TICK;
> +		if (rq->clock_update_flags < RQCF_UPDATED)
> +			update_rq_clock(rq);
> +		update_other_load_avgs(rq);
> +		sched_update_tick_dependency(rq);
> +	}
> +
> +	resched_curr_lazy(rq);
> +}

Setting the TIF bit before restoring the tick dependency would let the tick
IPI's return to user space serve the reschedule instead of waiting for the
next tick. Not critical.

> -/* common to both forms: only scx.disallow is writable */
> +/* common to both forms: only the fields below are writable */

Nothing else became writable. Drop the hunk.

> -	raw_spin_rq_lock_irqsave(rq, flags);
> +	rq_lock_irqsave(rq, &rf);

Please explain this in the description. Pinning clears RQCF_UPDATED, which is
what makes the clock_update_flags test in scx_resched_curr_lazy() do anything
on this path.

> +	if (unlikely(flags & ~(SCX_KICK_IDLE | SCX_KICK_PREEMPT | SCX_KICK_WAIT |
> +			       SCX_KICK_PREEMPT_LAZY))) {
> +		scx_error(sch, "invalid kick flags 0x%llx", flags);
> +		return;
> +	}

Unknown flags were silently ignored before, so this is a behavior change and
the description should say so. The SCX_KICK_IDLE check already existed, it
just returns early now.

> + *            - SCX_ENQ_PREEMPT and SCX_ENQ_PREEMPT_LAZY inserts
> + *            - SCX_KICK_PREEMPT and SCX_KICK_PREEMPT_LAZY kicks

insert, kick.

> +/*
> + * v7.4: scx_bpf_task_set_slice_expiry() added to enforce sub-scheduler task
> + * ownership. Preserve until v7.7.
> + */

This is copied from the block above and doesn't apply. The kfunc is new with
the feature, there was no field write to replace.

> +#define HAVE_SCX_KICK_PREEMPT_LAZY
>  #define HAVE_SCX_KICK_WAIT

The autogen headers follow definition order, so SCX_KICK_PREEMPT_LAZY goes
after SCX_KICK_WAIT in all four. Please regenerate them instead of editing.

Thanks.

--
tejun