Re: [PATCH 1/5] sched/fair: ignore SIS_UTIL when has idle core

From: Mel Gorman <mgorman@techsingularity.net>
To: Chen Yu <yu.c.chen@intel.com>
Cc: Abel Wu <wuyun.abel@bytedance.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mel Gorman <mgorman@suse.de>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Josh Don <joshdon@google.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] sched/fair: ignore SIS_UTIL when has idle core
Date: Wed, 7 Sep 2022 09:41:35 +0100	[thread overview]
Message-ID: <20220907084135.izfqd7rga7fdk6u3@techsingularity.net> (raw)
In-Reply-To: <YxhH7cB+OIMAB0dM@chenyu5-mobl1>

On Wed, Sep 07, 2022 at 03:27:41PM +0800, Chen Yu wrote:
> > Ok, that's rational. There will be corner cases where there was no idle
> > CPU near the target when there is an idle core far away but it would be
> > counter to the purpose of SIS_UTIL to care about that corner case.
> > 
> > >  2) Unconditionally full scan when has_idle_core is not good
> > >     for netperf_{udp,tcp} and tbench4. It is probably because
> > >     the SIS success rate of these workloads is already high
> > >     enough (netperf ~= 100%, tbench4 ~= 50%, compared to that
> > >     hackbench ~= 3.5%) which negate a lot of the benefit full
> > >     scan brings.
> > > 
> > 
> > That's also rational. For a single client/server on netperf, it's expected
> > that the SIS success rate is high and scanning is minimal. As the client
> > and server are sharing data on localhost and somewhat synchronous, it may
> > even partially benefit from SMT sharing.
> >
> Maybe off-topic, since we monitor the success rate and also other metrics
> for each optimization in SIS path, is it possible to merge your statistics
> patch [1] into upstream so we don't need to rebase in the future(although
> it is targeting kernel development)?
> 

I am doubtful it is a merge candidate. While it's very useful when modifying
SIS and gathering data on whether SIS is behaving as expected, it has little
practical benefit when debugging problems on normal systems.  Crude estimates
can be obtained by other methods. Probing when select_idle_sibling and
select_idle_cpu are called reveals the SIS fast and slow paths and the
ratio between time. Tracking the time spent in select_idle_cpu reveals
how much time is spent finding idle cores and cpus.

I would not object to someone trying but the changlog would benefit from
explaining why it's practically useful. Every time I've used it, it was to
justify another patch being merged or investigating various SIS corner cases.

-- 
Mel Gorman
SUSE Labs