diff mbox series

ratelimit: don't align wait time with slices

Message ID 20180207071758.6818-1-w.bumiller@proxmox.com
State New
Headers show
Series ratelimit: don't align wait time with slices | expand

Commit Message

Wolfgang Bumiller Feb. 7, 2018, 7:17 a.m. UTC
It is possible for rate limited writes to keep overshooting a slice's
quota by a tiny amount causing the slice-aligned waiting period to
effectively halve the rate.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
Copied the Ccs from the discussion thread, hope that's fine, as I also
just noticed that for my reply containing this snippet I had hit reply
on the mail that did not contain those Ccs yet, sorry about that.

 include/qemu/ratelimit.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Alberto Garcia Feb. 7, 2018, 12:57 p.m. UTC | #1
On Wed 07 Feb 2018 08:17:58 AM CET, Wolfgang Bumiller wrote:
> It is possible for rate limited writes to keep overshooting a slice's
> quota by a tiny amount causing the slice-aligned waiting period to
> effectively halve the rate.
>
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> Copied the Ccs from the discussion thread, hope that's fine, as I also
> just noticed that for my reply containing this snippet I had hit reply
> on the mail that did not contain those Ccs yet, sorry about that.

Stefan mentioned in another e-mail that someone proposed at some point
to unify this with the code in throttle.c. We can consider it but that
needs to be evaluated first. The other code is more complex and has
extra features (bursts) so it may not be worth the effort. Also the
algorithm is different. I can take a look in the future when I have some
time.

Otherwise, your patch looks good.

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
Stefan Hajnoczi Feb. 7, 2018, 3:53 p.m. UTC | #2
On Wed, Feb 07, 2018 at 08:17:58AM +0100, Wolfgang Bumiller wrote:
> It is possible for rate limited writes to keep overshooting a slice's
> quota by a tiny amount causing the slice-aligned waiting period to
> effectively halve the rate.
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> Copied the Ccs from the discussion thread, hope that's fine, as I also
> just noticed that for my reply containing this snippet I had hit reply
> on the mail that did not contain those Ccs yet, sorry about that.
> 
>  include/qemu/ratelimit.h | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan
diff mbox series

Patch

diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
index 8dece483f5..1b38291823 100644
--- a/include/qemu/ratelimit.h
+++ b/include/qemu/ratelimit.h
@@ -36,7 +36,7 @@  typedef struct {
 static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
 {
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-    uint64_t delay_slices;
+    double delay_slices;
 
     assert(limit->slice_quota && limit->slice_ns);
 
@@ -55,12 +55,11 @@  static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
         return 0;
     }
 
-    /* Quota exceeded. Calculate the next time slice we may start
-     * sending data again. */
-    delay_slices = (limit->dispatched + limit->slice_quota - 1) /
-        limit->slice_quota;
+    /* Quota exceeded. Wait based on the excess amount and then start a new
+     * slice. */
+    delay_slices = (double)limit->dispatched / limit->slice_quota;
     limit->slice_end_time = limit->slice_start_time +
-        delay_slices * limit->slice_ns;
+        (uint64_t)(delay_slices * limit->slice_ns);
     return limit->slice_end_time - now;
 }