Re: [PATCH 0/4] sched/fair: Avoid unnecessary migrations within SMT domains

From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Ricardo Neri <ricardo.neri@intel.com>,
	"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
	Ben Segall <bsegall@google.com>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Len Brown <len.brown@intel.com>, Mel Gorman <mgorman@suse.de>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Valentin Schneider <vschneid@redhat.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/4] sched/fair: Avoid unnecessary migrations within SMT domains
Date: Wed, 19 Oct 2022 18:38:20 -0700	[thread overview]
Message-ID: <20221020013820.GB20255@ranerica-svr.sc.intel.com> (raw)
In-Reply-To: <Y050e5XQkaUrwr5j@hirez.programming.kicks-ass.net>

On Tue, Oct 18, 2022 at 11:40:11AM +0200, Peter Zijlstra wrote:
> On Mon, Oct 17, 2022 at 07:35:27PM -0700, Ricardo Neri wrote:
> > On Thu, Aug 25, 2022 at 03:55:25PM -0700, Ricardo Neri wrote:
> > > Intel processors that support Intel Turbo Boost Max 3.0 use asym_packing
> > > to assign higher priorities to CPUs with higher maximum frequencies. It
> > > artificially assigns, however, a lower priority to the higher-numbered
> > > SMT siblings to ensure that they are used last.
> > > 
> > > This results in unnecessary task migrations within the SMT domains.
> > > 
> > > On processors with a mixture of higher-frequency SMT cores and lower-
> > > frequency non-SMT cores (such as Intel hybrid processors), a lower-
> > > priority CPU pulls tasks from the higher-priority cores if more than one
> > > SMT sibling is busy.
> > > 
> > > Do not use different priorities for each SMT sibling. Instead, tweak the
> > > asym_packing load balancer to recognize SMT cores with more than one
> > > busy sibling and let lower-priority CPUs pull tasks.
> > > 
> > > Removing these artificial priorities avoids superfluous migrations and
> > > lets lower-priority cores inspect all SMT siblings for the busiest queue.
> > 
> > Hello. I'd like to know if there are any comments on these patches. This
> > patchset is a requisite for the IPC classes of tasks patchset [1].

Thank you very much for your feedback Peter!

> 
> Urgh.. so I'm not liking this, afaict you're sprinkling SMT2
> assumptions.

I was careful to not introduce this assumption. I always look for one or
more busy SMT siblings. The goal of the series use the same priority for
all SMT siblings when possible (in x86 but not in Power7) so that lower-
priority sched groups can pull tasks from either siblings when needed (as
in [1]).

> 
> Why can't we make arch_asym_cpu_priority() depend on CPU state? Doesn't
> it then magically work?

That is an interesting idea. I will experiment with it.

Thanks and BR,
Ricardo