LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Valentin Schneider <valentin.schneider@arm.com>
To: Vincent Donnefort <vincent.donnefort@arm.com>,
	peterz@infradead.org, mingo@redhat.com,
	vincent.guittot@linaro.org
Cc: linux-kernel@vger.kernel.org, mgorman@techsingularity.net,
	dietmar.eggemann@arm.com,
	Vincent Donnefort <vincent.donnefort@arm.com>
Subject: Re: [PATCH] sched/fair: Fix per-CPU kthread and wakee stacking for asym CPU capacity
Date: Wed, 24 Nov 2021 17:11:32 +0000	[thread overview]
Message-ID: <87fsrlcwcb.mognet@arm.com> (raw)
In-Reply-To: <20211124141435.3125147-1-vincent.donnefort@arm.com>

On 24/11/21 14:14, Vincent Donnefort wrote:
> A shortcut has been introduced in select_idle_sibling() to return prev_cpu
> if the wakee is woken up by a per-CPU kthread. This is an issue for
> asymmetric CPU capacity systems where the wakee might not fit prev_cpu
> anymore. Evaluate asym_fits_capacity() for prev_cpu before using that
> shortcut.
>
> Fixes: 52262ee567ad ("sched/fair: Allow a per-CPU kthread waking a task to stack on the same CPU, to fix XFS performance regression")

Shouldn't that rather be

  b4c9c9f15649 ("sched/fair: Prefer prev cpu in asymmetric wakeup path")

? This is an ulterior commit to the one you point to, and before then
asymmetric CPU systems wouldn't use any of the sis() heuristics.

I reportedly reviewed said commit back then, and don't recall anything
specific about that conditional... The cover-letter for v2 states:

  https://lore.kernel.org/lkml/20201028174412.680-1-vincent.guittot@linaro.org/
  """
  don't check capacity for the per-cpu kthread UC because the assumption is
  that the wakee queued work for the per-cpu kthread that is now complete and
  the task was already on this cpu.
  """

So the assumption here is that current is gonna sleep right after waking up
p, so current's utilization doesn't matter, and p was already on prev, so
it should fit there...

I'm thinking things should actually be OK with your other patch that
excludes 'current == swapper' from this condition.

> Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 6291876a9d32..b90dc6fd86ca 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6410,7 +6410,8 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>        */
>       if (is_per_cpu_kthread(current) &&
>           prev == smp_processor_id() &&
> -	    this_rq()->nr_running <= 1) {
> +	    this_rq()->nr_running <= 1 &&
> +	    asym_fits_capacity(task_util, prev)) {
>               return prev;
>       }
>
> --
> 2.25.1

  reply	other threads:[~2021-11-24 17:11 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-24 14:14 Vincent Donnefort
2021-11-24 17:11 ` Valentin Schneider [this message]
2021-11-24 17:58   ` Vincent Donnefort
2021-11-24 18:04     ` Valentin Schneider
2021-11-25  9:10 ` Vincent Guittot

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=87fsrlcwcb.mognet@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@techsingularity.net \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=vincent.donnefort@arm.com \
    --cc=vincent.guittot@linaro.org \
    --subject='Re: [PATCH] sched/fair: Fix per-CPU kthread and wakee stacking for asym CPU capacity' \
    /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

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).