Commit 4ace53f1 authored by Omar Sandoval's avatar Omar Sandoval Committed by Jens Axboe

sbitmap: use test_and_set_bit_lock()/clear_bit_unlock()

sbitmap_queue_get()/sbitmap_queue_clear() are used for
allocating/freeing a resource, so they should provide acquire/release
barrier semantics, respectively. sbitmap_get() currently contains a full
barrier, which is unnecessary, so use test_and_set_bit_lock() instead of
test_and_set_bit() (these are equivalent on x86_64). sbitmap_clear_bit()
does not imply any barriers, which is incorrect, as accesses of the
resource (e.g., request) could potentially get reordered to after the
clear_bit(). Introduce sbitmap_clear_bit_unlock() and use it for
sbitmap_queue_clear() (this only adds a compiler barrier on x86_64). The
other existing user of sbitmap_clear_bit() (the blk-mq software queue
pending map) is serialized through a spinlock and does not need this.
Reported-by: default avatarTejun Heo <tj@kernel.org>
Acked-by: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent e9a99a63
...@@ -171,6 +171,8 @@ void sbitmap_resize(struct sbitmap *sb, unsigned int depth); ...@@ -171,6 +171,8 @@ void sbitmap_resize(struct sbitmap *sb, unsigned int depth);
* starting from the last allocated bit. This is less efficient * starting from the last allocated bit. This is less efficient
* than the default behavior (false). * than the default behavior (false).
* *
* This operation provides acquire barrier semantics if it succeeds.
*
* Return: Non-negative allocated bit number if successful, -1 otherwise. * Return: Non-negative allocated bit number if successful, -1 otherwise.
*/ */
int sbitmap_get(struct sbitmap *sb, unsigned int alloc_hint, bool round_robin); int sbitmap_get(struct sbitmap *sb, unsigned int alloc_hint, bool round_robin);
...@@ -300,6 +302,12 @@ static inline void sbitmap_clear_bit(struct sbitmap *sb, unsigned int bitnr) ...@@ -300,6 +302,12 @@ static inline void sbitmap_clear_bit(struct sbitmap *sb, unsigned int bitnr)
clear_bit(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr)); clear_bit(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr));
} }
static inline void sbitmap_clear_bit_unlock(struct sbitmap *sb,
unsigned int bitnr)
{
clear_bit_unlock(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr));
}
static inline int sbitmap_test_bit(struct sbitmap *sb, unsigned int bitnr) static inline int sbitmap_test_bit(struct sbitmap *sb, unsigned int bitnr)
{ {
return test_bit(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr)); return test_bit(SB_NR_TO_BIT(sb, bitnr), __sbitmap_word(sb, bitnr));
......
...@@ -100,7 +100,7 @@ static int __sbitmap_get_word(unsigned long *word, unsigned long depth, ...@@ -100,7 +100,7 @@ static int __sbitmap_get_word(unsigned long *word, unsigned long depth,
return -1; return -1;
} }
if (!test_and_set_bit(nr, word)) if (!test_and_set_bit_lock(nr, word))
break; break;
hint = nr + 1; hint = nr + 1;
...@@ -434,9 +434,9 @@ static void sbq_wake_up(struct sbitmap_queue *sbq) ...@@ -434,9 +434,9 @@ static void sbq_wake_up(struct sbitmap_queue *sbq)
/* /*
* Pairs with the memory barrier in set_current_state() to ensure the * Pairs with the memory barrier in set_current_state() to ensure the
* proper ordering of clear_bit()/waitqueue_active() in the waker and * proper ordering of clear_bit()/waitqueue_active() in the waker and
* test_and_set_bit()/prepare_to_wait()/finish_wait() in the waiter. See * test_and_set_bit_lock()/prepare_to_wait()/finish_wait() in the
* the comment on waitqueue_active(). This is __after_atomic because we * waiter. See the comment on waitqueue_active(). This is __after_atomic
* just did clear_bit() in the caller. * because we just did clear_bit_unlock() in the caller.
*/ */
smp_mb__after_atomic(); smp_mb__after_atomic();
...@@ -469,7 +469,7 @@ static void sbq_wake_up(struct sbitmap_queue *sbq) ...@@ -469,7 +469,7 @@ static void sbq_wake_up(struct sbitmap_queue *sbq)
void sbitmap_queue_clear(struct sbitmap_queue *sbq, unsigned int nr, void sbitmap_queue_clear(struct sbitmap_queue *sbq, unsigned int nr,
unsigned int cpu) unsigned int cpu)
{ {
sbitmap_clear_bit(&sbq->sb, nr); sbitmap_clear_bit_unlock(&sbq->sb, nr);
sbq_wake_up(sbq); sbq_wake_up(sbq);
if (likely(!sbq->round_robin && nr < sbq->sb.depth)) if (likely(!sbq->round_robin && nr < sbq->sb.depth))
*per_cpu_ptr(sbq->alloc_hint, cpu) = nr; *per_cpu_ptr(sbq->alloc_hint, cpu) = nr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment