All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Yu <yu.c.chen@intel.com>
To: Kayra Cizmeci <kayracizmeci@gmail.com>
Cc: Zhan Xusheng <zhanxusheng1024@gmail.com>, <peterz@infradead.org>,
	<mingo@redhat.com>, <tim.c.chen@linux.intel.com>,
	<kprateek.nayak@amd.com>, <juri.lelli@redhat.com>,
	<vincent.guittot@linaro.org>, <dietmar.eggemann@arm.com>,
	<rostedt@goodmis.org>, <bsegall@google.com>, <mgorman@suse.de>,
	<vschneid@redhat.com>, <linux-kernel@vger.kernel.org>,
	<zhanxusheng@xiaomi.com>
Subject: Re: [PATCH] sched/cache: Check reduced capacity before alb_break_llc()
Date: Thu, 29 Oct 2026 16:18:15 +0800	[thread overview]
Message-ID: <auMBRzyMRwBS2HFT@chenyu-dev> (raw)
In-Reply-To: <auLs7xeL9FsHcf-h@scheduler.schedules>

On Thu, Oct 29, 2026 at 10:13:10AM +0300, Kayra Cizmeci wrote:
> On Thu, Oct 29, 2026 at 10:36:50AM +0800, Zhan Xusheng wrote:
> 
> Hi Zhan,
> 
> > From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> > 
> > need_active_balance() moves the only CFS task off a CPU whose capacity
> > is reduced, for example by IRQ load, to an idle CPU with more.
> > alb_break_llc() runs first and refuses to move a lone task off its
> > preferred LLC, so if no other CPU in that LLC is idle the task stays on
> > the reduced CPU.  Like asym packing, which was moved ahead of
> > alb_break_llc() for the same reason, this is a performance case that
> > cache locality should not override.
> > 
> > With two LLCs of two CPUs, the other CPU of the task's LLC busy and 60%
> > of the task's CPU spent in a hardirq, a CPU-bound two-thread process
> > stayed there for 20 seconds at 38% of its normal throughput.  With this
> > change it moves to the idle LLC within a second, as it does with cache
> > aware scheduling disabled.
> 
> Sweet.
> 
> > 
> > Fixes: 714059f79ff0 ("sched/cache: Handle moving single tasks to/from their preferred LLC")
> > Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> > ---
> >  kernel/sched/fair.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index cfc982205b98..e313c6d212c6 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -13585,12 +13585,6 @@ static int need_active_balance(struct lb_env *env)
> >  	if (asym_active_balance(env))
> >  		return 1;
> >  
> > -	if (alb_break_llc(env))
> > -		return 0;
> > -
> > -	if (imbalanced_active_balance(env))
> > -		return 1;
> > -
> >  	/*
> >  	 * The dst_cpu is idle and the src_cpu CPU has only 1 CFS task.
> >  	 * It's worth migrating the task if the src_cpu's capacity is reduced
> > @@ -13604,6 +13598,12 @@ static int need_active_balance(struct lb_env *env)
> >  			return 1;
> >  	}
> >  
> > +	if (alb_break_llc(env))
> > +		return 0;
> > +
> > +	if (imbalanced_active_balance(env))
> > +		return 1;
> > +
> >  	if (env->migration_type == migrate_misfit ||
> >  	    env->migration_type == migrate_llc_task)
> >  		return 1;
> > 
> > base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
> > -- 
> > 2.43.0
> > 
> 
> Well yeah. alb_break_llc() just returns true in the nr_running <= 1
> case. FWIW, on my tip tree this just creates a conflict. On me
> alb_break_llc() is first, then asym_active_balance() and then
> imbalanced_active_balance(). I think in that case moving all three
> functions would be better? (below this other block that's for this case)
>

The position of asym has been adjusted in
https://lore.kernel.org/lkml/179284198232.4109677.752653731564503349.tip-bot2@tip-bot2/

thanks,
Chenyu

  parent reply	other threads:[~2026-10-29  8:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29  2:36 [PATCH] sched/cache: Check reduced capacity before alb_break_llc() Zhan Xusheng
2026-10-29  7:13 ` Kayra Cizmeci
2026-10-29  7:50   ` Zhan Xusheng
2026-10-29  8:13     ` Kayra Cizmeci
2026-10-29  8:18   ` Chen Yu [this message]
2026-10-29  8:18 ` Chen Yu
2026-10-29  8:55   ` Zhan Xusheng
2026-10-30 15:04     ` Chen Yu
2026-10-30 23:24 ` Tim Chen
2026-10-31  9:24   ` Zhan Xusheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auMBRzyMRwBS2HFT@chenyu-dev \
    --to=yu.c.chen@intel.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kayracizmeci@gmail.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=zhanxusheng1024@gmail.com \
    --cc=zhanxusheng@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.