All of lore.kernel.org
 help / color / mirror / Atom feed
From: Valentin Schneider <vschneid@redhat.com>
To: Tianchen Ding <dtcccc@linux.alibaba.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] sched: Queue task on wakelist in the same llc if the wakee cpu is idle
Date: Mon, 30 May 2022 17:24:05 +0100	[thread overview]
Message-ID: <xhsmhleuj7zve.mognet@vschneid.remote.csb> (raw)
In-Reply-To: <20220527090544.527411-1-dtcccc@linux.alibaba.com>

On 27/05/22 17:05, Tianchen Ding wrote:
> The main idea of wakelist is to avoid cache bouncing. However,
> commit 518cd6234178 ("sched: Only queue remote wakeups when
> crossing cache boundaries") disabled queuing tasks on wakelist when
> the cpus share llc. This is because, at that time, the scheduler must
> send IPIs to do ttwu_queue_wakelist. Nowadays, ttwu_queue_wakelist also
> supports TIF_POLLING, so this is not a problem now when the wakee cpu is
> in idle polling.

[...]

> Our patch has improvement on schbench, hackbench
> and Pipe-based Context Switching of unixbench
> when there exists idle cpus,
> and no obvious regression on other tests of unixbench.
> This can help improve rt in scenes where wakeup happens frequently.
>
> Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com>

This feels a bit like a generalization of

  2ebb17717550 ("sched/core: Offload wakee task activation if it the wakee is descheduling")

Given rq->curr is updated before prev->on_cpu is cleared, the waker
executing ttwu_queue_cond() can observe:

  p->on_rq=0
  p->on_cpu=1
  rq->curr=swapper/x (aka idle task)

So your addition of available_idle_cpu() in ttwu_queue_cond() (sort of)
matches that when invoked via:

        if (smp_load_acquire(&p->on_cpu) &&
            ttwu_queue_wakelist(p, task_cpu(p), wake_flags | WF_ON_CPU))
                goto unlock;

but it also affects

        ttwu_queue(p, cpu, wake_flags);

at the tail end of try_to_wake_up().

With all that in mind, I'm curious whether your patch is functionaly close
to the below. 

---
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 66c4e5922fe1..ffd43264722a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3836,7 +3836,7 @@ static inline bool ttwu_queue_cond(int cpu, int wake_flags)
 	 * the soon-to-be-idle CPU as the current CPU is likely busy.
 	 * nr_running is checked to avoid unnecessary task stacking.
 	 */
-	if ((wake_flags & WF_ON_CPU) && cpu_rq(cpu)->nr_running <= 1)
+	if (cpu_rq(cpu)->nr_running <= 1)
 		return true;
 
 	return false;


  reply	other threads:[~2022-05-30 16:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-27  9:05 [PATCH v2] sched: Queue task on wakelist in the same llc if the wakee cpu is idle Tianchen Ding
2022-05-30 16:24 ` Valentin Schneider [this message]
2022-05-31  7:20   ` Tianchen Ding
2022-05-31 11:50     ` Valentin Schneider
2022-05-31 13:55       ` Mel Gorman
2022-05-31 15:38         ` Tianchen Ding
2022-05-31 15:56         ` Valentin Schneider
2022-06-01  5:54           ` Tianchen Ding
2022-06-01 10:58             ` Valentin Schneider
2022-06-01 12:02               ` Tianchen Ding

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=xhsmhleuj7zve.mognet@vschneid.remote.csb \
    --to=vschneid@redhat.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=dtcccc@linux.alibaba.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.