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

[Tim Chen <tim.c.chen@linux.intel.com>]

On Mon, 2026-09-14 at 10:02 +0800, Chen Yu wrote:
> 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

I assume both CPU0 and CPU1 in preferred LLC, and task woken on
CPU1.  Should say so.

> +		 *
> +		 * 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++

							       rq1->nr_pref_llc_running++
> +		 *
> +		 *
> +		 * case 2 - LB for delayed task

LB for delayed task from a preferred LLC to a non-preferred LLC.

> +		 *
> +		 * 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

						The above block should be under CPU 0

> +		 *
> +		 *                                     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
>  		 */


Thanks for describing the operation scenario.
However the above cases are not comprehensive and a bit long for comment.


>  		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;

This change looks good and make the code cleaner.

Tim

>  
>  	for_each_sched_entity(se) {
>  		struct cfs_rq *cfs_rq = cfs_rq_of(se);