* [PATCH 2/2] locking/rwsem: Make DEBUG_RWSEMS and PREEMPT_RT mutually exclusive
2023-12-16 1:36 [PATCH 1/2] locking/mutex: Clean up mutex.h Waiman Long
@ 2023-12-16 1:36 ` Waiman Long
2023-12-16 11:47 ` [PATCH 1/2] locking/mutex: Clean up mutex.h George Stark
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Waiman Long @ 2023-12-16 1:36 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Andrew Morton
Cc: linux-kernel, George Stark, 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2023-12-16 1:36 [PATCH 1/2] locking/mutex: Clean up mutex.h Waiman Long
2023-12-16 1:36 ` [PATCH 2/2] locking/rwsem: Make DEBUG_RWSEMS and PREEMPT_RT mutually exclusive Waiman Long
@ 2023-12-16 11:47 ` George Stark
2023-12-16 12:07 ` Christophe JAILLET
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: George Stark @ 2023-12-16 11:47 UTC (permalink / raw)
To: Waiman Long, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, Andrew Morton
Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2023-12-16 1:36 [PATCH 1/2] locking/mutex: Clean up mutex.h Waiman Long
2023-12-16 1:36 ` [PATCH 2/2] locking/rwsem: Make DEBUG_RWSEMS and PREEMPT_RT mutually exclusive Waiman Long
2023-12-16 11:47 ` [PATCH 1/2] locking/mutex: Clean up mutex.h George Stark
@ 2023-12-16 12:07 ` Christophe JAILLET
2024-01-09 10:49 ` George Stark
2024-01-28 18:28 ` George Stark
4 siblings, 0 replies; 9+ messages in thread
From: Christophe JAILLET @ 2023-12-16 12:07 UTC (permalink / raw)
To: Waiman Long, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, Andrew Morton
Cc: linux-kernel, George Stark
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))
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2023-12-16 1:36 [PATCH 1/2] locking/mutex: Clean up mutex.h Waiman Long
` (2 preceding siblings ...)
2023-12-16 12:07 ` Christophe JAILLET
@ 2024-01-09 10:49 ` George Stark
2024-01-28 18:28 ` George Stark
4 siblings, 0 replies; 9+ messages in thread
From: George Stark @ 2024-01-09 10:49 UTC (permalink / raw)
To: Waiman Long
Cc: linux-kernel, Boqun Feng, Ingo Molnar, Andrew Morton,
Peter Zijlstra, Will Deacon
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2023-12-16 1:36 [PATCH 1/2] locking/mutex: Clean up mutex.h Waiman Long
` (3 preceding siblings ...)
2024-01-09 10:49 ` George Stark
@ 2024-01-28 18:28 ` George Stark
2024-02-11 23:28 ` George Stark
4 siblings, 1 reply; 9+ messages in thread
From: George Stark @ 2024-01-28 18:28 UTC (permalink / raw)
To: Waiman Long
Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng,
Andrew Morton, linux-kernel, gnstark
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2024-01-28 18:28 ` George Stark
@ 2024-02-11 23:28 ` George Stark
2024-02-13 3:28 ` Waiman Long
0 siblings, 1 reply; 9+ messages in thread
From: George Stark @ 2024-02-11 23:28 UTC (permalink / raw)
To: Waiman Long, Peter Zijlstra, Boqun Feng, Will Deacon,
Ingo Molnar, Andrew Morton
Cc: gregkh, linux-kernel, kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2024-02-11 23:28 ` George Stark
@ 2024-02-13 3:28 ` Waiman Long
2024-02-13 19:51 ` George Stark
0 siblings, 1 reply; 9+ messages in thread
From: Waiman Long @ 2024-02-13 3:28 UTC (permalink / raw)
To: George Stark, Peter Zijlstra, Boqun Feng, Will Deacon,
Ingo Molnar, Andrew Morton
Cc: gregkh, linux-kernel, kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] locking/mutex: Clean up mutex.h
2024-02-13 3:28 ` Waiman Long
@ 2024-02-13 19:51 ` George Stark
0 siblings, 0 replies; 9+ messages in thread
From: George Stark @ 2024-02-13 19:51 UTC (permalink / raw)
To: Waiman Long, Peter Zijlstra, Boqun Feng, Will Deacon,
Ingo Molnar, Andrew Morton
Cc: gregkh, linux-kernel, kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread