[PATCH 1/2] locking/mutex: Clean up mutex.h

[PATCH 1/2] locking/mutex: Clean up mutex.h

[Waiman Long]

CONFIG_DEBUG_MUTEXES and CONFIG_PREEMPT_RT are mutually exclusive. They
can't be both set at the same time.  Move down the mutex_destroy()
function declaration to the bottom of mutex.h to eliminate duplicated
mutex_destroy() declaration.

Also remove the duplicated mutex_trylock() function declaration in the
CONFIG_PREEMPT_RT section.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/mutex.h | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index a33aa9eb9fc3..f3ae911580bf 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -77,18 +77,10 @@ struct mutex {
 };
 
 #ifdef CONFIG_DEBUG_MUTEXES
-
-#define __DEBUG_MUTEX_INITIALIZER(lockname)				\
+# define __DEBUG_MUTEX_INITIALIZER(lockname)				\
 	, .magic = &lockname
-
-extern void mutex_destroy(struct mutex *lock);
-
 #else
-
 # define __DEBUG_MUTEX_INITIALIZER(lockname)
-
-static inline void mutex_destroy(struct mutex *lock) {}
-
 #endif
 
 /**
@@ -151,9 +143,6 @@ struct mutex {
 
 extern void __mutex_rt_init(struct mutex *lock, const char *name,
 			    struct lock_class_key *key);
-extern int mutex_trylock(struct mutex *lock);
-
-static inline void mutex_destroy(struct mutex *lock) { }
 
 #define mutex_is_locked(l)	rt_mutex_base_is_locked(&(l)->rtmutex)
 
@@ -220,6 +209,16 @@ extern void mutex_unlock(struct mutex *lock);
 
 extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
 
+#ifdef CONFIG_DEBUG_MUTEXES
+
+extern void mutex_destroy(struct mutex *lock);
+
+#else
+
+static inline void mutex_destroy(struct mutex *lock) {}
+
+#endif
+
 DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
 DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
 
-- 
2.39.3

[PATCH 2/2] locking/rwsem: Make DEBUG_RWSEMS and PREEMPT_RT mutually exclusive

[Waiman Long]

The debugging code enabled by CONFIG_DEBUG_RWSEMS will only be
compiled in when CONFIG_PREEMPT_RT isn't set. There is no point to
allow CONFIG_DEBUG_RWSEMS to be set in a kernel configuration where
CONFIG_PREEMPT_RT is also set. Make them mutually exclusive.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 lib/Kconfig.debug | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index cc7d53d9dc01..2a95caa7e122 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1297,7 +1297,7 @@ config PROVE_LOCKING
 	select DEBUG_SPINLOCK
 	select DEBUG_MUTEXES if !PREEMPT_RT
 	select DEBUG_RT_MUTEXES if RT_MUTEXES
-	select DEBUG_RWSEMS
+	select DEBUG_RWSEMS if !PREEMPT_RT
 	select DEBUG_WW_MUTEX_SLOWPATH
 	select DEBUG_LOCK_ALLOC
 	select PREEMPT_COUNT if !ARCH_NO_PREEMPT
@@ -1420,7 +1420,7 @@ config DEBUG_WW_MUTEX_SLOWPATH
 
 config DEBUG_RWSEMS
 	bool "RW Semaphore debugging: basic checks"
-	depends on DEBUG_KERNEL
+	depends on DEBUG_KERNEL && !PREEMPT_RT
 	help
 	  This debugging feature allows mismatched rw semaphore locks
 	  and unlocks to be detected and reported.
-- 
2.39.3

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[George Stark]

Hello Waiman

Thanks for this patch! It'll be much easier to extend mutex.h with this 
change.

Just curious is there a reason we can't move mutex_init macro outside 
big #ifdef to eliminate duplicated declaration too?

Either way,
Reviewed-by: George Stark <gnstark@salutedevices.com>

On 12/16/23 04:36, Waiman Long wrote:
> CONFIG_DEBUG_MUTEXES and CONFIG_PREEMPT_RT are mutually exclusive. They
> can't be both set at the same time.  Move down the mutex_destroy()
> function declaration to the bottom of mutex.h to eliminate duplicated
> mutex_destroy() declaration.
> 
> Also remove the duplicated mutex_trylock() function declaration in the
> CONFIG_PREEMPT_RT section.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>   include/linux/mutex.h | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index a33aa9eb9fc3..f3ae911580bf 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -77,18 +77,10 @@ struct mutex {
>   };
>   
>   #ifdef CONFIG_DEBUG_MUTEXES
> -
> -#define __DEBUG_MUTEX_INITIALIZER(lockname)				\
> +# define __DEBUG_MUTEX_INITIALIZER(lockname)				\
>   	, .magic = &lockname
> -
> -extern void mutex_destroy(struct mutex *lock);
> -
>   #else
> -
>   # define __DEBUG_MUTEX_INITIALIZER(lockname)
> -
> -static inline void mutex_destroy(struct mutex *lock) {}
> -
>   #endif
>   
>   /**
> @@ -151,9 +143,6 @@ struct mutex {
>   
>   extern void __mutex_rt_init(struct mutex *lock, const char *name,
>   			    struct lock_class_key *key);
> -extern int mutex_trylock(struct mutex *lock);
> -
> -static inline void mutex_destroy(struct mutex *lock) { }
>   
>   #define mutex_is_locked(l)	rt_mutex_base_is_locked(&(l)->rtmutex)
>   
> @@ -220,6 +209,16 @@ extern void mutex_unlock(struct mutex *lock);
>   
>   extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
>   
> +#ifdef CONFIG_DEBUG_MUTEXES
> +
> +extern void mutex_destroy(struct mutex *lock);
> +
> +#else
> +
> +static inline void mutex_destroy(struct mutex *lock) {}
> +
> +#endif
> +
>   DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
>   DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
>   

-- 
Best regards
George

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[Christophe JAILLET]

Le 16/12/2023 à 02:36, Waiman Long a écrit :
> CONFIG_DEBUG_MUTEXES and CONFIG_PREEMPT_RT are mutually exclusive. They
> can't be both set at the same time.  Move down the mutex_destroy()
> function declaration to the bottom of mutex.h to eliminate duplicated
> mutex_destroy() declaration.
> 
> Also remove the duplicated mutex_trylock() function declaration in the
> CONFIG_PREEMPT_RT section.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>   include/linux/mutex.h | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index a33aa9eb9fc3..f3ae911580bf 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -77,18 +77,10 @@ struct mutex {
>   };
>   
>   #ifdef CONFIG_DEBUG_MUTEXES
> -
> -#define __DEBUG_MUTEX_INITIALIZER(lockname)				\
> +# define __DEBUG_MUTEX_INITIALIZER(lockname)				\

Is this extra space added on purpose?

CJ

>   	, .magic = &lockname
> -
> -extern void mutex_destroy(struct mutex *lock);
> -
>   #else
> -
>   # define __DEBUG_MUTEX_INITIALIZER(lockname)
> -
> -static inline void mutex_destroy(struct mutex *lock) {}
> -
>   #endif
>   
>   /**
> @@ -151,9 +143,6 @@ struct mutex {
>   
>   extern void __mutex_rt_init(struct mutex *lock, const char *name,
>   			    struct lock_class_key *key);
> -extern int mutex_trylock(struct mutex *lock);
> -
> -static inline void mutex_destroy(struct mutex *lock) { }
>   
>   #define mutex_is_locked(l)	rt_mutex_base_is_locked(&(l)->rtmutex)
>   
> @@ -220,6 +209,16 @@ extern void mutex_unlock(struct mutex *lock);
>   
>   extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
>   
> +#ifdef CONFIG_DEBUG_MUTEXES
> +
> +extern void mutex_destroy(struct mutex *lock);
> +
> +#else
> +
> +static inline void mutex_destroy(struct mutex *lock) {}
> +
> +#endif
> +
>   DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
>   DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
>   

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[George Stark]

Hello Waiman

Do you have plans to merge it? Just kindly remind that we're trying to 
implement devm_mutex_destroy() and with this patch the solution would be 
straightforward.


On 12/16/23 04:36, Waiman Long wrote:
> CONFIG_DEBUG_MUTEXES and CONFIG_PREEMPT_RT are mutually exclusive. They
> can't be both set at the same time.  Move down the mutex_destroy()
> function declaration to the bottom of mutex.h to eliminate duplicated
> mutex_destroy() declaration.
> 
> Also remove the duplicated mutex_trylock() function declaration in the
> CONFIG_PREEMPT_RT section.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>   include/linux/mutex.h | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index a33aa9eb9fc3..f3ae911580bf 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -77,18 +77,10 @@ struct mutex {
>   };
>   
>   #ifdef CONFIG_DEBUG_MUTEXES
> -
> -#define __DEBUG_MUTEX_INITIALIZER(lockname)				\
> +# define __DEBUG_MUTEX_INITIALIZER(lockname)				\
>   	, .magic = &lockname
> -
> -extern void mutex_destroy(struct mutex *lock);
> -
>   #else
> -
>   # define __DEBUG_MUTEX_INITIALIZER(lockname)
> -
> -static inline void mutex_destroy(struct mutex *lock) {}
> -
>   #endif
>   
>   /**
> @@ -151,9 +143,6 @@ struct mutex {
>   
>   extern void __mutex_rt_init(struct mutex *lock, const char *name,
>   			    struct lock_class_key *key);
> -extern int mutex_trylock(struct mutex *lock);
> -
> -static inline void mutex_destroy(struct mutex *lock) { }
>   
>   #define mutex_is_locked(l)	rt_mutex_base_is_locked(&(l)->rtmutex)
>   
> @@ -220,6 +209,16 @@ extern void mutex_unlock(struct mutex *lock);
>   
>   extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
>   
> +#ifdef CONFIG_DEBUG_MUTEXES
> +
> +extern void mutex_destroy(struct mutex *lock);
> +
> +#else
> +
> +static inline void mutex_destroy(struct mutex *lock) {}
> +
> +#endif
> +
>   DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
>   DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
>   

-- 
Best regards
George

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[George Stark]

Hello Waiman

Sorry for disturbing you. I just wanted to know are there plans to 
develop / merge this patch? We're trying to implement 
devm_mutex_destroy() and with this patch the solution would be 
straightforward.

Here is the patch series and discussion why devm_mutex_destroy()'d be 
helpful:
https://lore.kernel.org/lkml/c709e0f33da06db40127f3a0adcbebbd@trvn.ru/T/#m885a348ea3f7c8d9a30bcfaf025a08e053c02c18


On 12/16/23 04:36, Waiman Long wrote:
> CONFIG_DEBUG_MUTEXES and CONFIG_PREEMPT_RT are mutually exclusive. They
> can't be both set at the same time.  Move down the mutex_destroy()
> function declaration to the bottom of mutex.h to eliminate duplicated
> mutex_destroy() declaration.
> 
> Also remove the duplicated mutex_trylock() function declaration in the
> CONFIG_PREEMPT_RT section.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>   include/linux/mutex.h | 23 +++++++++++------------
>   1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index a33aa9eb9fc3..f3ae911580bf 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -77,18 +77,10 @@ struct mutex {
>   };
>   
>   #ifdef CONFIG_DEBUG_MUTEXES
> -
> -#define __DEBUG_MUTEX_INITIALIZER(lockname)				\
> +# define __DEBUG_MUTEX_INITIALIZER(lockname)				\
>   	, .magic = &lockname
> -
> -extern void mutex_destroy(struct mutex *lock);
> -
>   #else
> -
>   # define __DEBUG_MUTEX_INITIALIZER(lockname)
> -
> -static inline void mutex_destroy(struct mutex *lock) {}
> -
>   #endif
>   
>   /**
> @@ -151,9 +143,6 @@ struct mutex {
>   
>   extern void __mutex_rt_init(struct mutex *lock, const char *name,
>   			    struct lock_class_key *key);
> -extern int mutex_trylock(struct mutex *lock);
> -
> -static inline void mutex_destroy(struct mutex *lock) { }
>   
>   #define mutex_is_locked(l)	rt_mutex_base_is_locked(&(l)->rtmutex)
>   
> @@ -220,6 +209,16 @@ extern void mutex_unlock(struct mutex *lock);
>   
>   extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock);
>   
> +#ifdef CONFIG_DEBUG_MUTEXES
> +
> +extern void mutex_destroy(struct mutex *lock);
> +
> +#else
> +
> +static inline void mutex_destroy(struct mutex *lock) {}
> +
> +#endif
> +
>   DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
>   DEFINE_FREE(mutex, struct mutex *, if (_T) mutex_unlock(_T))
>   

-- 
Best regards
George

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[George Stark]

Hello

Excuse me, this is my 4th letter here since December 2023 and there's no 
response unfortunately.


Just to recall that we had the discussion in December 2023 about 
implementing devm_mutex_init. We came to conclusion that the only 
effective way to do it in include/linux/mutex.h but mutex.h requires
some cleanups to simplify the devm_mutex_init patch.
mutex.h owners proposed such a cleanup patch themselves and there's no 
progress since that. How can we move forward on those patches?

Cleanup patch:
https://lore.kernel.org/lkml/20231216013656.1382213-1-longman@redhat.com/

Original problem that requires devm_mutex_init and discussion:
https://lore.kernel.org/lkml/ZcZeRmVjEPhVPW9-@smile.fi.intel.com/T/#m0f514f8f96e18db9568f84b2df37aaf648874a4a

-- 
Best regards
George

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[Waiman Long]

On 2/11/24 18:28, George Stark wrote:
> Hello
>
> Excuse me, this is my 4th letter here since December 2023 and there's 
> no response unfortunately.
>
>
> Just to recall that we had the discussion in December 2023 about 
> implementing devm_mutex_init. We came to conclusion that the only 
> effective way to do it in include/linux/mutex.h but mutex.h requires
> some cleanups to simplify the devm_mutex_init patch.
> mutex.h owners proposed such a cleanup patch themselves and there's no 
> progress since that. How can we move forward on those patches?
>
> Cleanup patch:
> https://lore.kernel.org/lkml/20231216013656.1382213-1-longman@redhat.com/
>
> Original problem that requires devm_mutex_init and discussion:
> https://lore.kernel.org/lkml/ZcZeRmVjEPhVPW9-@smile.fi.intel.com/T/#m0f514f8f96e18db9568f84b2df37aaf648874a4a 
>

Sorry for my late reply. My cleanup patch was sent at the later half of 
Dec 2023. It is probably not a good time as maintainers are either busy 
or taking time off. I have resent the patch again with other cleanup 
patches. Hopefully those patches can be merged soon.

Cheers,
Longman

Re: [PATCH 1/2] locking/mutex: Clean up mutex.h

[George Stark]

On 2/13/24 06:28, Waiman Long wrote:
> 
> On 2/11/24 18:28, George Stark wrote:
>> Hello
>>
>> Excuse me, this is my 4th letter here since December 2023 and there's 
>> no response unfortunately.
>>
>>
>> Just to recall that we had the discussion in December 2023 about 
>> implementing devm_mutex_init. We came to conclusion that the only 
>> effective way to do it in include/linux/mutex.h but mutex.h requires
>> some cleanups to simplify the devm_mutex_init patch.
>> mutex.h owners proposed such a cleanup patch themselves and there's no 
>> progress since that. How can we move forward on those patches?
>>
>> Cleanup patch:
>> https://lore.kernel.org/lkml/20231216013656.1382213-1-longman@redhat.com/
>>
>> Original problem that requires devm_mutex_init and discussion:
>> https://lore.kernel.org/lkml/ZcZeRmVjEPhVPW9-@smile.fi.intel.com/T/#m0f514f8f96e18db9568f84b2df37aaf648874a4a
> 
> Sorry for my late reply. My cleanup patch was sent at the later half of 
> Dec 2023. It is probably not a good time as maintainers are either busy 
> or taking time off. I have resent the patch again with other cleanup 
> patches. Hopefully those patches can be merged soon.

Hello Waiman.

Good to see you again. I'm sorry if I was too impatient. Thanks for 
updating your patch series.

> 
> Cheers,
> Longman
> 

-- 
Best regards
George