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 <vsement...@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <d...@openvz.org>
> ---
> 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.
Kevin
