Re: [PATCH] locking/mutex: Redo __mutex_init()

[Waiman Long]

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@xxxxxxxxxxxxx>
> ---
>   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