Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1404753 > unrolled thread

[PATCH v2] locking/mutex: Set and clear owner using WRITE_ONCE()

Started byJason Low <jason.low2@hpe.com>
First post2016-05-21 00:30 +0200
Last post2016-05-21 01:10 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2] locking/mutex: Set and clear owner using WRITE_ONCE() Jason Low <jason.low2@hpe.com> - 2016-05-21 00:30 +0200
    Re: [PATCH v2] locking/mutex: Set and clear owner using WRITE_ONCE() Waiman Long <waiman.long@hpe.com> - 2016-05-21 01:10 +0200

#1404753 — [PATCH v2] locking/mutex: Set and clear owner using WRITE_ONCE()

FromJason Low <jason.low2@hpe.com>
Date2016-05-21 00:30 +0200
Subject[PATCH v2] locking/mutex: Set and clear owner using WRITE_ONCE()
Message-ID<rB15D-3sM-11@gated-at.bofh.it>
The mutex owner can get read and written to locklessly.
Use WRITE_ONCE when setting and clearing the owner field
in order to avoid optimizations such as store tearing. This
avoids situations where the owner field gets written to with
multiple stores and another thread could concurrently read
and use a partially written owner value.

Signed-off-by: Jason Low <jason.low2@hpe.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
---
 kernel/locking/mutex-debug.h |  4 ++--
 kernel/locking/mutex.h       | 10 ++++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/locking/mutex-debug.h b/kernel/locking/mutex-debug.h
index 0799fd3..372e653 100644
--- a/kernel/locking/mutex-debug.h
+++ b/kernel/locking/mutex-debug.h
@@ -29,12 +29,12 @@ extern void debug_mutex_init(struct mutex *lock, const char *name,
 
 static inline void mutex_set_owner(struct mutex *lock)
 {
-	lock->owner = current;
+	WRITE_ONCE(lock->owner, current);
 }
 
 static inline void mutex_clear_owner(struct mutex *lock)
 {
-	lock->owner = NULL;
+	WRITE_ONCE(lock->owner, NULL);
 }
 
 #define spin_lock_mutex(lock, flags)			\
diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h
index 5cda397..12f9619 100644
--- a/kernel/locking/mutex.h
+++ b/kernel/locking/mutex.h
@@ -17,14 +17,20 @@
 		__list_del((waiter)->list.prev, (waiter)->list.next)
 
 #ifdef CONFIG_MUTEX_SPIN_ON_OWNER
+/*
+ * The mutex owner can get read and written to locklessly.
+ * We should use WRITE_ONCE when writing the owner value to
+ * avoid store tearing, otherwise, a thread could potentially
+ * read a partially written and incomplete owner value.
+ */
 static inline void mutex_set_owner(struct mutex *lock)
 {
-	lock->owner = current;
+	WRITE_ONCE(lock->owner, current);
 }
 
 static inline void mutex_clear_owner(struct mutex *lock)
 {
-	lock->owner = NULL;
+	WRITE_ONCE(lock->owner, NULL);
 }
 #else
 static inline void mutex_set_owner(struct mutex *lock)
-- 
2.1.4

[toc] | [next] | [standalone]


#1404763

FromWaiman Long <waiman.long@hpe.com>
Date2016-05-21 01:10 +0200
Message-ID<rB1Im-3UX-7@gated-at.bofh.it>
In reply to#1404753
On 05/20/2016 06:19 PM, Jason Low wrote:
> The mutex owner can get read and written to locklessly.
> Use WRITE_ONCE when setting and clearing the owner field
> in order to avoid optimizations such as store tearing. This
> avoids situations where the owner field gets written to with
> multiple stores and another thread could concurrently read
> and use a partially written owner value.
>
> Signed-off-by: Jason Low<jason.low2@hpe.com>
> Acked-by: Davidlohr Bueso<dave@stgolabs.net>
> ---
>   kernel/locking/mutex-debug.h |  4 ++--
>   kernel/locking/mutex.h       | 10 ++++++++--
>   2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/locking/mutex-debug.h b/kernel/locking/mutex-debug.h
> index 0799fd3..372e653 100644
> --- a/kernel/locking/mutex-debug.h
> +++ b/kernel/locking/mutex-debug.h
> @@ -29,12 +29,12 @@ extern void debug_mutex_init(struct mutex *lock, const char *name,
>
>   static inline void mutex_set_owner(struct mutex *lock)
>   {
> -	lock->owner = current;
> +	WRITE_ONCE(lock->owner, current);
>   }
>
>   static inline void mutex_clear_owner(struct mutex *lock)
>   {
> -	lock->owner = NULL;
> +	WRITE_ONCE(lock->owner, NULL);
>   }
>
>   #define spin_lock_mutex(lock, flags)			\
> diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h
> index 5cda397..12f9619 100644
> --- a/kernel/locking/mutex.h
> +++ b/kernel/locking/mutex.h
> @@ -17,14 +17,20 @@
>   		__list_del((waiter)->list.prev, (waiter)->list.next)
>
>   #ifdef CONFIG_MUTEX_SPIN_ON_OWNER
> +/*
> + * The mutex owner can get read and written to locklessly.
> + * We should use WRITE_ONCE when writing the owner value to
> + * avoid store tearing, otherwise, a thread could potentially
> + * read a partially written and incomplete owner value.
> + */
>   static inline void mutex_set_owner(struct mutex *lock)
>   {
> -	lock->owner = current;
> +	WRITE_ONCE(lock->owner, current);
>   }
>
>   static inline void mutex_clear_owner(struct mutex *lock)
>   {
> -	lock->owner = NULL;
> +	WRITE_ONCE(lock->owner, NULL);
>   }
>   #else
>   static inline void mutex_set_owner(struct mutex *lock)
Acked-by: Waiman Long <Waiman.Long@hpe.com>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web