[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 01/22] hbitmap: improve dirty iter
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 01/22] hbitmap: improve dirty iter |
Date: |
Sat, 1 Oct 2016 15:52:12 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
> Make dirty iter resistant to resetting bits in corresponding HBitmap.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> include/qemu/hbitmap.h | 24 ++----------------------
> util/hbitmap.c | 23 ++++++++++++++++++++++-
> 2 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index eb46475..9aa86c1 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -243,10 +243,7 @@ void hbitmap_free(HBitmap *hb);
> * the lowest-numbered bit that is set in @hb, starting at @first.
> *
> * 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.
> + * iteration to miss some of those bits. Concurrent bits resetting is ok too.
I'd prefer:
Concurrent resetting of bits is OK, too.
Or:
Concurrently resetting bits is OK, too.
Or:
Resetting bits concurrently is OK, too.
With or without that change:
Reviewed-by: Max Reitz <address@hidden>
> */
> void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
>
> @@ -285,24 +282,7 @@ void hbitmap_free_meta(HBitmap *hb);
> * Return the next bit that is set in @hbi's associated HBitmap,
> * or -1 if all remaining bits are zero.
> */
> -static inline int64_t hbitmap_iter_next(HBitmapIter *hbi)
> -{
> - unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1];
> - int64_t item;
> -
> - if (cur == 0) {
> - cur = hbitmap_iter_skip_words(hbi);
> - if (cur == 0) {
> - return -1;
> - }
> - }
> -
> - /* The next call will resume work from the next bit. */
> - hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
> - item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
> -
> - return item << hbi->granularity;
> -}
> +int64_t hbitmap_iter_next(HBitmapIter *hbi);
>
> /**
> * hbitmap_iter_next_word:
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 6a13c12..4f5cf96 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);
>
> /* Check for end of iteration. We always use fewer than BITS_PER_LONG
> @@ -139,6 +140,26 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
> return cur;
> }
>
> +int64_t hbitmap_iter_next(HBitmapIter *hbi)
> +{
> + unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1] &
> + hbi->hb->levels[HBITMAP_LEVELS - 1][hbi->pos];
> + int64_t item;
> +
> + if (cur == 0) {
> + cur = hbitmap_iter_skip_words(hbi);
> + if (cur == 0) {
> + return -1;
> + }
> + }
> +
> + /* The next call will resume work from the next bit. */
> + hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
> + item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
> +
> + return item << hbi->granularity;
> +}
> +
> void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
> {
> unsigned i, bit;
>
signature.asc
Description: OpenPGP digital signature
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-block] [PATCH 01/22] hbitmap: improve dirty iter,
Max Reitz <=