[Qemu-devel] [PATCH v2 0/2] Split up ram_find_and_save_block

[Qemu-devel] [PATCH v2 0/2] Split up ram_find_and_save_block

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Chop up ram_find_and_save_block so it's smaller again.
(from comments on my postcopy patch that adds more to it).

This pair is based on top of my previous 5 patch cleanup series posted
in August, but rebased on current qemu master.

Dave


Dr. David Alan Gilbert (2):
  Move dirty page search state into separate structure
  ram_find_and_save_block: Split out the finding

 migration/ram.c | 119 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 84 insertions(+), 35 deletions(-)

v2
  Typo fixes from Amit's review
-- 
2.5.0

[Qemu-devel] [PATCH v2 1/2] Move dirty page search state into separate structure

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Pull the search state for one iteration of the dirty page
search into a structure.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
---
 migration/ram.c | 55 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 7df9157..d79d79d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -227,6 +227,17 @@ static uint64_t migration_dirty_pages;
 static uint32_t last_version;
 static bool ram_bulk_stage;
 
+/* used by the search for pages to send */
+struct PageSearchStatus {
+    /* Current block being searched */
+    RAMBlock    *block;
+    /* Current offset to search from */
+    ram_addr_t   offset;
+    /* Set once we wrap around */
+    bool         complete_round;
+};
+typedef struct PageSearchStatus PageSearchStatus;
+
 struct CompressParam {
     bool start;
     bool done;
@@ -531,7 +542,6 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
         cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length);
 }
 
-
 /* Fix me: there are too many global variables used in migration process. */
 static int64_t start_time;
 static int64_t bytes_xfer_prev;
@@ -923,26 +933,30 @@ static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
 static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
                                    uint64_t *bytes_transferred)
 {
-    RAMBlock *block = last_seen_block;
-    ram_addr_t offset = last_offset;
-    bool complete_round = false;
+    PageSearchStatus pss;
     int pages = 0;
 
-    if (!block)
-        block = QLIST_FIRST_RCU(&ram_list.blocks);
+    pss.block = last_seen_block;
+    pss.offset = last_offset;
+    pss.complete_round = false;
+
+    if (!pss.block) {
+        pss.block = QLIST_FIRST_RCU(&ram_list.blocks);
+    }
 
     while (true) {
-        offset = migration_bitmap_find_and_reset_dirty(block, offset);
-        if (complete_round && block == last_seen_block &&
-            offset >= last_offset) {
+        pss.offset = migration_bitmap_find_and_reset_dirty(pss.block,
+                                                           pss.offset);
+        if (pss.complete_round && pss.block == last_seen_block &&
+            pss.offset >= last_offset) {
             break;
         }
-        if (offset >= block->used_length) {
-            offset = 0;
-            block = QLIST_NEXT_RCU(block, next);
-            if (!block) {
-                block = QLIST_FIRST_RCU(&ram_list.blocks);
-                complete_round = true;
+        if (pss.offset >= pss.block->used_length) {
+            pss.offset = 0;
+            pss.block = QLIST_NEXT_RCU(pss.block, next);
+            if (!pss.block) {
+                pss.block = QLIST_FIRST_RCU(&ram_list.blocks);
+                pss.complete_round = true;
                 ram_bulk_stage = false;
                 if (migrate_use_xbzrle()) {
                     /* If xbzrle is on, stop using the data compression at this
@@ -954,23 +968,24 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
             }
         } else {
             if (compression_switch && migrate_use_compression()) {
-                pages = ram_save_compressed_page(f, block, offset, last_stage,
+                pages = ram_save_compressed_page(f, pss.block, pss.offset,
+                                                 last_stage,
                                                  bytes_transferred);
             } else {
-                pages = ram_save_page(f, block, offset, last_stage,
+                pages = ram_save_page(f, pss.block, pss.offset, last_stage,
                                       bytes_transferred);
             }
 
             /* if page is unmodified, continue to the next */
             if (pages > 0) {
-                last_sent_block = block;
+                last_sent_block = pss.block;
                 break;
             }
         }
     }
 
-    last_seen_block = block;
-    last_offset = offset;
+    last_seen_block = pss.block;
+    last_offset = pss.offset;
 
     return pages;
 }
-- 
2.5.0

[Qemu-devel] [PATCH v2 2/2] ram_find_and_save_block: Split out the finding

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Split out the finding of the dirty page and all the wrap detection
into a separate function since it was getting a bit hairy.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c | 84 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 25 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index d79d79d..ba2b693 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -917,6 +917,59 @@ static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
     return pages;
 }
 
+/*
+ * Find the next dirty page and update any state associated with
+ * the search process.
+ *
+ * Returns: True if a page is found
+ *
+ * @f: Current migration stream.
+ * @pss: Data about the state of the current dirty page scan.
+ * @*again: Set to false if the search has scanned the whole of RAM
+ */
+static bool find_dirty_block(QEMUFile *f, PageSearchStatus *pss,
+                             bool *again)
+{
+    pss->offset = migration_bitmap_find_and_reset_dirty(pss->block,
+                                                       pss->offset);
+    if (pss->complete_round && pss->block == last_seen_block &&
+        pss->offset >= last_offset) {
+        /*
+         * We've been once around the RAM and haven't found anything
+         * give up.
+         */
+        *again = false;
+        return false;
+    }
+    if (pss->offset >= pss->block->used_length) {
+        /* Didn't find anything in this RAM Block */
+        pss->offset = 0;
+        pss->block = QLIST_NEXT_RCU(pss->block, next);
+        if (!pss->block) {
+            /* Hit the end of the list */
+            pss->block = QLIST_FIRST_RCU(&ram_list.blocks);
+            /* Flag that we've looped */
+            pss->complete_round = true;
+            ram_bulk_stage = false;
+            if (migrate_use_xbzrle()) {
+                /* If xbzrle is on, stop using the data compression at this
+                 * point. In theory, xbzrle can do better than compression.
+                 */
+                flush_compressed_data(f);
+                compression_switch = false;
+            }
+        }
+        /* Didn't find anything this time, but try again on the new block */
+        *again = true;
+        return false;
+    } else {
+        /* Can go around again, but... */
+        *again = true;
+        /* We've found something so probably don't need to */
+        return true;
+    }
+}
+
 /**
  * ram_find_and_save_block: Finds a dirty page and sends it to f
  *
@@ -935,6 +988,7 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
 {
     PageSearchStatus pss;
     int pages = 0;
+    bool again, found;
 
     pss.block = last_seen_block;
     pss.offset = last_offset;
@@ -944,29 +998,10 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
         pss.block = QLIST_FIRST_RCU(&ram_list.blocks);
     }
 
-    while (true) {
-        pss.offset = migration_bitmap_find_and_reset_dirty(pss.block,
-                                                           pss.offset);
-        if (pss.complete_round && pss.block == last_seen_block &&
-            pss.offset >= last_offset) {
-            break;
-        }
-        if (pss.offset >= pss.block->used_length) {
-            pss.offset = 0;
-            pss.block = QLIST_NEXT_RCU(pss.block, next);
-            if (!pss.block) {
-                pss.block = QLIST_FIRST_RCU(&ram_list.blocks);
-                pss.complete_round = true;
-                ram_bulk_stage = false;
-                if (migrate_use_xbzrle()) {
-                    /* If xbzrle is on, stop using the data compression at this
-                     * point. In theory, xbzrle can do better than compression.
-                     */
-                    flush_compressed_data(f);
-                    compression_switch = false;
-                }
-            }
-        } else {
+    do {
+        found = find_dirty_block(f, &pss, &again);
+
+        if (found) {
             if (compression_switch && migrate_use_compression()) {
                 pages = ram_save_compressed_page(f, pss.block, pss.offset,
                                                  last_stage,
@@ -979,10 +1014,9 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
             /* if page is unmodified, continue to the next */
             if (pages > 0) {
                 last_sent_block = pss.block;
-                break;
             }
         }
-    }
+    } while (!pages && again);
 
     last_seen_block = pss.block;
     last_offset = pss.offset;
-- 
2.5.0

Re: [Qemu-devel] [PATCH v2 0/2] Split up ram_find_and_save_block

[Amit Shah]

On (Wed) 23 Sep 2015 [15:27:09], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Chop up ram_find_and_save_block so it's smaller again.
> (from comments on my postcopy patch that adds more to it).
> 
> This pair is based on top of my previous 5 patch cleanup series posted
> in August, but rebased on current qemu master.

Reviewed-by: Amit Shah <amit.shah@redhat.com>

		Amit

Re: [Qemu-devel] [PATCH v2 2/2] ram_find_and_save_block: Split out the finding

[Amit Shah]

On (Wed) 23 Sep 2015 [15:27:11], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Split out the finding of the dirty page and all the wrap detection
> into a separate function since it was getting a bit hairy.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> +    if (pss->complete_round && pss->block == last_seen_block &&
> +        pss->offset >= last_offset) {
> +        /*
> +         * We've been once around the RAM and haven't found anything
> +         * give up.

I didn't notice this first time around - but I can fix this up while
merging.  Give up should be another sentence, right?

		Amit