On 10/13/21 6:54 PM, Jens Axboe wrote:
If we come across bits that are already set, then it's quicker to test
that first and gate the test_and_set_bit() operation on the result of
the bit test.
Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
---
lib/sbitmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index c6e2f1f2c4d2..11b244a8d00f 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -166,7 +166,7 @@ static int __sbitmap_get_word(unsigned long *word, unsigned long depth,
return -1;
}
- if (!test_and_set_bit_lock(nr, word))
+ if (!test_bit(nr, word) && !test_and_set_bit_lock(nr, word))
break;
hint = nr + 1;
Hah. Finally something to latch on.
I've seen this coding pattern quite a lot in the block layer, and, of
course, mostly in the hot path.
(Kinda the point, I guess :-)
However, my question is this:
While 'test_and_set_bit()' is atomic, the combination of
'test_bit && !test_and_set_bit()' is not.
IE this change moves an atomic operation into a non-atomic one.
So how can we be sure that this patch doesn't introduce a race condition?
And if it doesn't, should we add some comment above the code why this is
safe to do here?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
