[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4207482b-fc63-4db7-ab98-36b31a600173@redhat.com>
Date: Tue, 4 Nov 2025 11:21:27 -0500
From: Waiman Long <llong@...hat.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
linux-kernel@...r.kernel.org
Cc: Boqun Feng <boqun.feng@...il.com>, Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>, Will Deacon <will@...nel.org>
Subject: Re: [PATCH] locking/mutex: Redo __mutex_init()
On 11/4/25 9:00 AM, Sebastian Andrzej Siewior wrote:
> mutex_init() invokes __mutex_init() providing the name of the lock and
> a pointer to a the lock class. With LOCKDEP enabled this information is
> useful but without LOCKDEP it not used at all. Passing the pointer
> information of the lock class might be considered negligible but the
> name of the lock is passed as well and the string is stored. This
> information is wasting storage.
>
> Split __mutex_init() into a _plain() variant doing the initialisation of
> the lock and a _ld() version which does _plain() plus the lockdep bits.
> Restrict the lockdep version to lockdep enabled builds allowing the
> compiler to remove the unused parameter.
>
> This results in the following size reduction:
>
> text data bss dec filename
> | 30237599 8161430 1176624 39575653 vmlinux.defconfig
> | 30233269 8149142 1176560 39558971 vmlinux.defconfig.patched
> -4.2KiB -12KiB
>
> | 32455099 8471098 12934684 53860881 vmlinux.defconfig.lockdep
> | 32455100 8471098 12934684 53860882 vmlinux.defconfig.patched.lockdep
>
> | 27152407 7191822 2068040 36412269 vmlinux.defconfig.preempt_rt
> | 27145937 7183630 2067976 36397543 vmlinux.defconfig.patched.preempt_rt
> -6.3KiB -8KiB
>
> | 29382020 7505742 13784608 50672370 vmlinux.defconfig.preempt_rt.lockdep
> | 29376229 7505742 13784544 50666515 vmlinux.defconfig.patched.preempt_rt.lockdep
> -5.6KiB
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
> include/linux/mutex.h | 45 ++++++++++++++++++++++++++++--------
> kernel/locking/mutex.c | 22 +++++++++++++-----
> kernel/locking/rtmutex_api.c | 19 +++++++++++----
> 3 files changed, 66 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index 847b81ca64368..e731ef82aa0a0 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -86,8 +86,23 @@ do { \
> #define DEFINE_MUTEX(mutexname) \
> struct mutex mutexname = __MUTEX_INITIALIZER(mutexname)
>
> -extern void __mutex_init(struct mutex *lock, const char *name,
> - struct lock_class_key *key);
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +void mutex_init_ld(struct mutex *lock, const char *name, struct lock_class_key *key);
> +
> +static inline void __mutex_init(struct mutex *lock, const char *name,
> + struct lock_class_key *key)
> +{
> + mutex_init_ld(lock, name, key);
> +}
> +#else
> +extern void mutex_init_plain(struct mutex *lock);
> +
> +static inline void __mutex_init(struct mutex *lock, const char *name,
> + struct lock_class_key *key)
> +{
> + mutex_init_plain(lock);
> +}
> +#endif /* !CONFIG_DEBUG_LOCK_ALLOC */
I think it is a good idea to eliminate useless strings in non-lockdep
kernel. However, the function names are kind of awkward to me. First of
all, it is hard to associate "ld" with lockdep as ld is also the name of
the GNU linker. I would prefer to fully spell out as "lockdep". The
"_plain" suffix also looks odd to me. How about using the original
__mutex_init for the plain version and __mutex_init_lockdep as the
lockdep version which calls __mutex_init and use similar naming scheme
for the RT versions. What do you think?
Cheers,
Longman
Powered by blists - more mailing lists