Re: [PATCH 1/4] sched/cache: Keep nr_pref_llc_running in the runnable domain

[Chen Yu <yu.c.chen@intel.com>]

Hi Kayra,

On Fri, Sep 11, 2026 at 01:03:30AM +0300, Kayra Cizmeci wrote:
> Hello Tim,
> 
> > So moving the accounting next to (or after) the h_nr_runnable update
> > would make task_pref_llc_runnable() return false and skip the
> > decrement, leaving nr_pref_llc_running too high.
> 
> What I really wanted wasn't getting the accounting next to or after the h_nr_runnable. 
> If we are updating h_nr_runnable in some way that means we don't need
> its check since it's already getting updated. And if it's getting updated
> that means on that branch we know how our check should behave since we 
> are a subset of it. We can skip the delayed check on that way since we are
> trying to behave as h_nr_runnable's subset.
> 
> >        if (entity_is_task(se))
> >                pref_llc_running_dec(...);      /* sched_delayed still 0 */
> >        se->sched_delayed = 1;
> >        ...
> >        for_each_sched_entity(se)
> >                cfs_rq->h_nr_runnable--;        /* sched_delayed already 1 */
> 
> For example:
> 
> In this code the h_nr_runnable is updated the same way regarding what is sched_delayed.
> That means if we want to behave as a subset of it, we don't need the check delayed,
> since we check the delayed to be a subset but if h_nr_runnable is decreasing/increasing
> we should look into our checks.
>

I had a try according to your suggestion. It seems that the code becomes more complex
and brings more headache :-( due to several corner cases. The current version is a simpler
version with less code IMO. But I agree it looks a little hard to catch up with, so I added
some comments around account_llc_dequeue() and adjusted the code sequence of clear_delayed()
to make it easier to understand. Tim, could you please help check if this makes sense?

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e64d9ad7a108..89311bbadfac 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1609,8 +1609,58 @@ static void account_llc_dequeue(struct rq *rq, struct task_struct *p)
 	if (p->pref_llc_queued) {
 		/*
 		 * Skipped if still delayed (set_delayed() already removed it);
-		 * clearing pref_llc_queued below also stops clear_delayed()
-		 * from re-adding it.
+		 *
+		 * The nr_pref_llc_running varies with h_nr_runnable, it involves
+		 * set_delayed(), clear_delayed(), account_llc_enqueued() and account_llc_dequeue().
+		 * The following shows two typical cases of how nr_pref_llc_running is maintained
+		 * during enqueue/dequeue.
+		 *
+		 * case 1 - wakeup a delayed task
+		 *
+		 * CPU0                                       CPU1
+		 * __dequeue_task [fake dequeue]
+		 *   set_delayed
+		 *     rq0->nr_pref_llc_running--
+		 *     p->se.sched_delayed = 1
+		 *
+		 *                                     try_to_wake_up(p)
+		 *                                       enqueue_task_fair
+		 *                                         requeue_delayed_entity
+		 *                                           clear_delayed
+		 *                                             rq0->nr_pref_llc_running++
+		 *
+		 *
+		 * case 2 - LB for delayed task
+		 *
+		 * CPU0                                       CPU1
+		 * __dequeue_task [fake dequeue]
+		 *   set_delayed
+		 *     rq0->nr_pref_llc_running--
+		 *     p->se.sched_delayed = 1
+		 *
+		 *
+		 *                                     --------- load balance ---------
+		 *                                     detach_task(p, rq0, migrate_load)
+		 *                                       account_llc_dequeue
+		 *                                         ** DO-NOT-DECREASE **
+		 *                                         rq0->nr_pref_llc_running
+		 *
+		 *                                     attach_task(p, rq1)
+		 *                                       account_llc_enqueue
+		 *                                         ** DO-NOT-INCREASE **
+		 *                                         rq1->nr_pref_llc_running
+		 *
+		 *                                     pick_eevdf
+		 *                                       __dequeue_task  [real dequeue]
+		 *                                         account_llc_dequeue(DEQUEUE_DELAYED)
+		 *                                           ** DO-NOT-DECREASE **
+		 *                                           rq1->nr_pref_llc_running
+		 *                                           p->pref_llc_queued = 0;
+		 *
+		 *                                         clear_delayed
+		 *                                           p->se.sched_delayed = 0;
+		 *                                           ** DO-NOT-INCREASE as pref_llc_queued=0 **
+		 *                                           rq1->nr_pref_llc_running
 		 */
 		pref_llc_running_dec(rq, p);
 		/*
@@ -6415,23 +6465,18 @@ 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;
-
 	/*
 	 * Delayed se of cfs_rq have no tasks queued on them.
 	 * Do not adjust h_nr_runnable since __dequeue_task()
 	 * will account it for blocked tasks.
 	 */
-	if (!entity_is_task(se))
+	if (!entity_is_task(se)) {
+		se->sched_delayed = 1;
 		return;
+	}
+
+	pref_llc_running_dec(rq_of(cfs_rq_of(se)), task_of(se));
+	se->sched_delayed = 1;
 
 	for_each_sched_entity(se) {
 		struct cfs_rq *cfs_rq = cfs_rq_of(se);
-- 
2.43.0