Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 24 Aug 2022 09:46:14 +0100
From:   Mel Gorman <mgorman@suse.de>
To:     Peng Wang <rocking@linux.alibaba.com>
Cc:     mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, bristot@redhat.com,
        vschneid@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/fair: select waker's cpu for wakee on sync wakeup
Message-ID: <20220824084614.kykmtbhyawcei67z@suse.de>
References: <1508aa17d1a169077c8d8d8c22d2bd529101af0e.1661313074.git.rocking@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <1508aa17d1a169077c8d8d8c22d2bd529101af0e.1661313074.git.rocking@linux.alibaba.com>
Precedence: bulk

On Wed, Aug 24, 2022 at 12:37:50PM +0800, Peng Wang wrote:
> On sync wakeup, waker is about to sleep, and if it is the only
> running task, wakee can get warm data on waker's cpu.
> 
> Unixbench, schbench, and hackbench are tested on
> Intel(R) Xeon(R) Platinum 8163 CPU @ 2.50GHz (192 logic CPUs)
> 
> Unixbench get +20.7% improvement with full threads mainly
> because of the pipe-based context switch and fork test.
> 
> No obvious impact on schbench.
> 
> This change harms hackbench with lower concurrency, while gets improvement
> when concurrency increases.
> 

Note that historically patches in this direction have been hazardous because
it makes a key assumption "sync wakers always go to sleep in the near future"
when the sync hint is not that reliable. Networking from a brief glance
still uses sync wakeups where wakers could have a 1:N relationship between
work producers and work consumers that would then stack multiple tasks on
one CPU for multiple consumers. The workloads mentioned in the changelog
are mostly strictly-synchronous wakeups (i.e. the waker definitely goes
to sleep almost immediately) and benefit from this sort of patch but it's
not necessarily a universal benefit.

Note that most of these hazards occurred *LONG* before I was paying much
attention to how the scheduler behaved so I cannot state "sync is still
unreliable" with absolute certainty. However, long ago there was logic
that tried to track the accuracy of the sync hint that was ultimately
abandoned by commit e12f31d3e5d3 ("sched: Remove avg_overlap"). AFAIK,
the sync hint is still not 100% reliable and while stacking sync works
for some workloads, it's likely to be a regression magnet for network
intensive workloads or client/server workloads like databases where
"synchronous wakeups are not always synchronous".

-- 
Mel Gorman
SUSE Labs