qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [RFC 1/4] block: fix I/O throttling accounting blind sp


From: Zhi Yong Wu
Subject: Re: [Qemu-devel] [RFC 1/4] block: fix I/O throttling accounting blind spot
Date: Wed, 27 Mar 2013 20:49:50 +0800

On Wed, Mar 27, 2013 at 5:14 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Wed, Mar 27, 2013 at 9:50 AM, Zhi Yong Wu <address@hidden> wrote:
>> On Thu, Mar 21, 2013 at 10:49 PM, Stefan Hajnoczi <address@hidden> wrote:
>>> I/O throttling relies on bdrv_acct_done() which is called when a request
>>> completes.  This leaves a blind spot since we only charge for completed
>>> requests, not submitted requests.
>>>
>>> For example, if there is 1 operation remaining in this time slice the
>>> guest could submit 3 operations and they will all be submitted
>>> successfully since they don't actually get accounted for until they
>>> complete.
>>>
>>> Originally we probably thought this is okay since the requests will be
>>> accounted when the time slice is extended.  In practice it causes
>>> fluctuations since the guest can exceed its I/O limit and it will be
>>> punished for this later on.
>>>
>>> Account for I/O upon submission so that I/O limits are enforced
>>> properly.
>>>
>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>> ---
>>>  block.c                   | 19 ++++++++-----------
>>>  include/block/block_int.h |  2 +-
>>>  2 files changed, 9 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index 0a062c9..31fb0b0 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -141,7 +141,6 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
>>>      bs->slice_start = 0;
>>>      bs->slice_end   = 0;
>>>      bs->slice_time  = 0;
>>> -    memset(&bs->io_base, 0, sizeof(bs->io_base));
>> If we try some concussive operations, enable I/O throttling at first,
>> then disable it, and then enable it, how about? I guess that
>> bs->slice_submitted will maybe be not correct.
>
> The memset() was moved...
>
>>> @@ -3772,11 +3773,7 @@ static bool bdrv_exceed_io_limits(BlockDriverState 
>>> *bs, int nb_sectors,
>>>          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];
>>> +        memset(&bs->slice_submitted, 0, sizeof(bs->slice_submitted));
>>>      }
>>>
>>>      elapsed_time  = now - bs->slice_start;
>
> ...here.
>
> Since bs->slice_start = 0 when I/O throttling is disabled we will
> start a new slice next time bdrv_exceed_io_limits() is called.
>
> Therefore bs->slice_submitted is consistent across disable/enable.
Yes, i also realized this just when i came home by subway.
>
> Stefan



-- 
Regards,

Zhi Yong Wu



reply via email to

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