[1/2] sbitmap: use smp_mb__after_atomic() in sbq_wake_up()

Commit Message

Omar Sandoval Jan. 18, 2017, 7:55 p.m. UTC

From: Omar Sandoval <[e-mail]>

We always do an atomic clear_bit() right before we call sbq_wake_up(),
so we can use smp_mb__after_atomic(). While we're here, comment the
memory barriers in here a little more.

Signed-off-by: Omar Sandoval <[e-mail]>
---
 lib/sbitmap.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Jens Axboe Jan. 18, 2017, 8:42 p.m. UTC | #1

On 01/18/2017 11:55 AM, Omar Sandoval wrote:
> From: Omar Sandoval <[e-mail]>
> 
> We always do an atomic clear_bit() right before we call sbq_wake_up(),
> so we can use smp_mb__after_atomic(). While we're here, comment the
> memory barriers in here a little more.

Thanks Omar, applied 1-2.

Patch

diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index 2cecf05c82fd..df4e472df8a3 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -299,8 +299,14 @@  static void sbq_wake_up(struct sbitmap_queue *sbq)
 	struct sbq_wait_state *ws;
 	int wait_cnt;
 
-	/* Ensure that the wait list checks occur after clear_bit(). */
-	smp_mb();
+	/*
+	 * Pairs with the memory barrier in set_current_state() to ensure the
+	 * proper ordering of clear_bit()/waitqueue_active() in the waker and
+	 * test_and_set_bit()/prepare_to_wait()/finish_wait() in the waiter. See
+	 * the comment on waitqueue_active(). This is __after_atomic because we
+	 * just did clear_bit() in the caller.
+	 */
+	smp_mb__after_atomic();
 
 	ws = sbq_wake_ptr(sbq);
 	if (!ws)
@@ -331,7 +337,8 @@  void sbitmap_queue_wake_all(struct sbitmap_queue *sbq)
 	int i, wake_index;
 
 	/*
-	 * Make sure all changes prior to this are visible from other CPUs.
+	 * Pairs with the memory barrier in set_current_state() like in
+	 * sbq_wake_up().
 	 */
 	smp_mb();
 	wake_index = atomic_read(&sbq->wake_index);