All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org
Cc: Crystal Wood <swood@redhat.com>, John Keeping <john@metanate.com>,
	Boqun Feng <boqun.feng@gmail.com>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Waiman Long <longman@redhat.com>, Will Deacon <will@kernel.org>
Subject: [PATCH] locking/rtmutex: Do the trylock-slowpath with DEBUG_RT_MUTEXES enabled.
Date: Tue, 28 Mar 2023 18:54:30 +0200	[thread overview]
Message-ID: <20230328165430.9eOXd-55@linutronix.de> (raw)
In-Reply-To: <20230322162719.wYG1N0hh@linutronix.de>

With DEBUG_RT_MUTEXES enabled the fast-path locking
(rt_mutex_cmpxchg_acquire()) always fails. This leads to the invocation
of blk_flush_plug() even if the lock is not acquired which is
unnecessary and avoids batch processing of requests.

rt_mutex_slowtrylock() performs the trylock-slowpath and acquires the
lock if possible.
__rt_mutex_trylock() performs the fastpath try-lock and the slowpath
trylock. The latter is not desired in the non-debug case because it
fails very often even after rt_mutex_owner() reported that there is no
owner.
Here some numbers from a boot up + a few FS operations, hackbench:
- total __rt_mutex_lock() -> __rt_mutex_trylock() invocations with no
  owner: 32160
- success: 189
- failed: 31971
  - RT_MUTEX_HAS_WAITERS was set the whole time: 27469
  - owner appeared after the wait_lock has been obtained: 4502

The slowlock trylock failed in most cases without an owner because a
waiter was pending and did not acquire the lock yet. The few cases in
which it succeeded were because the pending bit was cleared after the
wait_lock was acquired.
Based on these numbers, rt_mutex_slowtrylock() in the non-DEBUG case
adds just overhead without contributing anything to the locking process.

In a dist-upgrade test with DEBUG_RT_MUTEXES enabled, the here proposed
rt_mutex_slowtrylock() optimisation acquired all locks with
current->plug set and avoided a blk_flush_plug() invocation.

Use rt_mutex_slowtrylock() in the DEBUG_RT_MUTEXES case to acquire the
lock instead the disabled rt_mutex_cmpxchg_acquire().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
On 2023-03-22 17:27:21 [+0100], To Thomas Gleixner wrote:
> > Aside of that for CONFIG_DEBUG_RT_MUTEXES=y builds it flushes on every
> > lock operation whether the lock is contended or not.
> 
> For mutex & ww_mutex operations. rwsem is not affected by
> CONFIG_DEBUG_RT_MUTEXES. As for mutex it could be mitigated by invoking
> try_to_take_rt_mutex() before blk_flush_plug().

This fixes the problem. I only observed blk_flush_plug() invocations
from down_read()/rwbase_read_lock() and down() which are not affected by
CONFIG_DEBUG_RT_MUTEXES.
I haven't observed anything in the ww-mutex path so we can ignore it or
do something similar to this.

 kernel/locking/rtmutex.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index c1bc2cb1522cb..08c599a5089a2 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1698,9 +1698,18 @@ static int __sched rt_mutex_slowlock(struct rt_mutex_base *lock,
 static __always_inline int __rt_mutex_lock(struct rt_mutex_base *lock,
 					   unsigned int state)
 {
+	/*
+	 * With DEBUG enabled cmpxchg trylock will always fail. Instead of
+	 * invoking blk_flush_plug() try the trylock-slowpath first which will
+	 * succeed if the lock is not contended.
+	 */
+#ifdef CONFIG_DEBUG_RT_MUTEXES
+	if (likely(rt_mutex_slowtrylock(lock)))
+		return 0;
+#else
 	if (likely(rt_mutex_cmpxchg_acquire(lock, NULL, current)))
 		return 0;
-
+#endif
 	/*
 	 * If we are going to sleep and we have plugged IO queued, make sure to
 	 * submit it to avoid deadlocks.
-- 
2.40.0

  reply	other threads:[~2023-03-28 16:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-22 16:27 [PATCH] locking/rtmutex: Flush the plug before entering the slowpath Sebastian Andrzej Siewior
2023-03-28 16:54 ` Sebastian Andrzej Siewior [this message]
2023-04-21 17:58   ` [PATCH] locking/rtmutex: Do the trylock-slowpath with DEBUG_RT_MUTEXES enabled Thomas Gleixner
2023-04-24  8:42     ` Sebastian Andrzej Siewior
2023-04-18 15:18 ` [PATCH] locking/rtmutex: Flush the plug before entering the slowpath Sebastian Andrzej Siewior
2023-04-18 23:43   ` Crystal Wood
2023-04-19 14:04     ` Sebastian Andrzej Siewior
2023-04-24 23:22       ` Crystal Wood
2023-04-21 19:18 ` Thomas Gleixner

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=20230328165430.9eOXd-55@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=boqun.feng@gmail.com \
    --cc=john@metanate.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=swood@redhat.com \
    --cc=tglx@linutronix.de \
    --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 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.