All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>, Will Deacon <will@kernel.org>,
	Waiman Long <longman@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Steven Rostedt <rostedt@goodmis.org>,
	Byungchul Park <byungchul.park@lge.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Radoslaw Burny <rburny@google.com>
Subject: [PATCH 12/12] locking: Move lock_acquired() from the fast path
Date: Tue,  8 Feb 2022 11:43:24 -0800	[thread overview]
Message-ID: <20220208194324.85333-13-namhyung@kernel.org> (raw)
In-Reply-To: <20220208194324.85333-1-namhyung@kernel.org>

The lock_acquired() function is used by CONFIG_LOCK_STAT to track wait
time for contended locks.  So it's meaningful only if the given lock
is in the slow path (contended).  Let's move the call into the if
block so that we can skip it in the fast path.  This also move the
tracepoint to be called only after lock_contended().

It might affect bounce_acquired stats rarely (if it's on a different
cpu than when you call lock_acquire) but I'm not sure it's possible in
uncontended cases.  Otherwise, this should have no functional changes
in the LOCKDEP and LOCK_STAT.

Userspace tools that use the tracepoint might see the difference, but
I think most of them can handle the missing lock_acquired() in
non-contended case properly as it's the case when using a trylock
function to grab a lock.  At least it seems ok for the perf
tools ('perf lock' command specifically).

Add similar change in the __mutex_lock_common() so that it can call
lock_acquired() only after lock_contended().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 Documentation/locking/lockstat.rst |  4 ++--
 include/linux/lockdep.h            | 12 ++++++------
 kernel/locking/mutex.c             |  4 +---
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/Documentation/locking/lockstat.rst b/Documentation/locking/lockstat.rst
index 536eab8dbd99..3638ad1113c2 100644
--- a/Documentation/locking/lockstat.rst
+++ b/Documentation/locking/lockstat.rst
@@ -28,11 +28,11 @@ The graph below shows the relation between the lock functions and the various
             |    __contended
             |         |
             |       <wait>
+            |         |
+            |    __acquired
             | _______/
             |/
             |
-       __acquired
-            |
             .
           <hold>
             .
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 4e728d2957db..63b75ad2e17c 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -559,8 +559,8 @@ do {								\
 	if (!try(_lock)) {					\
 		lock_contended(&(_lock)->dep_map, _RET_IP_);	\
 		lock(_lock);					\
+		lock_acquired(&(_lock)->dep_map, _RET_IP_);	\
 	}							\
-	lock_acquired(&(_lock)->dep_map, _RET_IP_);			\
 } while (0)
 
 #define LOCK_CONTENDED_RETURN(_lock, try, lock)			\
@@ -569,9 +569,9 @@ do {								\
 	if (!try(_lock)) {					\
 		lock_contended(&(_lock)->dep_map, _RET_IP_);	\
 		____err = lock(_lock);				\
+		if (!____err)					\
+			lock_acquired(&(_lock)->dep_map, _RET_IP_); \
 	}							\
-	if (!____err)						\
-		lock_acquired(&(_lock)->dep_map, _RET_IP_);	\
 	____err;						\
 })
 
@@ -600,8 +600,8 @@ do {								\
 	if (!try(_lock)) {					\
 		lock_contended(&(_lock)->dep_map, _RET_IP_);	\
 		lock(_lock);					\
+		lock_acquired(&(_lock)->dep_map, _RET_IP_);	\
 	}							\
-	lock_acquired(&(_lock)->dep_map, _RET_IP_);		\
 } while (0)
 
 #define LOCK_CONTENDED_RETURN(_lock, try, lock)			\
@@ -610,9 +610,9 @@ do {								\
 	if (!try(_lock)) {					\
 		lock_contended(&(_lock)->dep_map, _RET_IP_);	\
 		____err = lock(_lock);				\
+		if (!____err)					\
+			lock_acquired(&(_lock)->dep_map, _RET_IP_); \
 	}							\
-	if (!____err)						\
-		lock_acquired(&(_lock)->dep_map, _RET_IP_);	\
 	____err;						\
 })
 
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index f8bc4ae312a0..e67b5a16440b 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -605,8 +605,6 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 
 	if (__mutex_trylock(lock) ||
 	    mutex_optimistic_spin(lock, ww_ctx, NULL)) {
-		/* got the lock, yay! */
-		lock_acquired(&lock->dep_map, ip);
 		if (ww_ctx)
 			ww_mutex_set_context_fastpath(ww, ww_ctx);
 		preempt_enable();
@@ -708,10 +706,10 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 
 	debug_mutex_free_waiter(&waiter);
 
-skip_wait:
 	/* got the lock - cleanup and rejoice! */
 	lock_acquired(&lock->dep_map, ip);
 
+skip_wait:
 	if (ww_ctx)
 		ww_mutex_lock_acquired(ww, ww_ctx);
 
-- 
2.35.0.263.gb82422642f-goog


  parent reply	other threads:[~2022-02-08 22:35 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-08 19:43 [Intel-gfx] [RFC RESEND 00/12] locking: Separate lock tracepoints from lockdep/lock_stat (v1.1) Namhyung Kim
2022-02-08 19:43 ` Namhyung Kim
2022-02-08 19:43 ` Namhyung Kim
2022-02-08 19:43 ` [PATCH 01/12] locking: Pass correct outer wait type info Namhyung Kim
2022-02-08 19:43 ` [PATCH 02/12] cgroup: rstat: Make cgroup_rstat_cpu_lock name readable Namhyung Kim
2022-02-08 19:43   ` Namhyung Kim
2022-02-09 20:20   ` Waiman Long
2022-02-09 20:20     ` Waiman Long
2022-02-09 23:29     ` Tejun Heo
2022-02-09 23:29       ` Tejun Heo
2022-02-08 19:43 ` [PATCH 03/12] timer: Protect lockdep functions with #ifdef Namhyung Kim
2022-04-10 22:46   ` Thomas Gleixner
2022-02-08 19:43 ` [PATCH 04/12] workqueue: " Namhyung Kim
2022-02-08 19:43 ` [Intel-gfx] [PATCH 05/12] drm/i915: " Namhyung Kim
2022-02-08 19:43   ` Namhyung Kim
2022-02-08 19:43 ` [PATCH 06/12] btrfs: change lockdep class size check using ks->names Namhyung Kim
2022-02-08 19:43 ` [PATCH 07/12] locking: Introduce CONFIG_LOCK_INFO Namhyung Kim
2022-02-08 19:43 ` [PATCH 08/12] locking/mutex: Init name properly w/ CONFIG_LOCK_INFO Namhyung Kim
2022-02-08 19:43 ` [PATCH 09/12] locking: Add more static lockdep init macros Namhyung Kim
2022-02-08 19:43 ` [PATCH 10/12] locking: Add CONFIG_LOCK_TRACEPOINTS option Namhyung Kim
2022-02-08 19:43 ` [PATCH 11/12] locking/mutex: Revive fast functions for CONFIG_LOCK_TRACEPOINTS Namhyung Kim
2022-02-08 19:43 ` Namhyung Kim [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-02-08 18:41 [RFC 00/12] locking: Separate lock tracepoints from lockdep/lock_stat (v1) Namhyung Kim
2022-02-08 18:42 ` [PATCH 12/12] locking: Move lock_acquired() from the fast path Namhyung Kim

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=20220208194324.85333-13-namhyung@kernel.org \
    --to=namhyung@kernel.org \
    --cc=boqun.feng@gmail.com \
    --cc=byungchul.park@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rburny@google.com \
    --cc=rostedt@goodmis.org \
    --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.