On Mon, 11/23 17:01, Wen Congyang wrote: > On 11/20/2015 05:59 PM, Fam Zheng wrote: > > "s->bitmap" tracks done sectors, we only check bit states without using any > > iterator which HBitmap is good for. Switch to "Bitmap" which is simpler and > > more memory efficient. > > > > Meanwhile, rename it to done_bitmap, to reflect the intention. > > > > Signed-off-by: Fam Zheng <f...@redhat.com> > > --- > > block/backup.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/block/backup.c b/block/backup.c > > index 3b39119..d408f98 100644 > > --- a/block/backup.c > > +++ b/block/backup.c > > @@ -22,6 +22,7 @@ > > #include "qapi/qmp/qerror.h" > > #include "qemu/ratelimit.h" > > #include "sysemu/block-backend.h" > > +#include "qemu/bitmap.h" > > > > #define BACKUP_CLUSTER_BITS 16 > > #define BACKUP_CLUSTER_SIZE (1 << BACKUP_CLUSTER_BITS) > > @@ -47,7 +48,7 @@ typedef struct BackupBlockJob { > > BlockdevOnError on_target_error; > > CoRwlock flush_rwlock; > > uint64_t sectors_read; > > - HBitmap *bitmap; > > + unsigned long *done_bitmap; > > QLIST_HEAD(, CowRequest) inflight_reqs; > > } BackupBlockJob; > > > > @@ -113,7 +114,7 @@ static int coroutine_fn backup_do_cow(BlockDriverState > > *bs, > > cow_request_begin(&cow_request, job, start, end); > > > > for (; start < end; start++) { > > - if (hbitmap_get(job->bitmap, start)) { > > + if (test_bit(start, job->done_bitmap)) { > > trace_backup_do_cow_skip(job, start); > > continue; /* already copied */ > > } > > @@ -164,7 +165,7 @@ static int coroutine_fn backup_do_cow(BlockDriverState > > *bs, > > goto out; > > } > > > > - hbitmap_set(job->bitmap, start, 1); > > + bitmap_set(job->done_bitmap, start, 1); > > You can use set_bit() here.
Why? I think bitmap_set is a better match with bitmap_new below. Fam > > Thanks > Wen Congyang > > > > > /* Publish progress, guest I/O counts as progress too. Note that > > the > > * offset field is an opaque progress value, it is not a disk > > offset. > > @@ -394,7 +395,7 @@ static void coroutine_fn backup_run(void *opaque) > > start = 0; > > end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE); > > > > - job->bitmap = hbitmap_alloc(end, 0); > > + job->done_bitmap = bitmap_new(end); > > > > bdrv_set_enable_write_cache(target, true); > > if (target->blk) { > > @@ -475,7 +476,7 @@ static void coroutine_fn backup_run(void *opaque) > > /* wait until pending backup_do_cow() calls have completed */ > > qemu_co_rwlock_wrlock(&job->flush_rwlock); > > qemu_co_rwlock_unlock(&job->flush_rwlock); > > - hbitmap_free(job->bitmap); > > + g_free(job->done_bitmap); > > > > if (target->blk) { > > blk_iostatus_disable(target->blk); > > >