From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mel Gorman Date: Wed, 21 Oct 2020 15:26:37 +0000 Subject: Re: [PATCH] sched/fair: check for idle core Message-Id: <20201021152637.GH32041@suse.de> List-Id: References: <1603211879-1064-1-git-send-email-Julia.Lawall@inria.fr> <20201021112038.GC32041@suse.de> <20201021122532.GA30733@vingu-book> <20201021124700.GE32041@suse.de> <20201021131827.GF32041@suse.de> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Julia Lawall Cc: Vincent Guittot , Ingo Molnar , kernel-janitors@vger.kernel.org, Peter Zijlstra , Juri Lelli , Dietmar Eggemann , Steven Rostedt , Ben Segall , Daniel Bristot de Oliveira , linux-kernel@vger.kernel.org, Valentin Schneider , Gilles.Muller@inria.fr On Wed, Oct 21, 2020 at 03:48:59PM +0200, Julia Lawall wrote: > > I worry it's overkill because prev is always used if it is idle even > > if it is on a node remote to the waker. It cuts off the option of a > > wakee moving to a CPU local to the waker which is not equivalent to the > > original behaviour. > > Could it be possible to check p->recent_used_cpu? If that is prev (or on > the same socket?), then prev could be a good choice. If that is on the > same socket as the waker, then maybe the waker would be better. > It is an interesting idea but the treatment of p->recent_used_cpu is a bit strange though. At one point, I looked at reconciling all the select CPU paths into one single pass (prototype worked back in 5.6-era but was not a universal win). As part of that, I remembered that the setting of recent_used_cpu is a problem because it can be set to the wakers recent_used_cpu by this chunk if (want_affine) current->recent_used_cpu = cpu; Fixing that on its own causes other problems. From an unposted series that "fixed it" as the last part of 14 patch series; After select_idle_sibling, the *wakers* recent_used_cpu is set to the CPU used for the wakee. This was necessary at the time as without it, the miss rate was unacceptably high. It still works, but it is less efficient than it can be. This patch leaves the waker state alone and uses either the previous CPU on a hit or the target CPU on a miss when the recently used CPU is examined for idleness. The rest of the series is important as without it, this patch improves the number of hits but the miss rate gets ridiculously high. After the rest of the series, the hit and miss rates are both higher but the miss rate is acceptable. As the hunk means that recent_used_cpu could have been set based on a previous wakeup, it makes it unreliable for making cross-node decisions. p->recent_used_cpu's primary purpose at the moment is to avoid select_idle_sibling searches. -- Mel Gorman SUSE Labs