linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, will@kernel.org, linux-kernel@vger.kernel.org,
	boqun.feng@gmail.com
Subject: Re: [PATCH 3/6] locking/rwsem: Rework writer wakeup
Date: Mon, 27 Feb 2023 15:16:25 -0500	[thread overview]
Message-ID: <c126f079-88a2-4067-6f94-82f51cf5ff2b@redhat.com> (raw)
In-Reply-To: <Y/yGZgz1cJ1+pTt5@hirez.programming.kicks-ass.net>

On 2/27/23 05:31, Peter Zijlstra wrote:
>> I do have some concern that early lock transfer to a lock owner that has not
>> been woken up yet may suppress writer lock stealing from optimistic spinning
>> causing some performance regression in some cases. Let's see if the test
>> robot report anything.
> Ah yes, I suppose that is indeed a possibility. Given this is all under
> wait_lock and the spinner is not, I was hoping it would still have
> sufficient time to win. But yes, robots will tell us.
>
I run my rwsem locking microbenchmark on a 2-socket 96-thread x86-64
system with lock event turned on for 15 secs.

Before this patchset:

Running locktest with rwsem [runtime = 15s, r% = 50%, load = 100]
Threads = 96, Min/Mean/Max = 74,506/91,260/112,409
Threads = 96, Total Rate = 584,091 op/s; Percpu Rate = 6,084 op/s

rwsem_opt_fail=127305
rwsem_opt_lock=4252147
rwsem_opt_nospin=28920
rwsem_rlock=2713129
rwsem_rlock_fail=0
rwsem_rlock_fast=5
rwsem_rlock_handoff=280
rwsem_rlock_steal=1486617
rwsem_sleep_reader=2713085
rwsem_sleep_writer=4313369
rwsem_wake_reader=29876
rwsem_wake_writer=5829160
rwsem_wlock=127305
rwsem_wlock_fail=0
rwsem_wlock_handoff=2515

After this patchset:

Running locktest with rwsem [runtime = 15s, r% = 50%, load = 100]
Threads = 96, Min/Mean/Max = 26,573/26,749/26,833
Threads = 96, Total Rate = 171,184 op/s; Percpu Rate = 1,783 op/s

rwsem_opt_fail=1265481
rwsem_opt_lock=17939
rwsem_rlock=1266157
rwsem_rlock_fail=0
rwsem_rlock_fast=0
rwsem_rlock_handoff=0
rwsem_rlock_steal=551
rwsem_sleep_reader=1266157
rwsem_sleep_writer=1265481
rwsem_wake_reader=26612
rwsem_wake_writer=0
rwsem_wlock=1265481
rwsem_wlock_ehandoff=94
rwsem_wlock_fail=0
rwsem_wlock_handoff=94

So the locking rate is reduced to just 29.3% of the original. Looking at
the number of successful writer lock stealings from optimistic spinning
(rwsem_opt_lock), it is reduced from 4252147 to 17939. It is just about
0.4% of the original.

So for workloads that have a lot of writer contention, there will be
performance regressions. Do you mind if we try to keep the original
logic of my patchset to allow write lock acquisition in writer slow
path, but transfer the lock ownership in the wakeup path when handoff
is required. We can do this with some minor code changes on top of your
current patchset.

Regards,
Longman



  reply	other threads:[~2023-02-27 20:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-23 12:26 [PATCH 0/6] locking/rwsem: Rework writer wakeup and handoff Peter Zijlstra
2023-02-23 12:26 ` [PATCH 1/6] locking/rwsem: Minor code refactoring in rwsem_mark_wake() Peter Zijlstra
2023-02-23 12:26 ` [PATCH 2/6] locking/rwsem: Enforce queueing when HANDOFF Peter Zijlstra
2023-02-23 12:26 ` [PATCH 3/6] locking/rwsem: Rework writer wakeup Peter Zijlstra
2023-02-23 21:38   ` Waiman Long
2023-02-26 11:58     ` Peter Zijlstra
2023-02-26 12:00     ` Peter Zijlstra
2023-02-26 21:31       ` Waiman Long
2023-02-26 11:59   ` Peter Zijlstra
2023-02-26 15:04   ` Peter Zijlstra
2023-02-26 16:51     ` Peter Zijlstra
2023-02-27  0:22       ` Waiman Long
2023-02-27 10:31         ` Peter Zijlstra
2023-02-27 20:16           ` Waiman Long [this message]
2023-03-20  8:12             ` Peter Zijlstra
2023-03-20 17:36               ` Waiman Long
2023-02-23 12:26 ` [PATCH 4/6] locking/rwsem: Split out rwsem_reader_wake() Peter Zijlstra
2023-02-23 12:26 ` [PATCH 5/6] locking/rwsem: Unify wait loop Peter Zijlstra
2023-02-23 19:31   ` Boqun Feng
2023-02-24  1:33     ` Boqun Feng
2023-02-26 12:01       ` Peter Zijlstra
2023-02-26 18:22         ` Boqun Feng
2023-02-23 22:45   ` Waiman Long
2023-02-26 16:15     ` Peter Zijlstra
2023-02-23 12:26 ` [PATCH 6/6] locking/rwsem: Use the force Peter Zijlstra
2023-02-24  1:19 ` [PATCH 0/6] locking/rwsem: Rework writer wakeup and handoff Waiman Long
2023-02-24 11:55   ` Jiri Wiesner

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=c126f079-88a2-4067-6f94-82f51cf5ff2b@redhat.com \
    --to=longman@redhat.com \
    --cc=boqun.feng@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=will@kernel.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).