* [PATCH 1/2] locking/mutex: Move mutex flag macros to linux/mutex.h @ 2019-07-29 10:52 Mukesh Ojha 2019-07-29 10:52 ` [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value Mukesh Ojha 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2019-07-29 10:52 UTC (permalink / raw) To: peterz, mingo, will, linux-kernel; +Cc: Mukesh Ojha There exist a place where we use hard code value for mutex flag(e.g in mutex.h __mutex_owner()). Let's move the mutex flag macros to header linux/mutex.h, so that it could be reused at other places as well. Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> --- include/linux/mutex.h | 15 +++++++++++++++ kernel/locking/mutex.c | 15 --------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/linux/mutex.h b/include/linux/mutex.h index dcd03fe..79b28be 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -20,6 +20,21 @@ #include <linux/osq_lock.h> #include <linux/debug_locks.h> +/* + * @owner: contains: 'struct task_struct *' to the current lock owner, + * NULL means not owned. Since task_struct pointers are aligned at + * at least L1_CACHE_BYTES, we have low bits to store extra state. + * + * Bit0 indicates a non-empty waiter list; unlock must issue a wakeup. + * Bit1 indicates unlock needs to hand the lock to the top-waiter + * Bit2 indicates handoff has been done and we're waiting for pickup. + */ +#define MUTEX_FLAG_WAITERS 0x01 +#define MUTEX_FLAG_HANDOFF 0x02 +#define MUTEX_FLAG_PICKUP 0x04 + +#define MUTEX_FLAGS 0x07 + struct ww_acquire_ctx; /* diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c index 5e06973..b74c87d 100644 --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -50,21 +50,6 @@ } EXPORT_SYMBOL(__mutex_init); -/* - * @owner: contains: 'struct task_struct *' to the current lock owner, - * NULL means not owned. Since task_struct pointers are aligned at - * at least L1_CACHE_BYTES, we have low bits to store extra state. - * - * Bit0 indicates a non-empty waiter list; unlock must issue a wakeup. - * Bit1 indicates unlock needs to hand the lock to the top-waiter - * Bit2 indicates handoff has been done and we're waiting for pickup. - */ -#define MUTEX_FLAG_WAITERS 0x01 -#define MUTEX_FLAG_HANDOFF 0x02 -#define MUTEX_FLAG_PICKUP 0x04 - -#define MUTEX_FLAGS 0x07 - static inline struct task_struct *__owner_task(unsigned long owner) { return (struct task_struct *)(owner & ~MUTEX_FLAGS); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-29 10:52 [PATCH 1/2] locking/mutex: Move mutex flag macros to linux/mutex.h Mukesh Ojha @ 2019-07-29 10:52 ` Mukesh Ojha 2019-07-29 11:07 ` Peter Zijlstra 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2019-07-29 10:52 UTC (permalink / raw) To: peterz, mingo, will, linux-kernel; +Cc: Mukesh Ojha Let's use the mutex flag macro(which got moved from mutex.c to linux/mutex.h in the last patch) instead of hard code value which was used in __mutex_owner(). Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> --- include/linux/mutex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/mutex.h b/include/linux/mutex.h index 79b28be..c3833ba 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -87,7 +87,7 @@ struct mutex { */ static inline struct task_struct *__mutex_owner(struct mutex *lock) { - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); } /* -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-29 10:52 ` [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value Mukesh Ojha @ 2019-07-29 11:07 ` Peter Zijlstra 2019-07-30 7:53 ` Mukesh Ojha 0 siblings, 1 reply; 10+ messages in thread From: Peter Zijlstra @ 2019-07-29 11:07 UTC (permalink / raw) To: Mukesh Ojha; +Cc: mingo, will, linux-kernel On Mon, Jul 29, 2019 at 04:22:58PM +0530, Mukesh Ojha wrote: > Let's use the mutex flag macro(which got moved from mutex.c > to linux/mutex.h in the last patch) instead of hard code > value which was used in __mutex_owner(). > > Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> > --- > include/linux/mutex.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/mutex.h b/include/linux/mutex.h > index 79b28be..c3833ba 100644 > --- a/include/linux/mutex.h > +++ b/include/linux/mutex.h > @@ -87,7 +87,7 @@ struct mutex { > */ > static inline struct task_struct *__mutex_owner(struct mutex *lock) > { > - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); > + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); > } I would _much_ rather move __mutex_owner() out of line, you're exposing far too much stuff. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-29 11:07 ` Peter Zijlstra @ 2019-07-30 7:53 ` Mukesh Ojha 2019-07-30 8:03 ` Peter Zijlstra 0 siblings, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2019-07-30 7:53 UTC (permalink / raw) To: Peter Zijlstra; +Cc: mingo, will, linux-kernel On 7/29/2019 4:37 PM, Peter Zijlstra wrote: > On Mon, Jul 29, 2019 at 04:22:58PM +0530, Mukesh Ojha wrote: >> Let's use the mutex flag macro(which got moved from mutex.c >> to linux/mutex.h in the last patch) instead of hard code >> value which was used in __mutex_owner(). >> >> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> >> --- >> include/linux/mutex.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/mutex.h b/include/linux/mutex.h >> index 79b28be..c3833ba 100644 >> --- a/include/linux/mutex.h >> +++ b/include/linux/mutex.h >> @@ -87,7 +87,7 @@ struct mutex { >> */ >> static inline struct task_struct *__mutex_owner(struct mutex *lock) >> { >> - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); >> + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); >> } > I would _much_ rather move __mutex_owner() out of line, you're exposing > far too much stuff. if i understand you correctly, you want me to move __mutex_owner() to mutex.c __mutex_owner() is used in mutex_is_locked() and mutex_trylock_recursive inside linux/mutex.h. Shall i move them as well ? Thanks, Mukesh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-30 7:53 ` Mukesh Ojha @ 2019-07-30 8:03 ` Peter Zijlstra 2019-07-30 12:40 ` Mukesh Ojha 2019-07-30 15:49 ` [PATCH 1/2 v2] locking/mutex: Make __mutex_owner static to mutex.c Mukesh Ojha 0 siblings, 2 replies; 10+ messages in thread From: Peter Zijlstra @ 2019-07-30 8:03 UTC (permalink / raw) To: Mukesh Ojha; +Cc: mingo, will, linux-kernel On Tue, Jul 30, 2019 at 01:23:13PM +0530, Mukesh Ojha wrote: > > On 7/29/2019 4:37 PM, Peter Zijlstra wrote: > > On Mon, Jul 29, 2019 at 04:22:58PM +0530, Mukesh Ojha wrote: > > > Let's use the mutex flag macro(which got moved from mutex.c > > > to linux/mutex.h in the last patch) instead of hard code > > > value which was used in __mutex_owner(). > > > > > > Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> > > > --- > > > include/linux/mutex.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/linux/mutex.h b/include/linux/mutex.h > > > index 79b28be..c3833ba 100644 > > > --- a/include/linux/mutex.h > > > +++ b/include/linux/mutex.h > > > @@ -87,7 +87,7 @@ struct mutex { > > > */ > > > static inline struct task_struct *__mutex_owner(struct mutex *lock) > > > { > > > - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); > > > + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); > > > } > > I would _much_ rather move __mutex_owner() out of line, you're exposing > > far too much stuff. > > if i understand you correctly, you want me to move __mutex_owner() to > mutex.c > __mutex_owner() is used in mutex_is_locked() and mutex_trylock_recursive > inside linux/mutex.h. > > Shall i move them as well ? Yes, then you can make __mutex_owner() static. Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-30 8:03 ` Peter Zijlstra @ 2019-07-30 12:40 ` Mukesh Ojha 2019-07-30 16:02 ` Peter Zijlstra 2019-07-30 15:49 ` [PATCH 1/2 v2] locking/mutex: Make __mutex_owner static to mutex.c Mukesh Ojha 1 sibling, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2019-07-30 12:40 UTC (permalink / raw) To: Peter Zijlstra; +Cc: mingo, will, linux-kernel On 7/30/2019 1:33 PM, Peter Zijlstra wrote: > On Tue, Jul 30, 2019 at 01:23:13PM +0530, Mukesh Ojha wrote: >> On 7/29/2019 4:37 PM, Peter Zijlstra wrote: >>> On Mon, Jul 29, 2019 at 04:22:58PM +0530, Mukesh Ojha wrote: >>>> Let's use the mutex flag macro(which got moved from mutex.c >>>> to linux/mutex.h in the last patch) instead of hard code >>>> value which was used in __mutex_owner(). >>>> >>>> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> >>>> --- >>>> include/linux/mutex.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/include/linux/mutex.h b/include/linux/mutex.h >>>> index 79b28be..c3833ba 100644 >>>> --- a/include/linux/mutex.h >>>> +++ b/include/linux/mutex.h >>>> @@ -87,7 +87,7 @@ struct mutex { >>>> */ >>>> static inline struct task_struct *__mutex_owner(struct mutex *lock) >>>> { >>>> - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); >>>> + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); >>>> } >>> I would _much_ rather move __mutex_owner() out of line, you're exposing >>> far too much stuff. >> if i understand you correctly, you want me to move __mutex_owner() to >> mutex.c >> __mutex_owner() is used in mutex_is_locked() and mutex_trylock_recursive >> inside linux/mutex.h. >> >> Shall i move them as well ? > Yes, then you can make __mutex_owner() static. To make it static , i have to export mutex_is_locked() after moving it inside mutex.c, so that other module can use it. 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 . Thanks, Mukesh > > Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-30 12:40 ` Mukesh Ojha @ 2019-07-30 16:02 ` Peter Zijlstra 2019-07-30 16:25 ` Mukesh Ojha 0 siblings, 1 reply; 10+ messages in thread From: Peter Zijlstra @ 2019-07-30 16:02 UTC (permalink / raw) To: Mukesh Ojha; +Cc: mingo, will, linux-kernel 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 +}; + 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. */ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-30 16:02 ` Peter Zijlstra @ 2019-07-30 16:25 ` Mukesh Ojha 0 siblings, 0 replies; 10+ messages in thread From: Mukesh Ojha @ 2019-07-30 16:25 UTC (permalink / raw) To: Peter Zijlstra; +Cc: mingo, will, linux-kernel 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. > */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2 v2] locking/mutex: Make __mutex_owner static to mutex.c 2019-07-30 8:03 ` Peter Zijlstra 2019-07-30 12:40 ` Mukesh Ojha @ 2019-07-30 15:49 ` Mukesh Ojha 2019-07-30 15:49 ` [PATCH 2/2 v2] locking/mutex: Use mutex flags macro instead of hard code value Mukesh Ojha 1 sibling, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2019-07-30 15:49 UTC (permalink / raw) To: peterz, mingo, will, linux-kernel; +Cc: Mukesh Ojha __mutex_owner() should only be used by the mutex api's. So, put this restiction let's move the __mutex_owner() function definition from linux/mutex.h to mutex.c file. There exist functions that uses __mutex_owner() like mutex_is_locked() and mutex_trylock_recursive(), So to keep the thing intact move them as well and export them. Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> --- Changes in v2: - On Peterz suggestion, moved __mutex_owner() to mutex.c to make it static to mutex.c. - Exported mutex_is_owner() and mutex_trylock_recursive() to keep the existing thing intact as there are loadable modules which uses these functions. include/linux/mutex.h | 44 +++----------------------------------------- kernel/locking/mutex.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/include/linux/mutex.h b/include/linux/mutex.h index dcd03fe..841b47d 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -66,16 +66,6 @@ struct mutex { }; /* - * 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: */ @@ -138,17 +128,10 @@ static inline void mutex_destroy(struct mutex *lock) {} extern void __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key); -/** - * mutex_is_locked - is the mutex locked - * @lock: the mutex to be queried - * - * 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 inline bool mutex_is_locked(struct mutex *lock); +extern inline /* __deprecated */ __must_check enum mutex_trylock_recursive_enum +mutex_trylock_recursive(struct mutex *lock); /* * See kernel/locking/mutex.c for detailed documentation of these APIs. * Also see Documentation/locking/mutex-design.rst. @@ -208,25 +191,4 @@ enum mutex_trylock_recursive_enum { MUTEX_TRYLOCK_RECURSIVE, }; -/** - * mutex_trylock_recursive - trylock variant that allows recursive locking - * @lock: mutex to be locked - * - * This function should not be used, _ever_. It is purely for hysterical GEM - * raisins, and once those are gone this will be removed. - * - * Returns: - * - MUTEX_TRYLOCK_FAILED - trylock failed, - * - 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); -} - #endif /* __LINUX_MUTEX_H */ diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c index 5e06973..f73250a 100644 --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -65,6 +65,50 @@ #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) & ~0x07); +} + +/** + * mutex_is_locked - is the mutex locked + * @lock: the mutex to be queried + * + * Returns true if the mutex is locked, false if unlocked. + */ +inline bool mutex_is_locked(struct mutex *lock) +{ + return __mutex_owner(lock) != NULL; +} +EXPORT_SYMBOL(mutex_is_locked); + +/** + * mutex_trylock_recursive - trylock variant that allows recursive locking + * @lock: mutex to be locked + * + * This function should not be used, _ever_. It is purely for hysterical GEM + * raisins, and once those are gone this will be removed. + * + * Returns: + * - MUTEX_TRYLOCK_FAILED - trylock failed, + * - MUTEX_TRYLOCK_SUCCESS - lock acquired, + * - MUTEX_TRYLOCK_RECURSIVE - we already owned the lock. + */ +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); +} +EXPORT_SYMBOL(mutex_trylock_recursive); + static inline struct task_struct *__owner_task(unsigned long owner) { return (struct task_struct *)(owner & ~MUTEX_FLAGS); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2 v2] locking/mutex: Use mutex flags macro instead of hard code value 2019-07-30 15:49 ` [PATCH 1/2 v2] locking/mutex: Make __mutex_owner static to mutex.c Mukesh Ojha @ 2019-07-30 15:49 ` Mukesh Ojha 0 siblings, 0 replies; 10+ messages in thread From: Mukesh Ojha @ 2019-07-30 15:49 UTC (permalink / raw) To: peterz, mingo, will, linux-kernel; +Cc: Mukesh Ojha Use the mutex flag macro instead of hard code value inside __mutex_owner(). Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> --- Changes in v2: - Framed the commit according the changes done in 1/2 of the patchset. kernel/locking/mutex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c index f73250a..1c8f86a 100644 --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -72,7 +72,7 @@ */ static inline struct task_struct *__mutex_owner(struct mutex *lock) { - return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x07); + return (struct task_struct *)(atomic_long_read(&lock->owner) & ~MUTEX_FLAGS); } /** -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-30 16:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-29 10:52 [PATCH 1/2] locking/mutex: Move mutex flag macros to linux/mutex.h Mukesh Ojha 2019-07-29 10:52 ` [PATCH 2/2] locking/mutex: Use mutex flags macro instead of hard code value Mukesh Ojha 2019-07-29 11:07 ` Peter Zijlstra 2019-07-30 7:53 ` Mukesh Ojha 2019-07-30 8:03 ` Peter Zijlstra 2019-07-30 12:40 ` Mukesh Ojha 2019-07-30 16:02 ` Peter Zijlstra 2019-07-30 16:25 ` Mukesh Ojha 2019-07-30 15:49 ` [PATCH 1/2 v2] locking/mutex: Make __mutex_owner static to mutex.c Mukesh Ojha 2019-07-30 15:49 ` [PATCH 2/2 v2] locking/mutex: Use mutex flags macro instead of hard code value Mukesh Ojha
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).