kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Julia Lawall <julia.lawall@inria.fr>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
	Ingo Molnar <mingo@redhat.com>,
	kernel-janitors@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	linux-kernel@vger.kernel.org,
	Valentin Schneider <valentin.schneider@arm.com>,
	Gilles.Muller@inria.fr
Subject: Re: [PATCH] sched/fair: check for idle core
Date: Wed, 21 Oct 2020 15:26:37 +0000	[thread overview]
Message-ID: <20201021152637.GH32041@suse.de> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2010211547190.57356@hadrien>

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

      reply	other threads:[~2020-10-21 15:26 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20 16:37 [PATCH] sched/fair: check for idle core Julia Lawall
2020-10-21  7:29 ` Vincent Guittot
2020-10-21 11:13   ` Peter Zijlstra
2020-10-21 12:27   ` Vincent Guittot
2020-10-21 11:20 ` Mel Gorman
2020-10-21 11:56   ` Julia Lawall
2020-10-21 12:19     ` Peter Zijlstra
2020-10-21 12:42       ` Julia Lawall
2020-10-21 12:52         ` Peter Zijlstra
2020-10-21 18:18           ` Rafael J. Wysocki
2020-10-21 18:15         ` Rafael J. Wysocki
2020-10-21 19:47           ` Julia Lawall
2020-10-21 20:25             ` Rafael J. Wysocki
2020-10-21 13:10       ` Peter Zijlstra
2020-10-21 18:11         ` Rafael J. Wysocki
2020-10-22  4:53           ` Viresh Kumar
2020-10-22  7:11           ` Peter Zijlstra
2020-10-22 10:59             ` Viresh Kumar
2020-10-22 11:45               ` Rafael J. Wysocki
2020-10-22 12:02                 ` default cpufreq gov, was: " Peter Zijlstra
2020-10-22 12:19                   ` Rafael J. Wysocki
2020-10-22 12:29                     ` Peter Zijlstra
2020-10-22 14:52                       ` Mel Gorman
2020-10-22 14:58                         ` Colin Ian King
2020-10-22 15:12                           ` Phil Auld
2020-10-22 16:35                             ` Mel Gorman
2020-10-22 17:59                               ` Rafael J. Wysocki
2020-10-22 20:32                                 ` Mel Gorman
2020-10-22 20:39                                   ` Phil Auld
2020-10-22 15:25                         ` Peter Zijlstra
2020-10-22 15:55                           ` Rafael J. Wysocki
2020-10-22 16:29                           ` Mel Gorman
2020-10-22 20:10                           ` Giovanni Gherdovich
2020-10-22 20:16                             ` Giovanni Gherdovich
2020-10-23  7:03                             ` Peter Zijlstra
2020-10-23 17:46                               ` Tom Lendacky
2020-10-26 19:52                                 ` Fontenot, Nathan
2020-10-22 15:45                       ` A L
2020-10-22 15:55                         ` Vincent Guittot
2020-10-23  5:23                           ` Viresh Kumar
2020-10-22 16:23                   ` [PATCH] cpufreq: Avoid configuring old governors as default with intel_pstate Rafael J. Wysocki
2020-10-23  6:29                     ` Viresh Kumar
2020-10-23 11:59                       ` Rafael J. Wysocki
2020-10-23 15:15                     ` [PATCH v2] " Rafael J. Wysocki
2020-10-27  3:13                       ` Viresh Kumar
2020-10-27 11:11                   ` default cpufreq gov, was: [PATCH] sched/fair: check for idle core Qais Yousef
2020-10-27 11:26                     ` Valentin Schneider
2020-10-27 11:42                       ` Qais Yousef
2020-10-27 11:48                         ` Viresh Kumar
2020-10-23  6:24                 ` Viresh Kumar
2020-10-23 15:06                   ` Rafael J. Wysocki
2020-10-27  3:13                     ` Viresh Kumar
2020-10-22 11:21             ` AW: " Walter Harms
2020-10-21 12:28     ` Mel Gorman
2020-10-21 12:25   ` Vincent Guittot
2020-10-21 12:47     ` Mel Gorman
2020-10-21 12:56       ` Julia Lawall
2020-10-21 13:18         ` Mel Gorman
2020-10-21 13:24           ` Julia Lawall
2020-10-21 15:08             ` Mel Gorman
2020-10-21 15:18               ` Julia Lawall
2020-10-21 15:23                 ` Vincent Guittot
2020-10-21 15:33                   ` Julia Lawall
2020-10-21 15:19               ` Vincent Guittot
2020-10-21 17:00                 ` Mel Gorman
2020-10-21 17:39                   ` Julia Lawall
2020-10-21 13:48           ` Julia Lawall
2020-10-21 15:26             ` Mel Gorman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201021152637.GH32041@suse.de \
    --to=mgorman@suse.de \
    --cc=Gilles.Muller@inria.fr \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=julia.lawall@inria.fr \
    --cc=juri.lelli@redhat.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).