Re: [Qemu-devel] [PATCH] block: fix bdrv_exceed_iops_limits wait computation

["Benoît Canet" <benoit.canet@irqsave.net>]

The +1 was here to account the current request as already done in this slice.
Statistically there is 50% chance that it will be wrong.
I toyed adding + 0.5 add wait_time doesn't drift anymore while iops don't
oscillate.

diff --git a/block.c b/block.c
index 0a062c9..455d8b0 100644
--- a/block.c
+++ b/block.c
@@ -3739,7 +3739,7 @@ static bool bdrv_exceed_iops_limits(BlockDriverState *bs,
bool is_write,
     }
 
     /* Calc approx time to dispatch */
-    wait_time = (ios_base + 1) / iops_limit;
+    wait_time = (ios_base + 0.5) / iops_limit;
     if (wait_time > elapsed_time) {
         wait_time = wait_time - elapsed_time;
     } else {

I will let a vm run this patch for a while

Regards

Benoît

> Le Thursday 21 Mar 2013 à 11:34:53 (+0100), Stefan Hajnoczi a écrit :
> On Wed, Mar 20, 2013 at 04:27:14PM +0100, Benoît Canet wrote:
> > > Now there is no oscillation and the wait_times do not grow or shrink
> > > under constant load from dd(1).
> > >
> > > Can you try this patch by itself to see if it fixes the oscillation?
> > 
> > On my test setup it fixes the oscillation and lead to an average 149.88 iops.
> > However another pattern appear.
> > iostat -d 1 -x will show something between 150 and 160 iops for several sample
> > then a sample would show around 70 iops to compensate for the additional ios
> > and this cycle restart.
> 
> I've begun drilling down on these fluctuations.
> 
> I think the problem is that I/O throttling uses bdrv_acct_done()
> accounting.  bdrv_acct_done is only called when requests complete.  This
> has the following problem:
> 
> Number of IOPS in this slice @ 150 IOPS = 15 ops per 100 ms slice
> 
> 14 ops have completed already, only 1 more can proceed.
> 
> 3 ops arrive in rapid succession:
> 
> Op #1: Allowed through since 1 op can proceed.  We submit the op.
> Op #2: Allowed through since op #1 is still in progress so
>        bdrv_acct_done() has not been called yet.
> Op #3: Allowed through since op #1 & #2 are still in progress so
>        bdrv_acct_done() has not been called yet.
> 
> Now when the ops start completing and the slice is extended we end up
> with weird wait times since we overspent our budget.
> 
> I'm going to try a fix for delayed accounting.  Will report back with
> patches if it is successful.
> 
> Stefan
>