Messages in this thread | | | Date | Fri, 5 Apr 2024 22:39:55 -0400 | Subject | Re: [RFC][PATCH] locking/rwsem: Add __always_inline annotation to __down_write_common() and inlined callers | From | Waiman Long <> |
| |
On 4/5/24 16:05, John Stultz wrote: > Apparently despite it being marked inline, the compiler > may not inline __down_write_common() which makes it difficult > to identify the cause of lock contention, as the blocked > function in traceevents will always be listed as > __down_write_common(). > > So add __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. > > This mirrors commit 92cc5d00a431 ("locking/rwsem: Add > __always_inline annotation to __down_read_common() and inlined > callers") which did the same for __down_read_common. > > I sort of worry that I'm playing wack-a-mole here, and talking > with compiler people, they tell me inline means nothing, which > makes me want to cry a little. So I'm wondering if we need to > replace all the inlines with __always_inline, or remove them > because either we mean something by it, or not. > > Cc: Tim Murray <timmurray@google.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Will Deacon <will@kernel.org> > Cc: Waiman Long <longman@redhat.com> > Cc: Boqun Feng <boqun.feng@gmail.com> > Cc: kernel-team@android.com > Fixes: c995e638ccbb ("locking/rwsem: Fold __down_{read,write}*()") > Reported-by: Tim Murray <timmurray@google.com> > Signed-off-by: John Stultz <jstultz@google.com> > --- > kernel/locking/rwsem.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c > index c6d17aee4209..33cac79e3994 100644 > --- a/kernel/locking/rwsem.c > +++ b/kernel/locking/rwsem.c > @@ -1297,7 +1297,7 @@ static inline int __down_read_trylock(struct rw_semaphore *sem) > /* > * lock for writing > */ > -static inline int __down_write_common(struct rw_semaphore *sem, int state) > +static __always_inline int __down_write_common(struct rw_semaphore *sem, int state) > { > int ret = 0; > > @@ -1310,12 +1310,12 @@ static inline int __down_write_common(struct rw_semaphore *sem, int state) > return ret; > } > > -static inline void __down_write(struct rw_semaphore *sem) > +static __always_inline void __down_write(struct rw_semaphore *sem) > { > __down_write_common(sem, TASK_UNINTERRUPTIBLE); > } > > -static inline int __down_write_killable(struct rw_semaphore *sem) > +static __always_inline int __down_write_killable(struct rw_semaphore *sem) > { > return __down_write_common(sem, TASK_KILLABLE); > }
Whether inlining happens or not really depends on the compiler used. Anyway,
Acked-by: Waiman Long <longman@redhat.com>
Thanks, Longman
| |