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

[Tim Chen]

On Thu, 2026-09-03 at 23:21 +0800, Chen Yu wrote:
> On Tue, Sep 01, 2026 at 01:42:58PM -0700, Tim Chen wrote:
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 8dff37059faf..84c068f1deec 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -1549,7 +1549,13 @@ static void account_llc_enqueue(struct rq *rq, struct task_struct *p)
> >  
> >  	pref_llc_queued = (pref_llc == task_llc(p));
> >  	rq->nr_llc_running++;
> > -	rq->nr_pref_llc_running += pref_llc_queued;
> 
> Since the logic in
> account_llc_enqueue()/account_llc_dequeue()/
> account_llc_delayed()/account_llc_requeue_delayed() are very
> similar, can we introduce one helper for them:
> static void account_llc_pref_running(struct rq *rq, struct task_struct *p, int delta)
> {
>        if (p->pref_llc_queued && !p->se.sched_delayed)
>                rq->nr_pref_llc_running += delta;
> }
> 


Good idea - the four sites really are one operation ("if the task is
queued on its preferred LLC and runnable, move the counter"), and
folding the two conditions into one place is what keeps them from
drifting apart later. I've adopted it in v3; account_llc_delayed() and
account_llc_requeue_delayed() are gone.

I split it slightly differently: a membership predicate

        static bool task_pref_llc_runnable(struct task_struct *p)
        {
                return p->pref_llc_queued && !p->se.sched_delayed;
        }

with pref_llc_running_inc()/pref_llc_running_dec() wrappers over it, so
the call sites read as inc/dec rather than passing a +1/-1 delta.

Two things to note:

1) I kept the call site comments of pref_llc_running_inc/dec().
   The helper name says *what* happens,
   but not *why* it is safe across the delay-dequeue transition - that
   set_delayed() already did the decrement, so account_llc_dequeue()
   must skip it, and that clearing pref_llc_queued there is what
   neutralizes the following clear_delayed(). 

2) The pref_llc_running_dec() placement in set_delayed()
   is subtle. It has to be before se->sched_delayed = 1 or
   decrement would not happen. That deserves a comment so no
   one would move sched_delayed = 1 before the decrement.

Tim
---
From: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
Date: Wed, 3 Sep 2026 09:00:00 -0700
Subject: [PATCH v3] sched/cache: Keep nr_pref_llc_running in the runnable domain
To: Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Zhan Xusheng <zhanxusheng1024@xxxxxxxxx>, peterz@xxxxxxxxxxxxx, juri.lelli@xxxxxxxxxx, vincent.guittot@xxxxxxxxxx, dietmar.eggemann@xxxxxxx, rostedt@xxxxxxxxxxx, bsegall@xxxxxxxxxx,
mgorman@xxxxxxx, vschneid@xxxxxxxxxx, kprateek.nayak@xxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, zhanxusheng@xxxxxxxxxx

alb_break_llc() decides whether to break LLC preference during active
load balance. It does so by testing that every runnable fair task on the
source rq prefers its LLC:

	env->src_rq->nr_pref_llc_running == env->src_rq->cfs.h_nr_runnable

But the two counters cover different sets. nr_pref_llc_running is updated
in account_llc_enqueue()/account_llc_dequeue(), next to cfs_rq->nr_queued,
so it follows queued tasks. h_nr_runnable is updated in set_delayed()/
clear_delayed() and drops delay-dequeued tasks.

So under DELAY_DEQUEUE, a preferring task that goes to sleep stays counted
in nr_pref_llc_running while h_nr_runnable falls. The equality then breaks,
alb_break_llc() returns false, and active balance is free to pull a task
off its preferred LLC. Active balance only moves runnable tasks, and this
is the only LLC check it consults: once the stopper runs, LBF_ACTIVE_LB
skips the per-task test in can_migrate_task(). The runnable set is the one
we want.

Fix it on the counter side. A task should be counted in
nr_pref_llc_running exactly while it is both queued on its preferred LLC
(pref_llc_queued) and runnable (!sched_delayed). Define that membership
once in task_pref_llc_runnable(), and adjust the counter only through
pref_llc_running_inc()/pref_llc_running_dec() from the four sites that
change either input: account_llc_enqueue(), account_llc_dequeue(),
set_delayed() and clear_delayed(). Gating every update on the same
predicate keeps the delay, wake and dequeue paths from double-counting
or underflowing; see the comments at those sites for the ordering.

nr_llc_running and sd->llc_counts are not touched and stay on queued
semantics.

Reported-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
Closes: https://lore.kernel.org/lkml/20260827135000.735138-1-zhanxusheng@xxxxxxxxxx/
Suggested-by: Chen Yu <yu.c.chen@xxxxxxxxx>
Signed-off-by: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>
---
Based on v7.3-rc1.

Changes in v3:
- Route every nr_pref_llc_running adjustment through a single membership
  predicate task_pref_llc_runnable(), with pref_llc_running_inc()/
  pref_llc_running_dec() wrappers, instead of four open-coded sites
  (Chen Yu). Keep the per-site comments that explain the delay-dequeue
  interaction, and note that set_delayed() must adjust the counter
  before setting se->sched_delayed.

Changes in v2:
- Prevent a delay-dequeued task from being counted as running in the
  enqueue path (Chen Yu).

 kernel/sched/fair.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8dff37059faf..72aae7a50b8b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1538,6 +1538,28 @@ static bool invalid_llc_nr(struct mm_struct *mm, struct task_struct *p,
 			(scale * per_cpu(sd_llc_size, cpu)));
 }
 
+/*
+ * A task counts in nr_pref_llc_running while it is queued on its preferred
+ * LLC (pref_llc_queued) and runnable (!sched_delayed), keeping the counter in
+ * the runnable domain so alb_break_llc() can compare it with h_nr_runnable.
+ */
+static bool task_pref_llc_runnable(struct task_struct *p)
+{
+	return p->pref_llc_queued && !p->se.sched_delayed;
+}
+
+static void pref_llc_running_inc(struct rq *rq, struct task_struct *p)
+{
+	if (task_pref_llc_runnable(p))
+		rq->nr_pref_llc_running++;
+}
+
+static void pref_llc_running_dec(struct rq *rq, struct task_struct *p)
+{
+	if (task_pref_llc_runnable(p))
+		rq->nr_pref_llc_running--;
+}
+
 static void account_llc_enqueue(struct rq *rq, struct task_struct *p)
 {
 	int pref_llc, pref_llc_queued;
@@ -1549,7 +1571,6 @@ static void account_llc_enqueue(struct rq *rq, struct task_struct *p)
 
 	pref_llc_queued = (pref_llc == task_llc(p));
 	rq->nr_llc_running++;
-	rq->nr_pref_llc_running += pref_llc_queued;
 
 	/*
 	 * Record whether p is enqueued on its preferred
@@ -1567,6 +1588,9 @@ static void account_llc_enqueue(struct rq *rq, struct task_struct *p)
 	 */
 	p->pref_llc_queued = pref_llc_queued;
 
+	/* Skipped while delayed; clear_delayed() adds it back on wake. */
+	pref_llc_running_inc(rq, p);
+
 	sd = rcu_dereference_all(rq->sd);
 	if (sd && (unsigned int)pref_llc < sd->llc_max)
 		sd->llc_counts[pref_llc]++;
@@ -1583,7 +1607,12 @@ 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--;
+		/*
+		 * Skipped if still delayed (set_delayed() already removed it);
+		 * clearing pref_llc_queued below also stops clear_delayed()
+		 * from re-adding it.
+		 */
+		pref_llc_running_dec(rq, p);
 		/*
 		 * Update the status in case
 		 * other logic might query
@@ -2008,6 +2037,10 @@ static void account_llc_enqueue(struct rq *rq, struct task_struct *p) {}
 
 static void account_llc_dequeue(struct rq *rq, struct task_struct *p) {}
 
+static void pref_llc_running_inc(struct rq *rq, struct task_struct *p) {}
+
+static void pref_llc_running_dec(struct rq *rq, struct task_struct *p) {}
+
 #endif /* CONFIG_SCHED_CACHE */
 
 /*
@@ -6382,6 +6415,14 @@ static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq);
 
 static void set_delayed(struct sched_entity *se)
 {
+	/*
+	 * Drop a task leaving the runnable set. Must run before sched_delayed
+	 * is set, or task_pref_llc_runnable() would already exclude it;
+	 * clear_delayed() mirrors this after clearing the flag.
+	 */
+	if (entity_is_task(se))
+		pref_llc_running_dec(rq_of(cfs_rq_of(se)), task_of(se));
+
 	se->sched_delayed = 1;
 
 	/*
@@ -6412,6 +6453,13 @@ static void clear_delayed(struct sched_entity *se)
 	if (!entity_is_task(se))
 		return;
 
+	/*
+	 * Re-add on wake, after sched_delayed is cleared. On a final delayed
+	 * dequeue account_llc_dequeue() already cleared pref_llc_queued, so
+	 * this does nothing.
+	 */
+	pref_llc_running_inc(rq_of(cfs_rq_of(se)), task_of(se));
+
 	for_each_sched_entity(se) {
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
-- 
2.32.0