Message ID | 1473424308-19812-3-git-send-email-den@openvz.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 09/09/2016 07:31 AM, Denis V. Lunev wrote: > bdrv_is_allocated_above() returns true in the case even for completel s/completel/completely/ > zeroed areas as BDRV_BLOCK_ALLOCATED flag is set in both cases. > > The patch stops using bdrv_is_allocated_above() wrapper and switches to > bdrv_get_block_status_above() to distinguish zeroed areas and areas with > data to avoid extra IO operations if possible. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> > CC: Fam Zheng <famz@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Max Reitz <mreitz@redhat.com> > CC: Jeff Cody <jcody@redhat.com> > --- > block/mirror.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > @@ -597,13 +602,14 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > return 0; > } > > - ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n); > - if (ret < 0) { > - return ret; > + status = bdrv_get_block_status_above(bs, base, sector_num, > + nb_sectors, &n, &file); Eventually, we should probably fix bdrv_get_block_status_above() to be byte-based, but that's not a problem with this patch. Looks okay to me, but I haven't thought closely enough about potential corner cases to feel comfortable with giving R-b yet...
On Fri, Sep 09, 2016 at 03:31:48PM +0300, Denis V. Lunev wrote: > bdrv_is_allocated_above() returns true in the case even for completel > zeroed areas as BDRV_BLOCK_ALLOCATED flag is set in both cases. > > The patch stops using bdrv_is_allocated_above() wrapper and switches to > bdrv_get_block_status_above() to distinguish zeroed areas and areas with > data to avoid extra IO operations if possible. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> > CC: Fam Zheng <famz@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Max Reitz <mreitz@redhat.com> > CC: Jeff Cody <jcody@redhat.com> > --- > block/mirror.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index e0b3f41..da55375 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -548,14 +548,15 @@ static void mirror_throttle(MirrorBlockJob *s) > > static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > { > - int64_t sector_num, end; > + int64_t sector_num, end, alloc_mask; > BlockDriverState *base = s->base; > BlockDriverState *bs = blk_bs(s->common.blk); > BlockDriverState *target_bs = blk_bs(s->target); > - int ret, n; > + int n; > > end = s->bdev_length / BDRV_SECTOR_SIZE; > > + alloc_mask = BDRV_BLOCK_ALLOCATED; > if (base == NULL && !bdrv_has_zero_init(target_bs)) { > if (!bdrv_can_write_zeroes_with_unmap(target_bs)) { > bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end); > @@ -583,6 +584,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > } > > mirror_drain(s); > + > + alloc_mask = BDRV_BLOCK_DATA; What about when base == NULL, and bdrv_has_zero_init(target_bs) == true? In that case we also know the target image is zeroed, but this does not take advantage of that. > } > > /* First part, loop on the sectors and initialize the dirty bitmap. */ > @@ -590,6 +593,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > /* Just to make sure we are not exceeding int limit. */ > int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS, > end - sector_num); > + int64_t status; > + BlockDriverState *file; > > mirror_throttle(s); > > @@ -597,13 +602,14 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > return 0; > } > > - ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n); > - if (ret < 0) { > - return ret; > + status = bdrv_get_block_status_above(bs, base, sector_num, > + nb_sectors, &n, &file); > + if (status < 0) { > + return status; > } > > assert(n > 0); > - if (ret == 1) { > + if (status & alloc_mask) { > bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n); > } > sector_num += n; -Jeff
diff --git a/block/mirror.c b/block/mirror.c index e0b3f41..da55375 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -548,14 +548,15 @@ static void mirror_throttle(MirrorBlockJob *s) static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) { - int64_t sector_num, end; + int64_t sector_num, end, alloc_mask; BlockDriverState *base = s->base; BlockDriverState *bs = blk_bs(s->common.blk); BlockDriverState *target_bs = blk_bs(s->target); - int ret, n; + int n; end = s->bdev_length / BDRV_SECTOR_SIZE; + alloc_mask = BDRV_BLOCK_ALLOCATED; if (base == NULL && !bdrv_has_zero_init(target_bs)) { if (!bdrv_can_write_zeroes_with_unmap(target_bs)) { bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end); @@ -583,6 +584,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) } mirror_drain(s); + + alloc_mask = BDRV_BLOCK_DATA; } /* First part, loop on the sectors and initialize the dirty bitmap. */ @@ -590,6 +593,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) /* Just to make sure we are not exceeding int limit. */ int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS, end - sector_num); + int64_t status; + BlockDriverState *file; mirror_throttle(s); @@ -597,13 +602,14 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) return 0; } - ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n); - if (ret < 0) { - return ret; + status = bdrv_get_block_status_above(bs, base, sector_num, + nb_sectors, &n, &file); + if (status < 0) { + return status; } assert(n > 0); - if (ret == 1) { + if (status & alloc_mask) { bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n); } sector_num += n;
bdrv_is_allocated_above() returns true in the case even for completel zeroed areas as BDRV_BLOCK_ALLOCATED flag is set in both cases. The patch stops using bdrv_is_allocated_above() wrapper and switches to bdrv_get_block_status_above() to distinguish zeroed areas and areas with data to avoid extra IO operations if possible. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Stefan Hajnoczi <stefanha@redhat.com> CC: Fam Zheng <famz@redhat.com> CC: Kevin Wolf <kwolf@redhat.com> CC: Max Reitz <mreitz@redhat.com> CC: Jeff Cody <jcody@redhat.com> --- block/mirror.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)