Messages in this thread | | | Subject | Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value | From | Mukesh Ojha <> | Date | Tue, 30 Jul 2019 21:55:38 +0530 |
| |
On 7/30/2019 9:32 PM, Peter Zijlstra wrote: > On Tue, Jul 30, 2019 at 06:10:49PM +0530, Mukesh Ojha wrote: > >> To make it static , i have to export mutex_is_locked() after moving it >> inside mutex.c, so that other module can use it. > Yep, see below -- completely untested. > >> Also are we thinking of removing >> static inline /* __deprecated */ __must_check enum >> mutex_trylock_recursive_enum >> mutex_trylock_recursive(struct mutex *lock) >> >> inside linux/mutex.h in future ? >> >> As i see it is used at one or two places and there is a check inside >> checkpatch guarding its further use . > That was the idea; recursive locking is evil, but we have these two > legacy sites. > > --- > > diff --git a/include/linux/mutex.h b/include/linux/mutex.h > index dcd03fee6e01..eb8c62aba263 100644 > --- a/include/linux/mutex.h > +++ b/include/linux/mutex.h > @@ -65,29 +65,6 @@ struct mutex { > #endif > }; > > -/* > - * Internal helper function; C doesn't allow us to hide it :/ > - * > - * DO NOT USE (outside of mutex code). > - */ > -static inline struct task_struct *__mutex_owner(struct mutex *lock) > -{ > - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); > -} > - > -/* > - * This is the control structure for tasks blocked on mutex, > - * which resides on the blocked task's kernel stack: > - */ > -struct mutex_waiter { > - struct list_head list; > - struct task_struct *task; > - struct ww_acquire_ctx *ww_ctx; > -#ifdef CONFIG_DEBUG_MUTEXES > - void *magic; > -#endif > -}; > - > #ifdef CONFIG_DEBUG_MUTEXES > > #define __DEBUG_MUTEX_INITIALIZER(lockname) \ > @@ -144,10 +121,7 @@ extern void __mutex_init(struct mutex *lock, const char *name, > * > * Returns true if the mutex is locked, false if unlocked. > */ > -static inline bool mutex_is_locked(struct mutex *lock) > -{ > - return __mutex_owner(lock) != NULL; > -} > +extern bool mutex_is_locked(struct mutex *lock); > > /* > * See kernel/locking/mutex.c for detailed documentation of these APIs. > @@ -220,13 +194,7 @@ enum mutex_trylock_recursive_enum { > * - MUTEX_TRYLOCK_SUCCESS - lock acquired, > * - MUTEX_TRYLOCK_RECURSIVE - we already owned the lock. > */ > -static inline /* __deprecated */ __must_check enum mutex_trylock_recursive_enum > -mutex_trylock_recursive(struct mutex *lock) > -{ > - if (unlikely(__mutex_owner(lock) == current)) > - return MUTEX_TRYLOCK_RECURSIVE; > - > - return mutex_trylock(lock); > -} > +extern /* __deprecated */ __must_check enum mutex_trylock_recursive_enum > +mutex_trylock_recursive(struct mutex *lock); > > #endif /* __LINUX_MUTEX_H */ > diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c > index 5e069734363c..2f73935a6053 100644 > --- a/kernel/locking/mutex.c > +++ b/kernel/locking/mutex.c > @@ -36,6 +36,19 @@ > # include "mutex.h" > #endif > > +/* > + * This is the control structure for tasks blocked on mutex, > + * which resides on the blocked task's kernel stack: > + */ > +struct mutex_waiter { > + struct list_head list; > + struct task_struct *task; > + struct ww_acquire_ctx *ww_ctx; > +#ifdef CONFIG_DEBUG_MUTEXES > + void *magic; > +#endif > +};
i already did in v2 except this above waiter struct, will do it in v3.
Cheers,
Mukesh
> + > void > __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key) > { > @@ -65,6 +78,16 @@ EXPORT_SYMBOL(__mutex_init); > > #define MUTEX_FLAGS 0x07 > > +/* > + * Internal helper function; C doesn't allow us to hide it :/ > + * > + * DO NOT USE (outside of mutex code). > + */ > +static inline struct task_struct *__mutex_owner(struct mutex *lock) > +{ > + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); > +} > + > static inline struct task_struct *__owner_task(unsigned long owner) > { > return (struct task_struct *)(owner & ~MUTEX_FLAGS); > @@ -75,6 +98,22 @@ static inline unsigned long __owner_flags(unsigned long owner) > return owner & MUTEX_FLAGS; > } > > +bool mutex_is_locked(struct mutex *lock) > +{ > + return __mutex_owner(lock) != NULL; > +} > +EXPORT_SYMBOL(mutex_is_locked); > + > +__must_check enum mutex_trylock_recursive_enum > +mutex_trylock_recursive(struct mutex *lock) > +{ > + if (unlikely(__mutex_owner(lock) == current)) > + return MUTEX_TRYLOCK_RECURSIVE; > + > + return mutex_trylock(lock); > +} > +EXPORT_SYMBOL(mutex_trylock_recursive); > + > /* > * Trylock variant that retuns the owning task on failure. > */
| |