[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8e56c3d7-28b7-4931-95d1-8e311a16544b@redhat.com>
Date: Wed, 5 Nov 2025 11:22:37 -0500
From: Waiman Long <llong@...hat.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
linux-kernel@...r.kernel.org
Cc: Waiman Long <llong@...hat.com>, Boqun Feng <boqun.feng@...il.com>,
Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
Will Deacon <will@...nel.org>
Subject: Re: [PATCH v2] locking/mutex: Redo __mutex_init()
On 11/5/25 9:23 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 _genereic() variant doing the initialisation
> of the lock and a _lockdep() version which does _genereic() 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>
> ---
> v1…v2:
> - s/_init_ld/_init_lockep/
> - s/_init_plain/_init_generic/
>
> 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..bf535f0118bb8 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_lockep(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_lockep(lock, name, key);
> +}
> +#else
> +extern void mutex_init_generic(struct mutex *lock);
> +
> +static inline void __mutex_init(struct mutex *lock, const char *name,
> + struct lock_class_key *key)
> +{
> + mutex_init_generic(lock);
> +}
> +#endif /* !CONFIG_DEBUG_LOCK_ALLOC */
>
> /**
> * mutex_is_locked - is the mutex locked
> @@ -111,17 +126,27 @@ extern bool mutex_is_locked(struct mutex *lock);
> #define DEFINE_MUTEX(mutexname) \
> struct mutex mutexname = __MUTEX_INITIALIZER(mutexname)
>
> -extern void __mutex_rt_init(struct mutex *lock, const char *name,
> - struct lock_class_key *key);
> -
> #define mutex_is_locked(l) rt_mutex_base_is_locked(&(l)->rtmutex)
>
> -#define __mutex_init(mutex, name, key) \
> -do { \
> - rt_mutex_base_init(&(mutex)->rtmutex); \
> - __mutex_rt_init((mutex), name, key); \
> -} while (0)
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +extern void mutex_rt_init_lockdep(struct mutex *mutex, 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_rt_init_lockdep(lock, name, key);
> +}
> +
> +#else
> +extern void mutex_rt_init_generic(struct mutex *mutex);
> +
> +static inline void __mutex_init(struct mutex *lock, const char *name,
> + struct lock_class_key *key)
> +{
> + mutex_rt_init_generic(lock);
> +}
> +#endif /* !CONFIG_LOCKDEP */
> #endif /* CONFIG_PREEMPT_RT */
>
> #ifdef CONFIG_DEBUG_MUTEXES
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index de7d6702cd96c..f3bb352a368d9 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -43,8 +43,7 @@
> # define MUTEX_WARN_ON(cond)
> #endif
>
> -void
> -__mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
> +static void __mutex_init_generic(struct mutex *lock)
> {
> atomic_long_set(&lock->owner, 0);
> raw_spin_lock_init(&lock->wait_lock);
> @@ -52,10 +51,7 @@ __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
> #ifdef CONFIG_MUTEX_SPIN_ON_OWNER
> osq_lock_init(&lock->osq);
> #endif
> -
> - debug_mutex_init(lock, name, key);
> }
> -EXPORT_SYMBOL(__mutex_init);
>
> static inline struct task_struct *__owner_task(unsigned long owner)
> {
> @@ -142,6 +138,11 @@ static inline bool __mutex_trylock(struct mutex *lock)
> * There is nothing that would stop spreading the lockdep annotations outwards
> * except more code.
> */
> +void mutex_init_generic(struct mutex *lock)
> +{
> + __mutex_init_generic(lock);
> +}
> +EXPORT_SYMBOL(mutex_init_generic);
>
> /*
> * Optimistic trylock that only works in the uncontended case. Make sure to
> @@ -166,7 +167,16 @@ static __always_inline bool __mutex_unlock_fast(struct mutex *lock)
>
> return atomic_long_try_cmpxchg_release(&lock->owner, &curr, 0UL);
> }
> -#endif
> +
> +#else /* !CONFIG_DEBUG_LOCK_ALLOC */
> +
> +void mutex_init_lockep(struct mutex *lock, const char *name, struct lock_class_key *key)
> +{
> + __mutex_init_generic(lock);
> + debug_mutex_init(lock, name, key);
> +}
> +EXPORT_SYMBOL(mutex_init_lockep);
> +#endif /* !CONFIG_DEBUG_LOCK_ALLOC */
>
> static inline void __mutex_set_flag(struct mutex *lock, unsigned long flag)
> {
> diff --git a/kernel/locking/rtmutex_api.c b/kernel/locking/rtmutex_api.c
> index bafd5af98eaec..59dbd29cb219b 100644
> --- a/kernel/locking/rtmutex_api.c
> +++ b/kernel/locking/rtmutex_api.c
> @@ -515,13 +515,11 @@ void rt_mutex_debug_task_free(struct task_struct *task)
>
> #ifdef CONFIG_PREEMPT_RT
> /* Mutexes */
> -void __mutex_rt_init(struct mutex *mutex, const char *name,
> - struct lock_class_key *key)
> +static void __mutex_rt_init_generic(struct mutex *mutex)
> {
> + rt_mutex_base_init(&mutex->rtmutex);
> debug_check_no_locks_freed((void *)mutex, sizeof(*mutex));
> - lockdep_init_map_wait(&mutex->dep_map, name, key, 0, LD_WAIT_SLEEP);
> }
> -EXPORT_SYMBOL(__mutex_rt_init);
>
> static __always_inline int __mutex_lock_common(struct mutex *lock,
> unsigned int state,
> @@ -542,6 +540,13 @@ static __always_inline int __mutex_lock_common(struct mutex *lock,
> }
>
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
> +void mutex_rt_init_lockdep(struct mutex *mutex, const char *name, struct lock_class_key *key)
> +{
> + __mutex_rt_init_generic(mutex);
> + lockdep_init_map_wait(&mutex->dep_map, name, key, 0, LD_WAIT_SLEEP);
> +}
> +EXPORT_SYMBOL(mutex_rt_init_lockdep);
> +
> void __sched mutex_lock_nested(struct mutex *lock, unsigned int subclass)
> {
> __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, subclass, NULL, _RET_IP_);
> @@ -598,6 +603,12 @@ int __sched _mutex_trylock_nest_lock(struct mutex *lock,
> EXPORT_SYMBOL_GPL(_mutex_trylock_nest_lock);
> #else /* CONFIG_DEBUG_LOCK_ALLOC */
>
> +void mutex_rt_init_generic(struct mutex *mutex)
> +{
> + __mutex_rt_init_generic(mutex);
> +}
> +EXPORT_SYMBOL(mutex_rt_init_generic);
> +
> void __sched mutex_lock(struct mutex *lock)
> {
> __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, NULL, _RET_IP_);
Reviewed-by: Waiman Long <longman@...hat.com>
Powered by blists - more mailing lists