Am 08.08.2016 um 17:04 hat Vladimir Sementsov-Ogievskiy geschrieben:
If dirty bitmap was cleared during iterator life, we can went to zero
current in hbitmap_iter_skip_words, starting from saved (and currently
wrong hbi->cur[...]).
I was going to suggest improved grammar, but actually I find this
description hard to understand even with correct grammar.
If I understand correctly, hbi->cur contains a copy of hb->levels that
isn't updated after the iterator entered the subtree, because this way
we avoid going backwards if concurrent operations set a new bit.
However, in the opposite case of clearing a bit, we actually want to use
the new value because we wouldn't find anything to return when going to
the lower levels. This is what the & does in your patch.
Is this explanation reasonably correct? If so, maybe you can try to make
these points in the commit message in the next version.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Signed-off-by: Denis V. Lunev <address@hidden>
---
util/hbitmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 6a13c12..f807f64 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -106,8 +106,9 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
unsigned long cur;
do {
- cur = hbi->cur[--i];
+ i--;
pos >>= BITS_PER_LEVEL;
+ cur = hbi->cur[i] & hb->levels[i][pos];
} while (cur == 0);
I think you will want to update the comment for hbitmap_iter_init. This
is what it says today:
* Concurrent setting of bits is acceptable, and will at worst cause the
* iteration to miss some of those bits. Resetting bits before the current
* position of the iterator is also okay. However, concurrent resetting of
* bits can lead to unexpected behavior if the iterator has not yet reached
* those bits.