All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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

* 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.