diff mbox

[10/27] Separate migration bitmap

Message ID 1343155012-26316-11-git-send-email-quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela July 24, 2012, 6:36 p.m. UTC 
This patch creates a migration bitmap, which is periodically kept in
sync with the qemu bitmap. A separate copy of the dirty bitmap for the
migration limits the amount of concurrent access to the qemu bitmap
from iothread and migration thread (which requires taking the big
lock).

We use the qemu bitmap type.  We have to "undo" the dirty_pages
counting optimization on the general dirty bitmap and do the counting
optimization with the migration local bitmap.

Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c     |   57 +++++++++++++++++++++++++++++++++++++++----------------
 cpu-all.h       |    1 -
 exec-obsolete.h |   10 ----------
 3 files changed, 41 insertions(+), 27 deletions(-)

Comments

Avi Kivity July 25, 2012, 9:16 a.m. UTC | #1 
On 07/24/2012 09:36 PM, Juan Quintela wrote:
> This patch creates a migration bitmap, which is periodically kept in
> sync with the qemu bitmap. A separate copy of the dirty bitmap for the
> migration limits the amount of concurrent access to the qemu bitmap
> from iothread and migration thread (which requires taking the big
> lock).
> 
> We use the qemu bitmap type.  We have to "undo" the dirty_pages
> counting optimization on the general dirty bitmap and do the counting
> optimization with the migration local bitmap.

I had different plans for this (and a stale and possibly smelly patchset
moving in that direction):

- move the dirty bytemap from a single global instance to
per-memory-region instances (in the MemoryRegion structure)
- convert it from a bytemap to either a per-client bitmap (client =
VGA/CODE/MIGRATION) or a variable bit-length bitfieldmap
- allocate the bitmaps or strangely named thingie above on demand, so
ordinarily you have nothing allocated and the framebuffer has a bitmap,
when you start migration you allocate a bitmap for memory and a
twobitmap for the framebuffer
- protect the whole thing using rcu

The patchset is stalled, mostly because it's very difficult to
disentangle the tcg stuff.  I don't think we should introduce a
dependency here, just something to keep in mind.
Juan Quintela July 26, 2012, 9:22 a.m. UTC | #2 
Avi Kivity <avi@redhat.com> wrote:
> On 07/24/2012 09:36 PM, Juan Quintela wrote:
>> This patch creates a migration bitmap, which is periodically kept in
>> sync with the qemu bitmap. A separate copy of the dirty bitmap for the
>> migration limits the amount of concurrent access to the qemu bitmap
>> from iothread and migration thread (which requires taking the big
>> lock).
>> 
>> We use the qemu bitmap type.  We have to "undo" the dirty_pages
>> counting optimization on the general dirty bitmap and do the counting
>> optimization with the migration local bitmap.
>
> I had different plans for this (and a stale and possibly smelly patchset
> moving in that direction):
>
> - move the dirty bytemap from a single global instance to
> per-memory-region instances (in the MemoryRegion structure)
> - convert it from a bytemap to either a per-client bitmap (client =
> VGA/CODE/MIGRATION) or a variable bit-length bitfieldmap
> - allocate the bitmaps or strangely named thingie above on demand, so
> ordinarily you have nothing allocated and the framebuffer has a bitmap,
> when you start migration you allocate a bitmap for memory and a
> twobitmap for the framebuffer
> - protect the whole thing using rcu
>
> The patchset is stalled, mostly because it's very difficult to
> disentangle the tcg stuff.  I don't think we should introduce a
> dependency here, just something to keep in mind.

I tried something like that myself (before your memory regions work).
And got stuck on the same problem:
- for migration, I always had a ramblock handy
- for vga the same (well, I am not sure for the sparc ones, I think they
  were weird on that respect)
- for TCG, there is none around that I can find.  I guess it is there
  somewhere, but not in any obvious part.

So, to fix TCG, we need a TCG guru, and probably change the access
parters somehow :p

Later, Juan.
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index d21b2a3..72345c3 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -31,6 +31,8 @@ 
 #include "config.h"
 #include "monitor.h"
 #include "sysemu.h"
+#include "bitops.h"
+#include "bitmap.h"
 #include "arch_init.h"
 #include "audio/audio.h"
 #include "hw/pc.h"
@@ -341,35 +343,54 @@  static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
 static RAMBlock *last_block;
 static ram_addr_t last_offset;
 static uint32_t last_version;
+static unsigned long *migration_bitmap;
+static uint64_t migration_dirty_pages;

 static inline bool migration_bitmap_test_and_reset_dirty(MemoryRegion *mr,
                                                          ram_addr_t offset)
 {
-    bool ret = memory_region_get_dirty(mr, offset, TARGET_PAGE_SIZE,
-                                       DIRTY_MEMORY_MIGRATION);
+    bool ret;
+    int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;
+
+    ret = test_and_clear_bit(nr, migration_bitmap);

     if (ret) {
-        memory_region_reset_dirty(mr, offset, TARGET_PAGE_SIZE,
-                                  DIRTY_MEMORY_MIGRATION);
+        migration_dirty_pages--;
     }
     return ret;
 }

-static inline void migration_bitmap_set_dirty(MemoryRegion *mr, int length)
+static inline bool migration_bitmap_set_dirty(MemoryRegion *mr,
+                                              ram_addr_t offset)
 {
-    ram_addr_t addr;
+    bool ret;
+    int nr = (mr->ram_addr + offset) >> TARGET_PAGE_BITS;

-    for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
-        if (!memory_region_get_dirty(mr, addr, TARGET_PAGE_SIZE,
-                                     DIRTY_MEMORY_MIGRATION)) {
-            memory_region_set_dirty(mr, addr, TARGET_PAGE_SIZE);
-        }
+    ret = test_and_set_bit(nr, migration_bitmap);
+
+    if (!ret) {
+        migration_dirty_pages++;
     }
+    return ret;
 }

 static void migration_bitmap_sync(void)
 {
+    RAMBlock *block;
+    ram_addr_t addr;
+
     memory_global_sync_dirty_bitmap(get_system_memory());
+
+    QLIST_FOREACH(block, &ram_list.blocks, next) {
+        for (addr = 0; addr < block->length; addr += TARGET_PAGE_SIZE) {
+            if (memory_region_get_dirty(block->mr, addr, TARGET_PAGE_SIZE,
+                                        DIRTY_MEMORY_MIGRATION)) {
+                migration_bitmap_set_dirty(block->mr, addr);
+            }
+        }
+        memory_region_reset_dirty(block->mr, 0, block->length,
+                                  DIRTY_MEMORY_MIGRATION);
+    }
 }


@@ -447,7 +468,7 @@  static uint64_t bytes_transferred;

 static ram_addr_t ram_save_remaining(void)
 {
-    return ram_list.dirty_pages;
+    return migration_dirty_pages;
 }

 uint64_t ram_bytes_remaining(void)
@@ -532,6 +553,11 @@  static void reset_ram_globals(void)
 static int ram_save_setup(QEMUFile *f, void *opaque)
 {
     RAMBlock *block;
+    int64_t ram_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+
+    migration_bitmap = bitmap_new(ram_pages);
+    bitmap_set(migration_bitmap, 0, ram_pages);
+    migration_dirty_pages = ram_pages;

     qemu_mutex_lock_ramlist();

@@ -551,10 +577,6 @@  static int ram_save_setup(QEMUFile *f, void *opaque)
         acct_clear();
     }

-    QLIST_FOREACH(block, &ram_list.blocks, next) {
-        migration_bitmap_set_dirty(block->mr, block->length);
-    }
-
     memory_global_dirty_log_start();

     qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
@@ -669,6 +691,9 @@  static int ram_save_complete(QEMUFile *f, void *opaque)
     qemu_mutex_unlock_ramlist();
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);

+    g_free(migration_bitmap);
+    migration_bitmap = NULL;
+
     return 0;
 }

diff --git a/cpu-all.h b/cpu-all.h
index 45290b7..88ad1ba 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -495,7 +495,6 @@  typedef struct RAMList {
     QLIST_HEAD(, RAMBlock) blocks_mru;
     /* Protected by the ramlist lock.  */
     QLIST_HEAD(, RAMBlock) blocks;
-    uint64_t dirty_pages;
 } RAMList;
 extern RAMList ram_list;

diff --git a/exec-obsolete.h b/exec-obsolete.h
index c099256..f8ffce6 100644
--- a/exec-obsolete.h
+++ b/exec-obsolete.h
@@ -74,11 +74,6 @@  static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
 static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
                                                       int dirty_flags)
 {
-    if ((dirty_flags & MIGRATION_DIRTY_FLAG) &&
-        !cpu_physical_memory_get_dirty(addr, TARGET_PAGE_SIZE,
-                                       MIGRATION_DIRTY_FLAG)) {
-        ram_list.dirty_pages++;
-    }
     return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
 }

@@ -92,11 +87,6 @@  static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr,
 {
     int mask = ~dirty_flags;

-    if ((dirty_flags & MIGRATION_DIRTY_FLAG) &&
-        cpu_physical_memory_get_dirty(addr, TARGET_PAGE_SIZE,
-                                      MIGRATION_DIRTY_FLAG)) {
-        ram_list.dirty_pages--;
-    }
     return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] &= mask;
 }