[Top][All Lists]
[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
- [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Peter Lieven, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Markus Armbruster, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Paolo Bonzini, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Markus Armbruster, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking,
Peter Lieven <=
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Paolo Bonzini, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Peter Lieven, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Paolo Bonzini, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Peter Lieven, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Peter Lieven, 2013/01/10
- Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Paolo Bonzini, 2013/01/10
Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking, Peter Lieven, 2013/01/10