Patchwork [12/20] mirror: support arbitrarily-sized iterations

login
register
mail settings
Submitter Paolo Bonzini
Date Dec. 12, 2012, 1:46 p.m.
Message ID <1355319999-30627-13-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/205521/
State New
Headers show

Comments

Paolo Bonzini - Dec. 12, 2012, 1:46 p.m.
Yet another optimization is to extend the mirroring iteration to include more
adjacent dirty blocks.  This limits the number of I/O operations and makes
mirroring efficient even with a small granularity.  Most of the infrastructure
is already in place; we only need to put a loop around the computation of
the origin and sector count of the iteration.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/mirror.c | 100 +++++++++++++++++++++++++++++++++++++++------------------
 trace-events   |   1 +
 2 files changed, 69 insertions(+), 32 deletions(-)
Eric Blake - Dec. 14, 2012, 10:39 p.m.
On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> Yet another optimization is to extend the mirroring iteration to include more
> adjacent dirty blocks.  This limits the number of I/O operations and makes
> mirroring efficient even with a small granularity.  Most of the infrastructure
> is already in place; we only need to put a loop around the computation of
> the origin and sector count of the iteration.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/mirror.c | 100 +++++++++++++++++++++++++++++++++++++++------------------
>  trace-events   |   1 +
>  2 files changed, 69 insertions(+), 32 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

Patch

diff --git a/block/mirror.c b/block/mirror.c
index f9caaea..23f87d2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -135,7 +135,7 @@  static void coroutine_fn mirror_iteration(MirrorBlockJob *s)
 {
     BlockDriverState *source = s->common.bs;
     int nb_sectors, nb_sectors_chunk, nb_chunks;
-    int64_t end, sector_num, cluster_num, next_sector, hbitmap_next_sector;
+    int64_t end, sector_num, next_cluster, next_sector, hbitmap_next_sector;
     MirrorOp *op;
 
     s->sector_num = hbitmap_iter_next(&s->hbi);
@@ -147,47 +147,83 @@  static void coroutine_fn mirror_iteration(MirrorBlockJob *s)
     }
 
     hbitmap_next_sector = s->sector_num;
+    sector_num = s->sector_num;
+    nb_sectors_chunk = s->granularity >> BDRV_SECTOR_BITS;
+    end = s->common.len >> BDRV_SECTOR_BITS;
 
-    /* If we have no backing file yet in the destination, and the cluster size
-     * is very large, we need to do COW ourselves.  The first time a cluster is
-     * copied, copy it entirely.
+    /* Extend the QEMUIOVector to include all adjacent blocks that will
+     * be copied in this operation.
+     *
+     * We have to do this if we have no backing file yet in the destination,
+     * and the cluster size is very large.  Then we need to do COW ourselves.
+     * The first time a cluster is copied, copy it entirely.  Note that,
+     * because both the granularity and the cluster size are powers of two,
+     * the number of sectors to copy cannot exceed one cluster.
      *
-     * Because both the granularity and the cluster size are powers of two, the
-     * number of sectors to copy cannot exceed one cluster.
+     * We also want to extend the QEMUIOVector to include more adjacent
+     * dirty blocks if possible, to limit the number of I/O operations and
+     * run efficiently even with a small granularity.
      */
-    sector_num = s->sector_num;
-    nb_sectors_chunk = nb_sectors = s->granularity >> BDRV_SECTOR_BITS;
-    cluster_num = sector_num / nb_sectors_chunk;
-    if (s->cow_bitmap && !test_bit(cluster_num, s->cow_bitmap)) {
-        trace_mirror_cow(s, sector_num);
-        bdrv_round_to_clusters(s->target,
-                               sector_num, nb_sectors_chunk,
-                               &sector_num, &nb_sectors);
-
-        /* The rounding may make us copy sectors before the
-         * first dirty one.
-         */
-        cluster_num = sector_num / nb_sectors_chunk;
-    }
+    nb_chunks = 0;
+    nb_sectors = 0;
+    next_sector = sector_num;
+    next_cluster = sector_num / nb_sectors_chunk;
 
     /* Wait for I/O to this cluster (from a previous iteration) to be done.  */
-    while (test_bit(cluster_num, s->in_flight_bitmap)) {
+    while (test_bit(next_cluster, s->in_flight_bitmap)) {
         trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
         qemu_coroutine_yield();
     }
 
-    end = s->common.len >> BDRV_SECTOR_BITS;
-    nb_sectors = MIN(nb_sectors, end - sector_num);
-    nb_chunks = (nb_sectors + nb_sectors_chunk - 1) / nb_sectors_chunk;
-    while (s->buf_free_count < nb_chunks) {
-        trace_mirror_yield_buf_busy(s, nb_chunks, s->in_flight);
-        qemu_coroutine_yield();
-    }
+    do {
+        int added_sectors, added_chunks;
 
-    /* We have enough free space to copy these sectors.  */
-    if (s->cow_bitmap) {
-        bitmap_set(s->cow_bitmap, cluster_num, nb_chunks);
-    }
+        if (!bdrv_get_dirty(source, next_sector) ||
+            test_bit(next_cluster, s->in_flight_bitmap)) {
+            assert(nb_sectors > 0);
+            break;
+        }
+
+        added_sectors = nb_sectors_chunk;
+        if (s->cow_bitmap && !test_bit(next_cluster, s->cow_bitmap)) {
+            bdrv_round_to_clusters(s->target,
+                                   next_sector, added_sectors,
+                                   &next_sector, &added_sectors);
+
+            /* On the first iteration, the rounding may make us copy
+             * sectors before the first dirty one.
+             */
+            if (next_sector < sector_num) {
+                assert(nb_sectors == 0);
+                sector_num = next_sector;
+                next_cluster = next_sector / nb_sectors_chunk;
+            }
+        }
+
+        added_sectors = MIN(added_sectors, end - (sector_num + nb_sectors));
+        added_chunks = (added_sectors + nb_sectors_chunk - 1) / nb_sectors_chunk;
+
+        /* When doing COW, it may happen that there is not enough space for
+         * a full cluster.  Wait if that is the case.
+         */
+        while (nb_chunks == 0 && s->buf_free_count < added_chunks) {
+            trace_mirror_yield_buf_busy(s, nb_chunks, s->in_flight);
+            qemu_coroutine_yield();
+        }
+        if (s->buf_free_count < nb_chunks + added_chunks) {
+            trace_mirror_break_buf_busy(s, nb_chunks, s->in_flight);
+            break;
+        }
+
+        /* We have enough free space to copy these sectors.  */
+        if (s->cow_bitmap) {
+            bitmap_set(s->cow_bitmap, next_cluster, added_chunks);
+        }
+        nb_sectors += added_sectors;
+        nb_chunks += added_chunks;
+        next_sector += added_sectors;
+        next_cluster += added_chunks;
+    } while (next_sector < end);
 
     /* Allocate a MirrorOp that is used as an AIO callback.  */
     op = g_slice_new(MirrorOp);
diff --git a/trace-events b/trace-events
index 42068dd..3632efc 100644
--- a/trace-events
+++ b/trace-events
@@ -89,6 +89,7 @@  mirror_iteration_done(void *s, int64_t sector_num, int nb_sectors) "s %p sector_
 mirror_yield(void *s, int64_t cnt, int buf_free_count, int in_flight) "s %p dirty count %"PRId64" free buffers %d in_flight %d"
 mirror_yield_in_flight(void *s, int64_t sector_num, int in_flight) "s %p sector_num %"PRId64" in_flight %d"
 mirror_yield_buf_busy(void *s, int nb_chunks, int in_flight) "s %p requested chunks %d in_flight %d"
+mirror_break_buf_busy(void *s, int nb_chunks, int in_flight) "s %p requested chunks %d in_flight %d"
 
 # blockdev.c
 qmp_block_job_cancel(void *job) "job %p"