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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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))
>
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 <[email protected]>
> ---
> 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
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/[email protected]/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 <[email protected]>
> ---
> 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
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/[email protected]/
Original problem that requires devm_mutex_init and discussion:
https://lore.kernel.org/lkml/[email protected]/T/#m0f514f8f96e18db9568f84b2df37aaf648874a4a
--
Best regards
George
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/[email protected]/
>
> Original problem that requires devm_mutex_init and discussion:
> https://lore.kernel.org/lkml/[email protected]/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
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/[email protected]/
>>
>> Original problem that requires devm_mutex_init and discussion:
>> https://lore.kernel.org/lkml/[email protected]/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