All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PULL 38/69] block/block-copy: refactor copying
Date: Mon, 28 Oct 2019 13:14:30 +0100	[thread overview]
Message-ID: <20191028121501.15279-39-mreitz@redhat.com> (raw)
In-Reply-To: <20191028121501.15279-1-mreitz@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Merge copying code into one function block_copy_do_copy, which only
calls bdrv_ io functions and don't do any synchronization (like dirty
bitmap set/reset).

Refactor block_copy() function so that it takes full decision about
size of chunk to be copied and does all the synchronization (checking
intersecting requests, set/reset dirty bitmaps).

It will help:
 - introduce parallel processing of block_copy iterations: we need to
   calculate chunk size, start async chunk copying and go to the next
   iteration
 - simplify synchronization improvement (like memory limiting in
   further commit and reducing critical section (now we lock the whole
   requested range, when actually we need to lock only dirty region
   which we handle at the moment))

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20191022111805.3432-4-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c | 118 ++++++++++++++++++++-------------------------
 block/trace-events |   6 +--
 2 files changed, 54 insertions(+), 70 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index cd45b373a9..845b2423dc 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -126,79 +126,64 @@ void block_copy_set_callbacks(
 }
 
 /*
- * Copy range to target with a bounce buffer and return the bytes copied. If
- * error occurred, return a negative error number
+ * block_copy_do_copy
+ *
+ * Do copy of cluser-aligned chunk. @end is allowed to exceed s->len only to
+ * cover last cluster when s->len is not aligned to clusters.
+ *
+ * No sync here: nor bitmap neighter intersecting requests handling, only copy.
+ *
+ * Returns 0 on success.
  */
-static int coroutine_fn block_copy_with_bounce_buffer(BlockCopyState *s,
-                                                      int64_t start,
-                                                      int64_t end,
-                                                      bool *error_is_read)
+static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
+                                           int64_t start, int64_t end,
+                                           bool *error_is_read)
 {
     int ret;
-    int nbytes;
-    void *bounce_buffer = qemu_blockalign(s->source->bs, s->cluster_size);
+    int nbytes = MIN(end, s->len) - start;
+    void *bounce_buffer = NULL;
 
     assert(QEMU_IS_ALIGNED(start, s->cluster_size));
-    bdrv_reset_dirty_bitmap(s->copy_bitmap, start, s->cluster_size);
-    nbytes = MIN(s->cluster_size, s->len - start);
+    assert(QEMU_IS_ALIGNED(end, s->cluster_size));
+    assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size));
+
+    if (s->use_copy_range) {
+        ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
+                                 0, s->write_flags);
+        if (ret < 0) {
+            trace_block_copy_copy_range_fail(s, start, ret);
+            s->use_copy_range = false;
+            /* Fallback to read+write with allocated buffer */
+        } else {
+            goto out;
+        }
+    }
+
+    bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
 
     ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0);
     if (ret < 0) {
-        trace_block_copy_with_bounce_buffer_read_fail(s, start, ret);
+        trace_block_copy_read_fail(s, start, ret);
         if (error_is_read) {
             *error_is_read = true;
         }
-        goto fail;
+        goto out;
     }
 
     ret = bdrv_co_pwrite(s->target, start, nbytes, bounce_buffer,
                          s->write_flags);
     if (ret < 0) {
-        trace_block_copy_with_bounce_buffer_write_fail(s, start, ret);
+        trace_block_copy_write_fail(s, start, ret);
         if (error_is_read) {
             *error_is_read = false;
         }
-        goto fail;
+        goto out;
     }
 
+out:
     qemu_vfree(bounce_buffer);
 
-    return nbytes;
-fail:
-    qemu_vfree(bounce_buffer);
-    bdrv_set_dirty_bitmap(s->copy_bitmap, start, s->cluster_size);
     return ret;
-
-}
-
-/*
- * Copy range to target and return the bytes copied. If error occurred, return a
- * negative error number.
- */
-static int coroutine_fn block_copy_with_offload(BlockCopyState *s,
-                                                int64_t start,
-                                                int64_t end)
-{
-    int ret;
-    int nr_clusters;
-    int nbytes;
-
-    assert(QEMU_IS_ALIGNED(s->copy_range_size, s->cluster_size));
-    assert(QEMU_IS_ALIGNED(start, s->cluster_size));
-    nbytes = MIN(s->copy_range_size, MIN(end, s->len) - start);
-    nr_clusters = DIV_ROUND_UP(nbytes, s->cluster_size);
-    bdrv_reset_dirty_bitmap(s->copy_bitmap, start,
-                            s->cluster_size * nr_clusters);
-    ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
-                             0, s->write_flags);
-    if (ret < 0) {
-        trace_block_copy_with_offload_fail(s, start, ret);
-        bdrv_set_dirty_bitmap(s->copy_bitmap, start,
-                              s->cluster_size * nr_clusters);
-        return ret;
-    }
-
-    return nbytes;
 }
 
 /*
@@ -294,7 +279,7 @@ int coroutine_fn block_copy(BlockCopyState *s,
     block_copy_inflight_req_begin(s, &req, start, end);
 
     while (start < end) {
-        int64_t dirty_end;
+        int64_t next_zero, chunk_end;
 
         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
             trace_block_copy_skip(s, start);
@@ -302,10 +287,15 @@ int coroutine_fn block_copy(BlockCopyState *s,
             continue; /* already copied */
         }
 
-        dirty_end = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
-                                                (end - start));
-        if (dirty_end < 0) {
-            dirty_end = end;
+        chunk_end = MIN(end, start + (s->use_copy_range ?
+                                      s->copy_range_size : s->cluster_size));
+
+        next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
+                                                chunk_end - start);
+        if (next_zero >= 0) {
+            assert(next_zero > start); /* start is dirty */
+            assert(next_zero < chunk_end); /* no need to do MIN() */
+            chunk_end = next_zero;
         }
 
         if (s->skip_unallocated) {
@@ -316,27 +306,21 @@ int coroutine_fn block_copy(BlockCopyState *s,
                 continue;
             }
             /* Clamp to known allocated region */
-            dirty_end = MIN(dirty_end, start + status_bytes);
+            chunk_end = MIN(chunk_end, start + status_bytes);
         }
 
         trace_block_copy_process(s, start);
 
-        if (s->use_copy_range) {
-            ret = block_copy_with_offload(s, start, dirty_end);
-            if (ret < 0) {
-                s->use_copy_range = false;
-            }
-        }
-        if (!s->use_copy_range) {
-            ret = block_copy_with_bounce_buffer(s, start, dirty_end,
-                                                error_is_read);
-        }
+        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
+
+        ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
         if (ret < 0) {
+            bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
             break;
         }
 
-        start += ret;
-        s->progress_bytes_callback(ret, s->progress_opaque);
+        s->progress_bytes_callback(chunk_end - start, s->progress_opaque);
+        start = chunk_end;
         ret = 0;
     }
 
diff --git a/block/trace-events b/block/trace-events
index b8d70f5242..ccde15a14c 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -45,9 +45,9 @@ backup_do_cow_return(void *job, int64_t offset, uint64_t bytes, int ret) "job %p
 block_copy_skip(void *bcs, int64_t start) "bcs %p start %"PRId64
 block_copy_skip_range(void *bcs, int64_t start, uint64_t bytes) "bcs %p start %"PRId64" bytes %"PRId64
 block_copy_process(void *bcs, int64_t start) "bcs %p start %"PRId64
-block_copy_with_bounce_buffer_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
-block_copy_with_bounce_buffer_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
-block_copy_with_offload_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_copy_range_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
 
 # ../blockdev.c
 qmp_block_job_cancel(void *job) "job %p"
-- 
2.21.0



  parent reply	other threads:[~2019-10-28 12:57 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28 12:13 [PULL 00/69] Block patches for softfreeze Max Reitz
2019-10-28 12:13 ` [PULL 01/69] iotests: Prefer null-co over null-aio Max Reitz
2019-10-28 12:13 ` [PULL 02/69] iotests: Allow skipping test cases Max Reitz
2019-10-28 12:13 ` [PULL 03/69] iotests: Use case_skip() in skip_if_unsupported() Max Reitz
2019-10-28 12:13 ` [PULL 04/69] iotests: Let skip_if_unsupported accept a function Max Reitz
2019-10-28 12:13 ` [PULL 05/69] iotests: Test driver whitelisting in 093 Max Reitz
2019-10-28 12:13 ` [PULL 06/69] iotests: Test driver whitelisting in 136 Max Reitz
2019-10-28 12:13 ` [PULL 07/69] iotests: Cache supported_formats() Max Reitz
2019-10-28 12:14 ` [PULL 08/69] hbitmap: handle set/reset with zero length Max Reitz
2019-10-28 12:14 ` [PULL 09/69] block/mirror: simplify do_sync_target_write Max Reitz
2019-10-28 12:14 ` [PULL 10/69] block/block-backend: add blk_co_pwritev_part Max Reitz
2019-10-28 12:14 ` [PULL 11/69] block/mirror: support unaligned write in active mirror Max Reitz
2019-10-28 12:14 ` [PULL 12/69] Revert "mirror: Only mirror granularity-aligned chunks" Max Reitz
2019-10-28 12:14 ` [PULL 13/69] iotests: Introduce $SOCK_DIR Max Reitz
2019-10-28 12:14 ` [PULL 14/69] iotests.py: Store socket files in $SOCK_DIR Max Reitz
2019-10-28 12:14 ` [PULL 15/69] iotests.py: Add @base_dir to FilePaths etc Max Reitz
2019-10-28 12:14 ` [PULL 16/69] iotests: Filter $SOCK_DIR Max Reitz
2019-10-28 12:14 ` [PULL 17/69] iotests: Let common.nbd create socket in $SOCK_DIR Max Reitz
2019-10-28 12:14 ` [PULL 18/69] iotests/083: Create " Max Reitz
2019-10-28 12:14 ` [PULL 19/69] iotests/140: " Max Reitz
2019-10-28 12:14 ` [PULL 20/69] iotests/143: " Max Reitz
2019-10-28 12:14 ` [PULL 21/69] iotests/147: " Max Reitz
2019-10-28 12:14 ` [PULL 22/69] iotests/181: " Max Reitz
2019-10-28 12:14 ` [PULL 23/69] iotests/182: " Max Reitz
2019-10-28 12:14 ` [PULL 24/69] iotests/183: " Max Reitz
2019-10-28 12:14 ` [PULL 25/69] iotests/192: " Max Reitz
2019-10-28 12:14 ` [PULL 26/69] iotests/194: Create sockets " Max Reitz
2019-10-28 12:14 ` [PULL 27/69] iotests/201: Create socket " Max Reitz
2019-10-28 12:14 ` [PULL 28/69] iotests/205: " Max Reitz
2019-10-28 12:14 ` [PULL 29/69] iotests/208: " Max Reitz
2019-10-28 12:14 ` [PULL 30/69] iotests/209: " Max Reitz
2019-10-28 12:14 ` [PULL 31/69] iotests/222: " Max Reitz
2019-10-28 12:14 ` [PULL 32/69] iotests/223: " Max Reitz
2019-10-28 12:14 ` [PULL 33/69] iotests/240: " Max Reitz
2019-10-28 12:14 ` [PULL 34/69] iotests/267: " Max Reitz
2019-10-28 12:14 ` [PULL 35/69] iotests: Drop TEST_DIR filter from _filter_nbd Max Reitz
2019-10-28 12:14 ` [PULL 36/69] block/block-copy: allocate buffer in block_copy_with_bounce_buffer Max Reitz
2019-10-28 12:14 ` [PULL 37/69] block/block-copy: limit copy_range_size to 16 MiB Max Reitz
2019-10-28 12:14 ` Max Reitz [this message]
2019-10-28 12:14 ` [PULL 39/69] util: introduce SharedResource Max Reitz
2019-10-28 12:14 ` [PULL 40/69] block/block-copy: add memory limit Max Reitz
2019-10-28 12:14 ` [PULL 41/69] block/block-copy: increase buffered copy request Max Reitz
2019-10-28 12:14 ` [PULL 42/69] block/nvme: add support for write zeros Max Reitz
2019-10-28 12:14 ` [PULL 43/69] block/nvme: add support for discard Max Reitz
2019-10-28 12:14 ` [PULL 44/69] mirror: Do not dereference invalid pointers Max Reitz
2019-10-28 12:14 ` [PULL 45/69] include: Move endof() up from hw/virtio/virtio.h Max Reitz
2019-10-28 12:14 ` [PULL 46/69] qcow2: Use endof() Max Reitz
2019-10-28 12:14 ` [PULL 47/69] qcow2: Add Error ** to qcow2_read_snapshots() Max Reitz
2019-10-28 12:14 ` [PULL 48/69] qcow2: Keep unknown extra snapshot data Max Reitz
2019-10-28 12:14 ` [PULL 49/69] qcow2: Make qcow2_write_snapshots() public Max Reitz
2019-10-28 12:14 ` [PULL 50/69] qcow2: Put qcow2_upgrade() into its own function Max Reitz
2019-10-28 12:14 ` [PULL 51/69] qcow2: Write v3-compliant snapshot list on upgrade Max Reitz
2019-10-28 12:14 ` [PULL 52/69] qcow2: Separate qcow2_check_read_snapshot_table() Max Reitz
2019-10-28 12:14 ` [PULL 53/69] qcow2: Add qcow2_check_fix_snapshot_table() Max Reitz
2019-10-28 12:14 ` [PULL 54/69] qcow2: Fix broken snapshot table entries Max Reitz
2019-10-28 12:14 ` [PULL 55/69] qcow2: Keep track of the snapshot table length Max Reitz
2019-10-28 12:14 ` [PULL 56/69] qcow2: Fix overly long snapshot tables Max Reitz
2019-10-28 12:14 ` [PULL 57/69] qcow2: Repair snapshot table with too many entries Max Reitz
2019-10-28 12:14 ` [PULL 58/69] qcow2: Fix v3 snapshot table entry compliancy Max Reitz
2019-10-28 12:14 ` [PULL 59/69] iotests: Add peek_file* functions Max Reitz
2019-10-28 12:14 ` [PULL 60/69] iotests: Test qcow2's snapshot table handling Max Reitz
2019-10-28 12:14 ` [PULL 61/69] block: Handle filter truncation like native impl Max Reitz
2019-10-28 12:14 ` [PULL 62/69] block/cor: Drop cor_co_truncate() Max Reitz
2019-10-28 12:14 ` [PULL 63/69] block: Do not truncate file node when formatting Max Reitz
2019-10-28 12:14 ` [PULL 64/69] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
2019-10-28 12:14 ` [PULL 65/69] block: Evaluate @exact in protocol drivers Max Reitz
2019-10-28 12:14 ` [PULL 66/69] block: Let format drivers pass @exact Max Reitz
2019-10-28 12:14 ` [PULL 67/69] block: Pass truncate exact=true where reasonable Max Reitz
2019-10-28 12:15 ` [PULL 68/69] Revert "qemu-img: Check post-truncation size" Max Reitz
2019-10-28 12:15 ` [PULL 69/69] qemu-iotests: restrict 264 to qcow2 only Max Reitz
2019-10-28 21:13 ` [PULL 00/69] Block patches for softfreeze Peter Maydell

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=20191028121501.15279-39-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --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.