Re: [PATCH v3] sched: Restart fair hrtick after same-task repicks

[Shubhang]

Hi Zhan,

Thanks for the review and testing. Yes, skipping 
put_prev_task_*() does not distinguish fair from DL.

For fair class, pick_task_fair() calls update_curr_eevdf() before 
selecting the current task again. This refreshes the task entity used 
by hrtick_start_fair(), including with group scheduling. When that update
observes an expired slice, it advances the entity's virtual deadline 
before hrtick_start_fair() computes the next expiry.

pick_task_dl() has no corresponding picker-side runtime update, so
p->dl.runtime can be stale on SNT_REPICK.

I will clarify this in the changelog, add a comment on the fair repick 
path, and document the SNT_NORMAL and SNT_PICK meanings.

I also rebuilt with CONFIG_SCHED_CLASS_EXT=y and it compiled
cleanly on my side.

Thanks,
Shubhang Kaushik

On Wed, 16 Sep 2026, Zhan Xusheng wrote:

> On Tue, Sep 15, 2026 at 06:13:18PM -0700, Shubhang Kaushik (Ampere) wrote:
> > Do not restart the DL hrtick for SNT_REPICK. put_prev_task_dl() is
> > skipped for next == prev, so dl_se->runtime may be stale.
>
> put_prev_task_fair() is skipped for next == prev too, so as written this
> reason applies to fair as well.
>
> What separates them is that rq->cfs.curr holds the task entity rather
> than the root-level one, so pick_task_fair()'s update_curr_eevdf()
> refreshes exactly the entity hrtick_start_fair() reads, group scheduling
> or not.  pick_task_dl() has no counterpart.  update_deadline() then exits
> with deadline > vruntime, which is what keeps the repick out of
>
> 	if ((s64)vdelta < 0) {
> 		if (task_current_donor(rq, p))
> 			resched_curr(rq);
> 		return;
> 	}
>
> That branch arms nothing, and __schedule() clears TIF_NEED_RESCHED just
> after pick_next_task(), so a repick reaching it would come out with no
> preemption point at all, which is the state you are fixing.  Two lines in
> the changelog, or a comment at the repick label, would stop that
> invariant from being broken silently from three functions away.
>
> > +enum snt_e {
> > + SNT_NORMAL,
> > + SNT_PICK,
> > + SNT_REPICK,
> > +};
>
> Three values, six classes reading them, no comment.  SNT_REPICK explains
> itself; NORMAL versus PICK does not.  At the call sites they mean
> "through set_next_task()" and "the running task changed".
>
> I checked the conversion with the compiler rather than by grepping:
> reverting any one class to bool fails on the incompatible function
> pointer, and rt, stop, fair, idle and deadline build W=1 clean.
> ext/ext.c I could not compile, SCHED_CLASS_EXT wants DEBUG_INFO_BTF and
> pahole is broken here.  first is unused in set_next_task_scx()'s body so
> the rename is safe, but someone with BTF should build it.
>
> Booted 4 CPUs, HRTICK on, four spinners under panic_on_warn=1: clean.  I
> did not reproduce the latency numbers.
>
> Reviewed-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>