On 11/23/2015 05:55 PM, Fam Zheng wrote:
> On Mon, 11/23 17:24, Wen Congyang wrote:
>> On 11/23/2015 05:19 PM, Fam Zheng wrote:
>>> 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.
>>
>> set_bit() is quicker than bitmap_set() if you only set one bit.
>>
>
> How much quicker is it? This doesn't sound convincing enough for me to lose
> the
> readability.
I don't test it. It is just a suggestion.
Thanks
Wen Congyang
>
> Fam
> .
>
