Re: [RFC PATCH] sched/fair: Choose the CPU where short task is running during wake up

From: Chen Yu <yu.c.chen@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
	Tim Chen <tim.c.chen@intel.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Juri Lelli <juri.lelli@redhat.com>,
	Rik van Riel <riel@surriel.com>, Aaron Lu <aaron.lu@intel.com>,
	Abel Wu <wuyun.abel@bytedance.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	Yicong Yang <yangyicong@hisilicon.com>,
	"Gautham R . Shenoy" <gautham.shenoy@amd.com>,
	Ingo Molnar <mingo@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>,
	Valentin Schneider <vschneid@redhat.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] sched/fair: Choose the CPU where short task is running during wake up
Date: Sat, 17 Sep 2022 21:55:45 +0800	[thread overview]
Message-ID: <YyXR4YgHeTR4Negc@chenyu5-mobl1> (raw)
In-Reply-To: <YyRhvEGS4ivPGf+z@hirez.programming.kicks-ass.net>

On 2022-09-16 at 13:45:00 +0200, Peter Zijlstra wrote:
> On Fri, Sep 16, 2022 at 12:54:07AM +0800, Chen Yu wrote:
> > And the rq lock bottleneck is composed of two paths(perf profile):
> > 
> > (path1):
> > raw_spin_rq_lock_nested.constprop.0;
> > try_to_wake_up;
> > default_wake_function;
> > autoremove_wake_function;
> > __wake_up_common;
> > __wake_up_common_lock;
> > __wake_up_sync_key;
> > pipe_write;
> > new_sync_write;
> > vfs_write;
> > ksys_write;
> > __x64_sys_write;
> > do_syscall_64;
> > entry_SYSCALL_64_after_hwframe;write
> 
> Can you please addr2line -i the raw_spin_rq_lock callsite so we know which is
> the one causing grief?
> 
> Specifically; I'm worried about PSI, psi_ttwu_dequeue() can cause ttwu()
> to take _2_ rq->lock, which absolutely blows for this case.
Above perf profile result was captured with 'psi=0' appended in the boot
commandline, and with NO_TTWU_QUEUE on 6.0-rc4. To narrow down we disabled
psi the first time we saw a rq lock contention. But even with psi=0 we still
observe the rq lock contention.

To confirm this, the 'perf report -F+period,srcline' was used to leverage
addr2line to parse the line. However it seems that with DWARF v4 enabled
in the kernel, the rq lock issue could not be reproduced. So I hacked the
code to make ttwu_queue() non-static, and perf profile shows that it grabs
the rq lock:

raw_spin_rq_lock_nested.constprop.0;
ttwu_queue;    <----------
try_to_wake_up;
default_wake_function;
autoremove_wake_function;
__wake_up_common;
__wake_up_common_lock;
__wake_up_sync_key;
pipe_write;
vfs_write;
ksys_write;
__x64_sys_write;
do_syscall_64;
entry_SYSCALL_64_after_hwframe;
write

Then if TTWU_QUEUE is enabled, the rq lock contention issue could
not be reproduced, but long idle duration was still observed due to
sched_ttwu_pending(as descibed in the commit log).

thanks,
Chenyu