Re: sched/fair: which tasks should nr_pref_llc_running be compared against?

[Tim Chen]

On Sun, 2026-08-30 at 16:17 +0800, Chen Yu wrote:
> On Thu, Aug 27, 2026 at 01:57:33PM -0700, Tim Chen wrote:
> > On Thu, 2026-08-27 at 21:50 +0800, Zhan Xusheng wrote:
> > Signed-off-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
> > ---
> >  kernel/sched/fair.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 44 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index d78467ec6ee1..1673b17273c5 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -1544,7 +1544,14 @@ static void account_llc_dequeue(struct rq *rq, struct task_struct *p)
> >  
> >  	rq->nr_llc_running--;
> >  	if (p->pref_llc_queued) {
> > -		rq->nr_pref_llc_running--;
> > +		/*
> > +		 * If the task is being finally dequeued while still delayed,
> > +		 * set_delayed() already removed it from nr_pref_llc_running;
> > +		 * skip here to avoid underflow. Clearing pref_llc_queued also
> > +		 * stops the subsequent clear_delayed() from re-adding it.
> > +		 */
> > +		if (!p->se.sched_delayed)
> > +			rq->nr_pref_llc_running--;
> 
> Should we also do similar check in account_llc_enqueue()? It is possible during
> load balance migration, a migrate_load allows the task to be migrated across
> CPUs. And load balance leverages detach_tasks()/attach_tasks() to move tasks.
> During this stage the p->se.sched_delayed remained unchanged:
> In enqueue_task_fair(), only when if (se->on_rq && se->sched_delayed) is true,
> p->se.sched_delayed will be cleared by requeue_delayed_entity() - during migration,
> se->on_rq is false.

Yes, you're right.  We should extend the sched_delayed check also to enqueue.

> 
> As a result, attach_tasks -> enqueue_hierarchy -> enqueue_entity -> account_llc_enqueue(rq, p)
> incorrectly increase rq->nr_pref_llc_running - even that task is in delayed state, and
> not runnable.
> 
> >  		/*
> >  		 * Update the status in case
> >  		 * other logic might query
> > @@ -1572,6 +1579,24 @@ static void account_llc_dequeue(struct rq *rq, struct task_struct *p)
> >  	}
> >  }
> >  
> > +/*
> > + * A task becoming delay-dequeued leaves the runnable set while staying
> > + * queued. Keep nr_pref_llc_running in the runnable domain (like
> > + * h_nr_runnable) so alb_break_llc() can compare the two directly.
> > + */
> > 
> 
> Regarding alb_break_llc(), since we have compared nr_pref_llc_running vs 
> nr_runnable, I wonder if we should also change the code?
> 	if (env->src_rq->nr_pref_llc_running &&
>  	    env->src_rq->nr_pref_llc_running == env->src_rq->cfs.h_nr_runnable)
> 		unsigned long util = 0;
> 		struct task_struct *cur;
>  
>                 if (env->src_rq->cfs.h_nr_runnable <= 1)
>                         return true;

We already know that there is at least a cfs running task
preferring src LLC.

And if there is only 1 running task, (i.e. env->src_rq->nr_running <= 1)
then env->src_rq->>cfs.h_nr_runnable should be also 1 here. So switching
the check to use cfs.h_nr_runnable should have the same effect.

So I'll leave that change out for now.

The patch is updated as below.  Chen Yu and Xusheng,
please add your reviewed by if the updated patch looks good.

Thanks.

Tim

---