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

[Shubhang]

Hi Zhan,

Thanks for the review.

My original intent behind h_nr_runnable == h_nr_queued was to avoid
rearming an hrtick merely because delayed dequeued entities remain
queued. However, that condition also suppresses the rearm when there is
a real competing runnable entity plus an unrelated delayed entity. Which 
makes it be restrictive. I will change this to h_nr_runnable > 1 and add
a mixed workload test that exercises delayed dequeue.

Ack, I will also rework the rq flag. The reason I put the rearm on the
same task repick path was to retain the normal schedule side deferred
hrtick programming, rather than start a timer directly from the hrtick
callback. But the extra state does not appear necessary: the same task
path can check whether fair hrtick is enabled, inactive, and has another
runnable fair entity, then rearm directly.

Thanks,
Shubhang Kaushik

On Wed, 26 Aug 2026, Zhan Xusheng wrote:

> From: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
>
> On Thu, 13 Aug 2026 14:23:48 -0700, Shubhang Kaushik (Ampere) wrote:
> > +		rq->hrtick_rearm_fair = hrtick_enabled_fair(rq) &&
> > +				rq->cfs.h_nr_runnable > 1 &&
> > +				rq->cfs.h_nr_runnable == rq->cfs.h_nr_queued;
>
> The last term switches the fix off whenever anything on the rq sits in
> delayed dequeue.  set_delayed() decrements h_nr_runnable and leaves
> h_nr_queued alone (kernel/sched/fair.c:6398), clear_delayed() puts it
> back (6418), so the two differ exactly while a delay-dequeued entity is
> present.  With DELAY_DEQUEUE that is routine, and it says nothing about
> whether the running task still needs its slice bounded.
>
> Your test cannot show that either way: two CPU-bound tasks pinned to one
> CPU never sleep, so nothing is ever delay-dequeued there and the term is
> true for the whole run.  Adding a third task that sleeps in a loop should
> bring the missed hrtick back while the term is false.
>
> If the intent is only to skip rqs whose other queued entities are not
> competing for the CPU, h_nr_runnable > 1 already says that by itself.
>
> > +static inline void hrtick_rearm_fair(struct rq *rq, struct task_struct *p)
> > +{
> > +	if (rq->hrtick_rearm_fair)
> > +		__hrtick_rearm_fair(rq, p);
> > +}
>
> What does the rq field buy?  __hrtick_rearm_fair() already tests
> hrtick_enabled_fair(), hrtick_active() and the class, and a same-task
> repick that finds no hrtick armed wants one regardless of what triggered
> the repick.  If there is a same-task repick that must not arm one, the
> changelog is the place to name it.
>
> Last one is only a question.  entity_tick() -> update_curr() ->
> update_deadline() has already pushed se->deadline by a slice before
> task_tick_fair() reaches the queued branch, so hrtick_start_fair() would
> compute a valid delay if called right there, with no new field and no
> change to put_prev_set_next_task().  The difference I can see is that the
> tick callback runs with rq->hrtick_sched == 0, so hrtick_start() would
> program the hrtimer immediately from inside its own callback instead of
> leaving it to hrtick_schedule_exit().  Is that what moved you to the pick
> side?
>
> Thanks,
> Zhan Xusheng