[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] block: split write_zeroes always
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] block: split write_zeroes always |
Date: |
Wed, 25 May 2016 12:36:14 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 05/17/2016 10:34 AM, Kevin Wolf wrote:
> Am 17.05.2016 um 11:15 hat Denis V. Lunev geschrieben:
>> We should split requests even if they are less than write_zeroes_alignment.
>> For example we can have the following request:
>> offset 62k
>> size 4k
>> write_zeroes_alignment 64k
>> The original code sent 1 request covering 2 qcow2 clusters, and resulted
>> in both clusters being allocated. But by splitting the request, we can
>> cater to the case where one of the two clusters can be zeroed as a
>> whole, for only 1 cluster allocated after the operation.
>>
>> Signed-off-by: Denis V. Lunev <address@hidden>
>> CC: Eric Blake <address@hidden>
>> CC: Kevin Wolf <address@hidden>
>> ---
>> block/io.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index cd6d71a..6a24ea8 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1172,13 +1172,13 @@ static int coroutine_fn
>> bdrv_co_do_write_zeroes(BlockDriverState *bs,
>> /* Align request. Block drivers can expect the "bulk" of the
>> request
>> * to be aligned.
>> */
>> - if (bs->bl.write_zeroes_alignment
>> - && num > bs->bl.write_zeroes_alignment) {
>> + if (bs->bl.write_zeroes_alignment) {
>> if (sector_num % bs->bl.write_zeroes_alignment != 0) {
>> /* Make a small request up to the first aligned sector. */
>> num = bs->bl.write_zeroes_alignment;
>> num -= sector_num % bs->bl.write_zeroes_alignment;
>
> Turns out this doesn't work. If this is a small request that zeros
> something in the middle of a single cluster (i.e. we have untouched data
> both before and after the request in the same cluster), then num can now
> become greater than nb_sectors, so that we end up zeroing too much.
I'm planning on folding in a working version of this patch in my
byte-based write_zeroes conversion series. As part of the patch, I'm
also hoisting the division out of the loop (no guarantees that the
compiler can spot that bs->bl.write_zeroes_alignment will be a power of
two, to optimize it to a shift).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 0/5] qcow2_co_write_zeroes and related improvements, Denis V. Lunev, 2016/05/17
- [Qemu-devel] [PATCH 4/5] qcow2: fix condition in is_zero_cluster, Denis V. Lunev, 2016/05/17
- [Qemu-devel] [PATCH 5/5] qcow2: merge is_zero_cluster helpers into qcow2_co_write_zeroes, Denis V. Lunev, 2016/05/17
- [Qemu-devel] [PATCH 2/5] qcow2: simplify logic in qcow2_co_write_zeroes, Denis V. Lunev, 2016/05/17
- [Qemu-devel] [PATCH 3/5] qcow2: add tracepoints for qcow2_co_write_zeroes, Denis V. Lunev, 2016/05/17
- [Qemu-devel] [PATCH 1/5] block: split write_zeroes always, Denis V. Lunev, 2016/05/17
- Re: [Qemu-devel] [PATCH v2 0/5] qcow2_co_write_zeroes and related improvements, Kevin Wolf, 2016/05/17