All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, qemu-block@nongnu.org, jsnow@redhat.com,
	Jeff Cody <jcody@redhat.com>, Max Reitz <mreitz@redhat.com>
Subject: [Qemu-devel] [PATCH v3 01/20] blockjob: Track job ratelimits via bytes, not sectors
Date: Tue, 27 Jun 2017 14:24:39 -0500	[thread overview]
Message-ID: <20170627192458.15519-2-eblake@redhat.com> (raw)
In-Reply-To: <20170627192458.15519-1-eblake@redhat.com>

The user interface specifies job rate limits in bytes/second.
It's pointless to have our internal representation track things
in sectors/second, particularly since we want to move away from
sector-based interfaces.

Fix up a doc typo found while verifying that the ratelimit
code handles the scaling difference.

Repetition of expressions like 'n * BDRV_SECTOR_SIZE' will be
cleaned up later when functions are converted to iterate over
images by bytes rather than by sectors.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>

---
v2: adjust commit message based on review; no code change
---
 include/qemu/ratelimit.h |  3 ++-
 block/backup.c           |  5 +++--
 block/commit.c           |  5 +++--
 block/mirror.c           | 13 +++++++------
 block/stream.c           |  5 +++--
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
index 8da1232..8dece48 100644
--- a/include/qemu/ratelimit.h
+++ b/include/qemu/ratelimit.h
@@ -24,7 +24,8 @@ typedef struct {

 /** Calculate and return delay for next request in ns
  *
- * Record that we sent @p n data units. If we may send more data units
+ * Record that we sent @n data units (where @n matches the scale chosen
+ * during ratelimit_set_speed). If we may send more data units
  * in the current time slice, return 0 (i.e. no delay). Otherwise
  * return the amount of time (in ns) until the start of the next time
  * slice that will permit sending the next chunk of data.
diff --git a/block/backup.c b/block/backup.c
index 5387fbd..9ca1d8e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -208,7 +208,7 @@ static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER, "speed");
         return;
     }
-    ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
+    ratelimit_set_speed(&s->limit, speed, SLICE_TIME);
 }

 static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
@@ -359,7 +359,8 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job)
      */
     if (job->common.speed) {
         uint64_t delay_ns = ratelimit_calculate_delay(&job->limit,
-                                                      job->sectors_read);
+                                                      job->sectors_read *
+                                                      BDRV_SECTOR_SIZE);
         job->sectors_read = 0;
         block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
     } else {
diff --git a/block/commit.c b/block/commit.c
index 524bd54..6993994 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -209,7 +209,8 @@ static void coroutine_fn commit_run(void *opaque)
         s->common.offset += n * BDRV_SECTOR_SIZE;

         if (copy && s->common.speed) {
-            delay_ns = ratelimit_calculate_delay(&s->limit, n);
+            delay_ns = ratelimit_calculate_delay(&s->limit,
+                                                 n * BDRV_SECTOR_SIZE);
         }
     }

@@ -231,7 +232,7 @@ static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER, "speed");
         return;
     }
-    ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
+    ratelimit_set_speed(&s->limit, speed, SLICE_TIME);
 }

 static const BlockJobDriver commit_job_driver = {
diff --git a/block/mirror.c b/block/mirror.c
index 61a862d..eb27efc 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -396,7 +396,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     bitmap_set(s->in_flight_bitmap, sector_num / sectors_per_chunk, nb_chunks);
     while (nb_chunks > 0 && sector_num < end) {
         int64_t ret;
-        int io_sectors, io_sectors_acct;
+        int io_sectors;
+        int64_t io_bytes_acct;
         BlockDriverState *file;
         enum MirrorMethod {
             MIRROR_METHOD_COPY,
@@ -444,16 +445,16 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         switch (mirror_method) {
         case MIRROR_METHOD_COPY:
             io_sectors = mirror_do_read(s, sector_num, io_sectors);
-            io_sectors_acct = io_sectors;
+            io_bytes_acct = io_sectors * BDRV_SECTOR_SIZE;
             break;
         case MIRROR_METHOD_ZERO:
         case MIRROR_METHOD_DISCARD:
             mirror_do_zero_or_discard(s, sector_num, io_sectors,
                                       mirror_method == MIRROR_METHOD_DISCARD);
             if (write_zeroes_ok) {
-                io_sectors_acct = 0;
+                io_bytes_acct = 0;
             } else {
-                io_sectors_acct = io_sectors;
+                io_bytes_acct = io_sectors * BDRV_SECTOR_SIZE;
             }
             break;
         default:
@@ -463,7 +464,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         sector_num += io_sectors;
         nb_chunks -= DIV_ROUND_UP(io_sectors, sectors_per_chunk);
         if (s->common.speed) {
-            delay_ns = ratelimit_calculate_delay(&s->limit, io_sectors_acct);
+            delay_ns = ratelimit_calculate_delay(&s->limit, io_bytes_acct);
         }
     }
     return delay_ns;
@@ -929,7 +930,7 @@ static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER, "speed");
         return;
     }
-    ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
+    ratelimit_set_speed(&s->limit, speed, SLICE_TIME);
 }

 static void mirror_complete(BlockJob *job, Error **errp)
diff --git a/block/stream.c b/block/stream.c
index 52d329f..29273a5 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -191,7 +191,8 @@ static void coroutine_fn stream_run(void *opaque)
         /* Publish progress */
         s->common.offset += n * BDRV_SECTOR_SIZE;
         if (copy && s->common.speed) {
-            delay_ns = ratelimit_calculate_delay(&s->limit, n);
+            delay_ns = ratelimit_calculate_delay(&s->limit,
+                                                 n * BDRV_SECTOR_SIZE);
         }
     }

@@ -220,7 +221,7 @@ static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER, "speed");
         return;
     }
-    ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
+    ratelimit_set_speed(&s->limit, speed, SLICE_TIME);
 }

 static const BlockJobDriver stream_job_driver = {
-- 
2.9.4

  reply	other threads:[~2017-06-27 19:25 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-27 19:24 [Qemu-devel] [PATCH v3 00/20] make bdrv_is_allocated[_above] byte-based Eric Blake
2017-06-27 19:24 ` Eric Blake [this message]
2017-06-30 19:42   ` [Qemu-devel] [PATCH v3 01/20] blockjob: Track job ratelimits via bytes, not sectors Jeff Cody
2017-07-04 15:28   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 02/20] trace: Show blockjob actions " Eric Blake
2017-06-30 19:56   ` Jeff Cody
2017-07-04 15:28   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 03/20] stream: Switch stream_populate() to byte-based Eric Blake
2017-06-30 19:57   ` Jeff Cody
2017-07-04 15:28   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 04/20] stream: Switch stream_run() " Eric Blake
2017-06-30 20:01   ` Jeff Cody
2017-07-04 15:00   ` Kevin Wolf
2017-07-05 12:13     ` Eric Blake
2017-07-05 13:41       ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 05/20] commit: Switch commit_populate() " Eric Blake
2017-06-30 20:17   ` Jeff Cody
2017-07-04 15:28   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 06/20] commit: Switch commit_run() " Eric Blake
2017-06-30 20:19   ` Jeff Cody
2017-07-04 15:29   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 07/20] mirror: Switch MirrorBlockJob " Eric Blake
2017-06-30 20:20   ` Jeff Cody
2017-07-05 11:42   ` Kevin Wolf
2017-07-05 20:18     ` Eric Blake
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 08/20] mirror: Switch mirror_do_zero_or_discard() " Eric Blake
2017-06-30 20:22   ` Jeff Cody
2017-07-05 16:36   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 09/20] mirror: Update signature of mirror_clip_sectors() Eric Blake
2017-06-30 20:51   ` Jeff Cody
2017-07-05 16:37   ` Kevin Wolf
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 10/20] mirror: Switch mirror_cow_align() to byte-based Eric Blake
2017-06-30 21:03   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 11/20] mirror: Switch mirror_do_read() " Eric Blake
2017-06-30 21:07   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 12/20] mirror: Switch mirror_iteration() " Eric Blake
2017-06-30 21:14   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 13/20] block: Drop unused bdrv_round_sectors_to_clusters() Eric Blake
2017-06-30 21:17   ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 14/20] backup: Switch BackupBlockJob to byte-based Eric Blake
2017-06-30 21:18   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 15/20] backup: Switch block_backup.h " Eric Blake
2017-06-28  0:38   ` Xie Changlong
2017-06-30 21:23   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 16/20] backup: Switch backup_do_cow() " Eric Blake
2017-06-30 21:24   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 17/20] backup: Switch backup_run() " Eric Blake
2017-06-30 21:25   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 18/20] block: Make bdrv_is_allocated() byte-based Eric Blake
2017-06-28  9:14   ` Juan Quintela
2017-06-30 21:32   ` Jeff Cody
2017-07-03 18:22   ` Eric Blake
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 19/20] block: Minimize raw use of bds->total_sectors Eric Blake
2017-06-28  8:57   ` [Qemu-devel] [Qemu-block] " Manos Pitsidianakis
2017-06-30 21:34   ` Jeff Cody
2017-06-27 19:24 ` [Qemu-devel] [PATCH v3 20/20] block: Make bdrv_is_allocated_above() byte-based Eric Blake
2017-06-28  0:38   ` Xie Changlong
2017-06-30 21:36   ` Jeff Cody

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170627192458.15519-2-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=jcody@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.