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

[Waiman Long]

On 2/21/24 23:33, Boqun Feng wrote:
> On Mon, Feb 12, 2024 at 10:16:54PM -0500, 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@xxxxxxxxxx>
> Reviewed-by: Boqun Feng <boqun.feng@xxxxxxxxx>
>
> Although, can you move the first "#ifdef CONFIG_DEBUG_MUTEXES" out of
> the "#ifndef CONFIG_PREEMPT_RT" and combine it with the second one, i.e.
>
> 	#ifdef CONFIG_DEBUG_MUTEXES
> 	# define __DEBUG_MUTEX_INITIALIZER(lockname) ...
> 	extern void mutex_destroy(struct mutex *lock);
> 	#else
> 	# define __DEBUG_MUTEX_INITIALIZER(lockname) ..
> 	static inline void mutex_destroy(struct mutex *lock) {}
> 	#endif
>
> 	#ifndef CONFIG_PREEMPT_RT
> 	...
>
> Thoughts?

Sure. I can move it up and combine the two pieces.

Thanks for the review.

Cheers,
Longman

> Regards,
> Boqun
>
> > ---
> >   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 7e208d46ba5b..bad383713db2 100644
> > --- a/include/linux/mutex.h
> > +++ b/include/linux/mutex.h
> > @@ -35,18 +35,10 @@
> >   #ifndef CONFIG_PREEMPT_RT
> >   
> >   #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
> >   
> >   /**
> > @@ -101,9 +93,6 @@ extern bool mutex_is_locked(struct mutex *lock);
> >   
> >   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)
> >   
> > @@ -170,6 +159,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_GUARD_COND(mutex, _try, mutex_trylock(_T))
> >   DEFINE_GUARD_COND(mutex, _intr, mutex_lock_interruptible(_T) == 0)
> > --
> > 2.39.3