Skip to contentSkip to conversation

locking/rwsem: Add __always_inline annotation to __down_read_common() and inlined callers

PATCH Applied John Stultz 3 participants 3 messages May 3, 2023 – May 10, 2023

Subsystems (2)
Stable branch Locking primitives
lore
Share & tools
lore

John Stultz

author
#0 lore raw

PATCH v3 1/1 locking/rwsem: Add __always_inline annotation to __down_read_common() and inlined callers

Apparently despite it being marked inline, the compiler may not inline __down_read_common() which makes it difficult to identify the cause of lock contention, as the blocked function in traceevents will always be listed as __down_read_common().

So this patch adds __always_inline annotation to the common function (as well as the inlined helper callers) to force it to be inlined so the blocking function will be listed (via Wchan) in traceevents.

Cc Minchan Kim <[email protected]>
Cc Tim Murray <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Will Deacon <[email protected]>
Cc Waiman Long <[email protected]>
Cc Boqun Feng <[email protected]>
Cc [email protected]
Cc [email protected]
Fixes c995e638ccbb ("locking/rwsem: Fold __down_{read,write}*()")
Reviewed-by Waiman Long <[email protected]>
Reported-by Tim Murray <[email protected]>
Signed-off-by John Stultz <[email protected]>

---
v2: Reworked to use __always_inline instead of __sched as
suggested by Waiman Long

v3: Add __always_inline annotations to currently inlined users
of __down_read_common() to avoid the compiler later doing the same thing there. (Suggested by Peter).

---
 kernel/locking/rwsem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/locking/rwsem.c +4 -4
index acb5a50309a1..9eabd585ce7a 100644
@@ -1240,7 +1240,7 @@ static struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
 /*
  * lock for reading
  */
-static inline int __down_read_common(struct rw_semaphore *sem, int state)
+static __always_inline int __down_read_common(struct rw_semaphore *sem, int state)
 {
 	int ret = 0;
 	long count;

@@ -1258,17 +1258,17 @@ static inline int __down_read_common(struct rw_semaphore *sem, int state)
 	return ret;
 }
 
-static inline void __down_read(struct rw_semaphore *sem)
+static __always_inline void __down_read(struct rw_semaphore *sem)
 {
 	__down_read_common(sem, TASK_UNINTERRUPTIBLE);
 }
 
-static inline int __down_read_interruptible(struct rw_semaphore *sem)
+static __always_inline int __down_read_interruptible(struct rw_semaphore *sem)
 {
 	return __down_read_common(sem, TASK_INTERRUPTIBLE);
 }
 
-static inline int __down_read_killable(struct rw_semaphore *sem)
+static __always_inline int __down_read_killable(struct rw_semaphore *sem)
 {
 	return __down_read_common(sem, TASK_KILLABLE);
 }
Signature · 1 line
2.40.1.495.gc816e09b53d-goog
maintainer
#1 lore raw
Quoted 11 lines · John Stultz

Apparently despite it being marked inline, the compiler may not inline __down_read_common() which makes it difficult to identify the cause of lock contention, as the blocked function in traceevents will always be listed as __down_read_common().

So this patch adds __always_inline annotation to the common function (as well as the inlined helper callers) to force it to be inlined so the blocking function will be listed (via Wchan) in traceevents.

Quoted 13 lines
Cc Minchan Kim <[email protected]>
Cc Tim Murray <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Will Deacon <[email protected]>
Cc Waiman Long <[email protected]>
Cc Boqun Feng <[email protected]>
Cc [email protected]
Cc [email protected]
Fixes c995e638ccbb ("locking/rwsem: Fold __down_{read,write}*()")
Reviewed-by Waiman Long <[email protected]>
Reported-by Tim Murray <[email protected]>
Signed-off-by John Stultz <[email protected]>

Thanks!

merge bot Applied
#2 lore raw
In reply to John Stultz
The following commit has been merged into the locking/urgent branch of tip:

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     92cc5d00a431e96e5a49c0b97e5ad4fa7536bd4b
Gitweb:        https://git.kernel.org/tip/92cc5d00a431e96e5a49c0b97e5ad4fa7536bd4b
Author:        John Stultz <[email protected]>
AuthorDate:    Wed, 03 May 2023 02:33:51 
Committer:     Peter Zijlstra <[email protected]>
CommitterDate: Mon, 08 May 2023 10:58:24 +02:00

locking/rwsem: Add __always_inline annotation to __down_read_common() and inlined callers

Apparently despite it being marked inline, the compiler may not inline __down_read_common() which makes it difficult to identify the cause of lock contention, as the blocked function in traceevents will always be listed as __down_read_common().

So this patch adds __always_inline annotation to the common function (as well as the inlined helper callers) to force it to be inlined so the blocking function will be listed (via Wchan) in traceevents.

Fixes c995e638ccbb ("locking/rwsem: Fold __down_{read,write}*()")
Reported-by Tim Murray <[email protected]>
Signed-off-by John Stultz <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Reviewed-by Waiman Long <[email protected]>
Cc [email protected]
Link https://lkml.kernel.org/r/[email protected] open here
---
 kernel/locking/rwsem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/locking/rwsem.c +4 -4
index acb5a50..9eabd58 100644
@@ -1240,7 +1240,7 @@ static struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
 /*
  * lock for reading
  */
-static inline int __down_read_common(struct rw_semaphore *sem, int state)
+static __always_inline int __down_read_common(struct rw_semaphore *sem, int state)
 {
 	int ret = 0;
 	long count;

@@ -1258,17 +1258,17 @@ out:
 	return ret;
 }
 
-static inline void __down_read(struct rw_semaphore *sem)
+static __always_inline void __down_read(struct rw_semaphore *sem)
 {
 	__down_read_common(sem, TASK_UNINTERRUPTIBLE);
 }
 
-static inline int __down_read_interruptible(struct rw_semaphore *sem)
+static __always_inline int __down_read_interruptible(struct rw_semaphore *sem)
 {
 	return __down_read_common(sem, TASK_INTERRUPTIBLE);
 }
 
-static inline int __down_read_killable(struct rw_semaphore *sem)
+static __always_inline int __down_read_killable(struct rw_semaphore *sem)
 {
 	return __down_read_common(sem, TASK_KILLABLE);
 }