On 06/15/2016 02:41 AM, Denis V. Lunev wrote: > On 06/15/2016 05:36 AM, Eric Blake wrote: >> On 06/14/2016 09:25 AM, Denis V. Lunev wrote: >>> There is no need to scan allocation tables if we have mark_all_dirty >>> flag >>> set. Just mark it all dirty. >>>
>>> int ret, n; >>> end = s->bdev_length / BDRV_SECTOR_SIZE; >>> + if (base == NULL && !bdrv_has_zero_init(target_bs)) { >>> + bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end); >> Won't work as written. 'end' is 64 bits, but bdrv_set_dirty_bitmap() is >> limited to a 32-bit sector count. Might be first worth updating >> bdrv_set_dirty_bitmap() and friends to be byte-based rather than >> sector-based (but still tracking a sector per bit, obviously), as well >> as expand it to operate on 64-bit sizes rather than 32-bit. > very nice catch! thank you > [meta-comment - your reply style is a bit hard to read. It's best to include a blank line both before and after any text you write when replying in context, as the extra spacing calls visual attention to your reply rather than being part of a wall of text] >> I'm also worried slightly that the existing code repeated things in a >> loop, and therefore had pause points every iteration and could thus >> remain responsive to an early cancel. But doing the entire operation in >> one chunk (assuming you fix bitmap code to handle a 64-bit size) may end >> up running for so long without interruption that you lose the benefits >> of an early interruption that you have by virtue of a 32-bit limit. >> > I do not think that this should be worried actually. We just perform memset > inside for not that big area (1 Tb disk will have 2 Mb dirty area > bitmap) under > default parameters. > Okay, so the real slowdown in the loop was the rest of the code (checking backing status in bdrv_is_allocated_above() and not in actually writing to the bitmap (bdrv_set_dirty_bitmap()). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature