Re: [PATCH 1/2] sched/fair: Couple wakee flips with heavy wakers

From: Mike Galbraith <efault@gmx.de>
To: Vincent Guittot <vincent.guittot@linaro.org>,
	Mel Gorman <mgorman@techsingularity.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Valentin Schneider <Valentin.Schneider@arm.com>,
	Aubrey Li <aubrey.li@linux.intel.com>,
	Barry Song <song.bao.hua@hisilicon.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] sched/fair: Couple wakee flips with heavy wakers
Date: Sat, 30 Oct 2021 05:11:30 +0200	[thread overview]
Message-ID: <ab98b05314846102baec5cbb46893675f4ea5cfb.camel@gmx.de> (raw)
In-Reply-To: <CAKfTPtB-fJ7Pd6eYPDrHB8Ts0o7SCbN7nniAD9PSoF4Pf+xB3w@mail.gmail.com>

On Fri, 2021-10-29 at 17:17 +0200, Vincent Guittot wrote:
>
> I have a hard time understanding the rationale behind these changes
> and the one below. Could you provide more details about why to
> increase p->wakee_flips here ?

The rationale behind it was me discovering wake_affine_weight() use of
weight averages causing X waking a way too big thread pool to stack the
entire herd of 21 threads onto the waker's CPU while all other CPUs in
my little i7 box had one task each.  Preventing stacking is SIS or
wake_wide(), but because I was test driving a patch I had some fairness
concerns about, box was kept busy.  I was subsequently asked about
wake_wide()'s role, and while I don't think it should have one in a
single LLC box, looked into it, found that while X is a candidate,
event thread wakees were not. I think to self, what if I loosely couple
zero flip earning wakees, do so, then allow for a bit of decay wiggle
room while after watching it do its thing in realtime.

There you have the rationale.

While it did help, it did not eliminate the aforementioned worst case
because as desktop behavior changes, decay turns off the heuristic,
stacking follows.  I profiled it with a perf that sums delay (local mod
I find useful), and found that there was no real benefit to the light
desktop test load, at which point, no longer having NUMA boxen at my
disposal where wake_wide() does have a mission, I lost interest.  Mel
was interested however, fed it to SUSE's test array, and here we are.

Executive summary: patchlet is not so lovely mitigation of an even more
not so lovely scheduler behavior. The two deserve each other ;-)

Kidding aside, way better would be wake_wide() becoming obsolete.

	-Mike