qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] qemu-img: use blk_co_pwrite_zeroes for zero sec


From: 858585 jemmy
Subject: Re: [Qemu-devel] [PATCH] qemu-img: use blk_co_pwrite_zeroes for zero sectors when compressed
Date: Fri, 21 Apr 2017 10:58:36 +0800

On Thu, Apr 20, 2017 at 6:00 PM, Kevin Wolf <address@hidden> wrote:
> Am 20.04.2017 um 10:38 hat address@hidden geschrieben:
>> From: Lidong Chen <address@hidden>
>>
>> when the buffer is zero, blk_co_pwrite_zeroes is more effectively than
>> blk_co_pwritev with BDRV_REQ_WRITE_COMPRESSED. this patch can reduces
>> the time when converts the qcow2 image with lots of zero.
>>
>> Signed-off-by: Lidong Chen <address@hidden>
>
> Good catch, using blk_co_pwrite_zeroes() makes sense even for compressed
> images.
>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b220cf7..0256539 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1675,13 +1675,20 @@ static int coroutine_fn 
>> convert_co_write(ImgConvertState *s, int64_t sector_num,
>>               * write if the buffer is completely zeroed and we're allowed to
>>               * keep the target sparse. */
>>              if (s->compressed) {
>> -                if (s->has_zero_init && s->min_sparse &&
>> -                    buffer_is_zero(buf, n * BDRV_SECTOR_SIZE))
>> -                {
>> -                    assert(!s->target_has_backing);
>> -                    break;
>> +                if (buffer_is_zero(buf, n * BDRV_SECTOR_SIZE)) {
>> +                    if (s->has_zero_init && s->min_sparse) {
>> +                        assert(!s->target_has_backing);
>> +                        break;
>> +                    } else {
>> +                        ret = blk_co_pwrite_zeroes(s->target,
>> +                                           sector_num << BDRV_SECTOR_BITS,
>> +                                           n << BDRV_SECTOR_BITS, 0);
>> +                        if (ret < 0) {
>> +                            return ret;
>> +                        }
>> +                        break;
>> +                    }
>>                  }
>
> If s->min_sparse == 0, we may neither skip the write not use
> blk_co_pwrite_zeroes(), because this requires actual full allocation
> with explicit zero sectors.
>
> Of course, if you fix this, what you end up with here is a duplicate of
> the code path for non-compressed images. The remaining difference seems
> to be the BDRV_REQ_WRITE_COMPRESSED flag and buffer_is_zero() vs.
> is_allocated_sectors_min() (because uncompressed clusters can be written
> partially, but compressed clusters can't).

I have a try to unify the code.

I don't understand why use  is_allocated_sectors_min when don't compressed.
the s->min_sparse is 8 default, which is smaller than cluster_sectors.

if a cluster which data is  8 sector zero and 8 sector non-zero
repeated, it will call
blk_co_pwritev and blk_co_pwrite_zeroes many times for a cluster.

why not compare the zero by cluster_sectors size?

>
> So I suppose that instead of just fixing the above bug, we could actually
> mostly unify the two code paths, if you want to have a try at it.
>
> Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]