qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid lock


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking
Date: Thu, 10 Jan 2013 12:57:08 +0100

Am 10.01.2013 um 12:52 schrieb Markus Armbruster <address@hidden>:

> Paolo Bonzini <address@hidden> writes:
> 
>> Il 10/01/2013 11:45, Markus Armbruster ha scritto:
>>> 
>>>>> If io_limits are specified during runtime that exceed the number
>>>>> of operations in flight
>>>>> bs->io_base is not initialized in the else statement in
>>>>> bdrv_exceed_io_limits().
>>> I'm confused.
>>> 
>>>    if ((bs->slice_start < now)
>>>        && (bs->slice_end > now)) {
>>>        bs->slice_end = now + bs->slice_time;
>>>    } else {
>>>        bs->slice_time  =  5 * BLOCK_IO_SLICE_TIME;
>>>        bs->slice_start = now;
>>>        bs->slice_end   = now + bs->slice_time;
>>> 
>>>        bs->io_base.bytes[is_write]  = bs->nr_bytes[is_write];
>>>        bs->io_base.bytes[!is_write] = bs->nr_bytes[!is_write];
>>> 
>>>        bs->io_base.ios[is_write]    = bs->nr_ops[is_write];
>>>        bs->io_base.ios[!is_write]   = bs->nr_ops[!is_write];
>>>    }
>> 
>> bdrv_io_limits_enable correctly starts a new slice (the first three
>> lines of the else), but does not set io_base correctly for that slice.
>> Here is how io_base is used:
>> 
>>    bytes_base  = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
>>    bytes_res   = (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>> 
>>    if (bytes_base + bytes_res <= bytes_limit) {
>>        /* no wait */
>>    } else {
>>        /* operation needs to be throttled */
>>    }
>> 
>> As a result, any I/O operations that are triggered between now and
>> bs->slice_end are incorrectly limited.  If 10 MB of data has been
>> written since the VM was started, QEMU thinks that 10 MB of data has
>> been written in this slice.
> 
> Work that into the commit message, and I'm happy :)

Paolo, if you agree I would resubmit the patch (using your description).

I would not directly collapse the code to as its not obvious what 
bdrv_exceed_io_limits(bs, 0, 0, NULL); 
is doing. Maybe this could be done in a later patch.

Peter




reply via email to

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