[Qemu-devel] [PATCH] ratelimit: don't align wait time with slices

[Qemu-devel] [PATCH] ratelimit: don't align wait time with slices

[Wolfgang Bumiller]

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(-)

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;
 }
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH] ratelimit: don't align wait time with slices

[Alberto Garcia]

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

Re: [Qemu-devel] [PATCH] ratelimit: don't align wait time with slices

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]