[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 09/12] dirty-bitmap: Change bdrv_[re]set_dirt
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v2 09/12] dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes |
Date: |
Wed, 17 May 2017 08:40:05 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 05/17/2017 06:59 AM, Juan Quintela wrote:
> Eric Blake <address@hidden> wrote:
>> Some of the callers were already scaling bytes to sectors; others
>> can be easily converted to pass byte offsets, all in our shift
>> towards a consistent byte interface everywhere. Making the change
>> will also make it easier to write the hold-out callers to use byte
>> rather than sectors for their iterations; it also makes it easier
>> for a future dirty-bitmap patch to offload scaling over to the
>> internal hbitmap. Although all callers happen to pass
>> sector-aligned values, make the internal scaling robust to any
>> sub-sector requests.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> Reviewed-by: John Snow <address@hidden>
>
> Reviewed-by: Juan Quintela <address@hidden>
>
>
>> +
>> assert(bdrv_dirty_bitmap_enabled(bitmap));
>> - hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
>> + hbitmap_set(bitmap->bitmap, offset >> BDRV_SECTOR_BITS,
>> + end_sector - (offset >> BDRV_SECTOR_BITS));
>
> It is just me, or we use:
>
> bytes = sectors * BDRV_SECTOR_SIZE
> or
> bytes = sectors << BDRV_SECTOR_BITS
BDRV_SECTOR_SIZE is ULL, while BDRV_SECTOR_BITS is not. Multiplying by
SIZE guarantees a 64-bit answer, but shifting by bits will give only a
32-bit answer if the left hand side of << is not already 64 bits (I've
had bugs in earlier conversion patches where I used << but should have
used *). So for scaling up, my recent round of patches have been
favoring * (ideally, since BDRV_SECTOR_SIZE is a power-of-2 constant,
the compiler optimizes it into a shift anyways instead of wasting time
on a multiply).
For scaling down, '/ SIZE' and '>> BITS' are equivalent, but I prefer
the >> form.
>
> and the same from bytes to sectors (no, this patch is consistent,
> but just looking at the file ....)
The real end-goal is to update the file to track the entire process with
bytes everywhere (and skip the intermediate representation in sectors),
which will reduce the number of scaling operations needed in the first
place. migration/block.c isn't quite there yet, but we'll get there.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v2 05/12] dirty-bitmap: Set iterator start by offset, not sector, (continued)
- [Qemu-block] [PATCH v2 05/12] dirty-bitmap: Set iterator start by offset, not sector, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 04/12] dirty-bitmap: Track size in bytes, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 07/12] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 06/12] dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 08/12] dirty-bitmap: Change bdrv_get_dirty() to take bytes, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 11/12] dirty-bitmap: Switch bdrv_set_dirty() to bytes, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 09/12] dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 10/12] mirror: Switch mirror_dirty_init() to byte-based iteration, Eric Blake, 2017/05/16
- [Qemu-block] [PATCH v2 12/12] dirty-bitmap: Convert internal hbitmap size/granularity, Eric Blake, 2017/05/16