set_mb: Use smp_store_release() instead of set_mb()

set_mb() and smp_store_release() perform the same function. Also there are only a few users of set_mb(). We can convert these users to use smp_store_release() and delete the set_mb() definition.

The following patch changes the users and if this is OK I will go ahead and delete the set_mb() definition. Comments and suggestions welcome.

Thanks!
Pranith

Signed-off-by Pranith Kumar <[email protected]>

---
 fs/select.c           |  6 +++---
 include/linux/sched.h | 14 +++++++-------
 kernel/futex.c        |  4 ++--
 kernel/sched/wait.c   |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

fs/select.c +3 -3

index 467bb1c..959a908 100644
@@ -189,7 +189,7 @@ static int __pollwake(wait_queue_t *wait, unsigned mode, int sync, void *key)
 	 * doesn't imply write barrier and the users expect write
 	 * barrier semantics on wakeup functions.  The following
 	 * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up()
-	 * and is paired with set_mb() in poll_schedule_timeout.
+	 * and is paired with smp_store_release() in poll_schedule_timeout.
 	 */
 	smp_wmb();
 	pwq->triggered = 1;

@@ -244,7 +244,7 @@ int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
 	/*
 	 * Prepare for the next iteration.
 	 *
-	 * The following set_mb() serves two purposes.  First, it's
+	 * The following smp_store_release() serves two purposes.  First, it's
 	 * the counterpart rmb of the wmb in pollwake() such that data
 	 * written before wake up is always visible after wake up.
 	 * Second, the full barrier guarantees that triggered clearing

@@ -252,7 +252,7 @@ int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
 	 * this problem doesn't exist for the first iteration as
 	 * add_wait_queue() has full barrier semantics.
 	 */
-	set_mb(pwq->triggered, 0);
+	smp_store_release(pwq->triggered, 0);
 
 	return rc;
 }

include/linux/sched.h +7 -7

index 8db31ef..4621d0b 100644
@@ -253,7 +253,7 @@ extern char ___assert_task_state[1 - 2*!!(
 #define set_task_state(tsk, state_value)			\
 	do {							\
 		(tsk)->task_state_change = _THIS_IP_;		\
-		set_mb((tsk)->state, (state_value));		\
+		smp_store_release((tsk)->state, (state_value));	\
 	} while (0)
 
 /*

@@ -272,10 +272,10 @@ extern char ___assert_task_state[1 - 2*!!(
 		current->task_state_change = _THIS_IP_;		\
 		current->state = (state_value);			\
 	} while (0)
-#define set_current_state(state_value)				\
-	do {							\
-		current->task_state_change = _THIS_IP_;		\
-		set_mb(current->state, (state_value));		\
+#define set_current_state(state_value)					\
+	do {								\
+		current->task_state_change = _THIS_IP_;			\
+		smp_store_release(current->state, (state_value));	\
 	} while (0)
 
 #else

@@ -283,7 +283,7 @@ extern char ___assert_task_state[1 - 2*!!(
 #define __set_task_state(tsk, state_value)		\
 	do { (tsk)->state = (state_value); } while (0)
 #define set_task_state(tsk, state_value)		\
-	set_mb((tsk)->state, (state_value))
+	smp_store_release((tsk)->state, (state_value))
 
 /*
  * set_current_state() includes a barrier so that the write of current->state

@@ -299,7 +299,7 @@ extern char ___assert_task_state[1 - 2*!!(
 #define __set_current_state(state_value)		\
 	do { current->state = (state_value); } while (0)
 #define set_current_state(state_value)			\
-	set_mb(current->state, (state_value))
+	smp_store_release(current->state, (state_value))
 
 #endif
 

kernel/futex.c +2 -2

index 63678b5..0604355 100644
@@ -2055,8 +2055,8 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
 {
 	/*
 	 * The task state is guaranteed to be set before another task can
-	 * wake it. set_current_state() is implemented using set_mb() and
-	 * queue_me() calls spin_unlock() upon completion, both serializing
+	 * wake it. set_current_state() is implemented using smp_store_release()
+	 * and queue_me() calls spin_unlock() upon completion, both serializing
 	 * access to the hash list and forcing another memory barrier.
 	 */
 	set_current_state(TASK_INTERRUPTIBLE);

kernel/sched/wait.c +2 -2

index 852143a..7d990c0 100644
@@ -341,7 +341,7 @@ long wait_woken(wait_queue_t *wait, unsigned mode, long timeout)
 	 * condition being true _OR_ WQ_FLAG_WOKEN such that we will not miss
 	 * an event.
 	 */
-	set_mb(wait->flags, wait->flags & ~WQ_FLAG_WOKEN); /* B */
+	smp_store_release(wait->flags, wait->flags & ~WQ_FLAG_WOKEN); /* B */
 
 	return timeout;
 }

@@ -354,7 +354,7 @@ int woken_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key)
 	 * doesn't imply write barrier and the users expects write
 	 * barrier semantics on wakeup functions.  The following
 	 * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up()
-	 * and is paired with set_mb() in wait_woken().
+	 * and is paired with smp_store_release() in wait_woken().
 	 */
 	smp_wmb(); /* C */
 	wait->flags |= WQ_FLAG_WOKEN;

Signature · 1 line

1.9.1

set_mb() and smp_store_release() perform the same function. Also there are only a few users of set_mb(). We can convert these users to use smp_store_release() and delete the set_mb() definition.

The following patch changes the users and if this is OK I will go ahead and delete the set_mb() definition. Comments and suggestions welcome.

Thanks!
Pranith

Signed-off-by Pranith Kumar <[email protected]>

---
 fs/select.c           |  6 +++---
 include/linux/sched.h | 14 +++++++-------
 kernel/futex.c        |  4 ++--
 kernel/sched/wait.c   |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

fs/select.c +3 -3

index 467bb1c..959a908 100644
@@ -189,7 +189,7 @@ static int __pollwake(wait_queue_t *wait, unsigned mode, int sync, void *key)
         * doesn't imply write barrier and the users expect write
         * barrier semantics on wakeup functions.  The following
         * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up()
-        * and is paired with set_mb() in poll_schedule_timeout.
+        * and is paired with smp_store_release() in poll_schedule_timeout.
         */
        smp_wmb();
        pwq->triggered = 1;

@@ -244,7 +244,7 @@ int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
        /*
         * Prepare for the next iteration.
         *
-        * The following set_mb() serves two purposes.  First, it's
+        * The following smp_store_release() serves two purposes.  First, it's
         * the counterpart rmb of the wmb in pollwake() such that data
         * written before wake up is always visible after wake up.
         * Second, the full barrier guarantees that triggered clearing

@@ -252,7 +252,7 @@ int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
         * this problem doesn't exist for the first iteration as
         * add_wait_queue() has full barrier semantics.
         */
-       set_mb(pwq->triggered, 0);
+       smp_store_release(pwq->triggered, 0);
 
        return rc;
 }

include/linux/sched.h +7 -7

index 8db31ef..4621d0b 100644
@@ -253,7 +253,7 @@ extern char ___assert_task_state[1 - 2*!!(
 #define set_task_state(tsk, state_value)                       \
        do {                                                    \
                (tsk)->task_state_change = _THIS_IP_;           \
-               set_mb((tsk)->state, (state_value));            \
+               smp_store_release((tsk)->state, (state_value)); \
        } while (0)
 
 /*

@@ -272,10 +272,10 @@ extern char ___assert_task_state[1 - 2*!!(
                current->task_state_change = _THIS_IP_;         \
                current->state = (state_value);                 \
        } while (0)
-#define set_current_state(state_value)                         \
-       do {                                                    \
-               current->task_state_change = _THIS_IP_;         \
-               set_mb(current->state, (state_value));          \
+#define set_current_state(state_value)                                 \
+       do {                                                            \
+               current->task_state_change = _THIS_IP_;                 \
+               smp_store_release(current->state, (state_value));       \
        } while (0)
 
 #else

@@ -283,7 +283,7 @@ extern char ___assert_task_state[1 - 2*!!(
 #define __set_task_state(tsk, state_value)             \
        do { (tsk)->state = (state_value); } while (0)
 #define set_task_state(tsk, state_value)               \
-       set_mb((tsk)->state, (state_value))
+       smp_store_release((tsk)->state, (state_value))
 
 /*
  * set_current_state() includes a barrier so that the write of current->state

@@ -299,7 +299,7 @@ extern char ___assert_task_state[1 - 2*!!(
 #define __set_current_state(state_value)               \
        do { current->state = (state_value); } while (0)
 #define set_current_state(state_value)                 \
-       set_mb(current->state, (state_value))
+       smp_store_release(current->state, (state_value))
 
 #endif
 

kernel/futex.c +2 -2

index 63678b5..0604355 100644
@@ -2055,8 +2055,8 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
 {
        /*
         * The task state is guaranteed to be set before another task can
-        * wake it. set_current_state() is implemented using set_mb() and
-        * queue_me() calls spin_unlock() upon completion, both serializing
+        * wake it. set_current_state() is implemented using smp_store_release()
+        * and queue_me() calls spin_unlock() upon completion, both serializing
         * access to the hash list and forcing another memory barrier.
         */
        set_current_state(TASK_INTERRUPTIBLE);

kernel/sched/wait.c +2 -3

index 852143a..7d990c0 100644
@@ -341,7 +341,7 @@ long wait_woken(wait_queue_t *wait, unsigned mode, long timeout)
         * condition being true _OR_ WQ_FLAG_WOKEN such that we will not miss
         * an event.
         */
-       set_mb(wait->flags, wait->flags & ~WQ_FLAG_WOKEN); /* B */
+       smp_store_release(wait->flags, wait->flags & ~WQ_FLAG_WOKEN); /* B */
 
        return timeout;
 }

@@ -354,7 +354,7 @@ int woken_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key)
         * doesn't imply write barrier and the users expects write
         * barrier semantics on wakeup functions.  The following
         * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up()
-        * and is paired with set_mb() in wait_woken().
+        * and is paired with smp_store_release() in wait_woken().
         */
        smp_wmb(); /* C */
        wait->flags |= WQ_FLAG_WOKEN;
--

1.9.1