On Wed, Mar 27, 2013 at 9:50 AM, Zhi Yong Wu <zwu.ker...@gmail.com> wrote: > On Thu, Mar 21, 2013 at 10:49 PM, Stefan Hajnoczi <stefa...@redhat.com> 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 <stefa...@redhat.com> >> --- >> 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. Stefan