Re: wakeup_affine_weight() is b0rked - was Re: [PATCH 2/2] sched/fair: Scale wakeup granularity relative to nr_running

From: Mike Galbraith <efault@gmx.de>
To: Barry Song <21cnbao@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.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: wakeup_affine_weight() is b0rked - was Re: [PATCH 2/2] sched/fair: Scale wakeup granularity relative to nr_running
Date: Mon, 04 Oct 2021 11:06:30 +0200	[thread overview]
Message-ID: <4f571c5c759b9d356d1bb4114fb169181194a780.camel@gmx.de> (raw)
In-Reply-To: <f1b421f956fa044b4efa7f5fef015725b27223cf.camel@gmx.de>

On Mon, 2021-10-04 at 06:34 +0200, Mike Galbraith wrote:
> On Sun, 2021-10-03 at 20:34 +1300, Barry Song wrote:
> >
> > I am wondering if this should be the responsibility of wake_wide()?
>
> Those event threads we stacked so high (which are kde minions btw),
> don't generally accrue _any_ wakee_flips, so when X wakes a slew of the
> things, wake_wide()'s heuristic rejects the lot.
>
> So yeah, the blame game for this issue is a target rich environment.
> Shoot either of 'em (or both), and you'll hit the bad guy.

The mallet below convinced wake_wide() that X waking event threads is
something it most definitely should care about.  It's not perfect, can
get caught with its pants down, because behavior changes a LOT, but I
at least have to work at it a bit to stack tasks to the ceiling.

With make -j8 running along with firefox with two tabs, one containing
youtube's suggestions of stuff you probably don't want to watch, the
other a running clip, if I have the idle tab in focus, and don't drive
mouse around, flips decay enough for wake_wide() to lose interest, but
just wiggle the mouse, and it starts waking wide. Focus on the running
clip, and it continuously wakes wide.  

Hacky, but way better behavior.. at this particular testcase.. in this
particular box.. at least once :)

---
 kernel/sched/fair.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5865,6 +5865,14 @@ static void record_wakee(struct task_str
 	}

 	if (current->last_wakee != p) {
+		int min = __this_cpu_read(sd_llc_size) << 1;
+		/*
+		 * Couple the wakee flips to the waker for the case where it
+		 * doesn't accrue flips, taking care to not push the wakee
+		 * high enough that the wake_wide() heuristic fails.
+		 */
+		if (current->wakee_flips > p->wakee_flips * min)
+			p->wakee_flips++;
 		current->last_wakee = p;
 		current->wakee_flips++;
 	}
@@ -5895,7 +5903,7 @@ static int wake_wide(struct task_struct

 	if (master < slave)
 		swap(master, slave);
-	if (slave < factor || master < slave * factor)
+	if ((slave < factor && master < (factor>>1)*factor) || master < slave * factor)
 		return 0;
 	return 1;
 }

