[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 04/10] block: Support meta dirty bitmap
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v5 04/10] block: Support meta dirty bitmap |
Date: |
Thu, 14 Jul 2016 16:00:26 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 06/22/2016 11:53 AM, Max Reitz wrote:
> On 03.06.2016 06:32, Fam Zheng wrote:
>> The added group of operations enables tracking of the changed bits in
>> the dirty bitmap.
>>
>> Signed-off-by: Fam Zheng <address@hidden>
>> ---
>> block/dirty-bitmap.c | 52
>> ++++++++++++++++++++++++++++++++++++++++++++
>> include/block/dirty-bitmap.h | 9 ++++++++
>> 2 files changed, 61 insertions(+)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 628b77c..9c53c56 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -38,6 +38,7 @@
>> */
>> struct BdrvDirtyBitmap {
>> HBitmap *bitmap; /* Dirty sector bitmap implementation */
>> + HBitmap *meta; /* Meta dirty bitmap */
>> BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
>> char *name; /* Optional non-empty unique ID */
>> int64_t size; /* Size of the bitmap (Number of sectors) */
>> @@ -103,6 +104,56 @@ BdrvDirtyBitmap
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>> return bitmap;
>> }
>>
>> +/* bdrv_create_meta_dirty_bitmap
>> + *
>> + * Create a meta dirty bitmap that tracks the changes of bits in @bitmap.
>> I.e.
>> + * when a dirty status bit in @bitmap is changed (either from reset to set
>> or
>> + * the other way around), its respective meta dirty bitmap bit will be
>> marked
>> + * dirty as well.
>
> Not wrong, but I'd like a note here that this is not an
> when-and-only-when relationship, i.e. that bits in the meta bitmap may
> be set even without the tracked bits in the dirty bitmap having changed.
>
How?
You mean, if the caller manually starts setting things in the meta
bitmap object?
That's their fault then, isn't it?
> Maybe this should be mentioned somewhere in patch 2, too. Or maybe only
> in patch 2.
>
>> + *
>> + * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
>> + * @chunk_size: how many bytes of bitmap data does each bit in the meta
>> bitmap
>> + * track.
>> + */
>> +void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>> + int chunk_size)
>> +{
>> + assert(!bitmap->meta);
>> + bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
>> + chunk_size * BITS_PER_BYTE);
>> +}
>> +
>> +void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>> +{
>> + assert(bitmap->meta);
>> + hbitmap_free_meta(bitmap->bitmap);
>> + bitmap->meta = NULL;
>> +}
>> +
>> +int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
>> + BdrvDirtyBitmap *bitmap, int64_t sector,
>> + int nb_sectors)
>> +{
>> + uint64_t i;
>> + int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
>> +
>> + /* To optimize: we can make hbitmap to internally check the range in a
>> + * coarse level, or at least do it word by word. */
>
> We could also multiply gran by the granularity of the meta bitmap. Each
> bit of the meta bitmap tracks at least eight bits of the dirty bitmap,
> so we're calling hbitmap_get() at least eight times as often as
> necessary here.
>
> Or we just use int gran = hbitmap_granularity(bitmap->meta);.
>
> Not exactly wrong, though, so:
>
> Reviewed-by: Max Reitz <address@hidden>
>
>> + for (i = sector; i < sector + nb_sectors; i += gran) {
>> + if (hbitmap_get(bitmap->meta, i)) {
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>
- Re: [Qemu-devel] [PATCH v5 04/10] block: Support meta dirty bitmap,
John Snow <=