[02/41] fix migration sync

Submitted by Paolo Bonzini on Sept. 25, 2012, 8:45 a.m.

Details

Message ID 50616F38.1000201@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Sept. 25, 2012, 8:45 a.m.
Il 21/09/2012 14:17, Paolo Bonzini ha scritto:
> 
> -    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)) {
> -                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
> -            }
> -        }
> -    }
> -
>      memory_global_dirty_log_start();
> +    memory_global_sync_dirty_bitmap(get_system_memory());

Hmm, perhaps for KVM but probably not for TCG.  But this looks like a bug in
KVM, maybe a fix there would be better?

Comments

Juan Quintela Oct. 2, 2012, 10:43 a.m.
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 21/09/2012 14:17, Paolo Bonzini ha scritto:
>> 
>> -    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)) {
>> -                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
>> -            }
>> -        }
>> -    }
>> -
>>      memory_global_dirty_log_start();
>> +    memory_global_sync_dirty_bitmap(get_system_memory());

With the part of moving it after the memory_global_dirty_log_start() I
agree.

With the other suggestion, I will take another look at it.  The problem
is that vga code can also sync the kvm bitmap, and we want to get that
notifications also.

Later, Juan.
Paolo Bonzini Oct. 2, 2012, 10:53 a.m.
Il 02/10/2012 12:43, Juan Quintela ha scritto:
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 21/09/2012 14:17, Paolo Bonzini ha scritto:
>>>
>>> -    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)) {
>>> -                memory_region_set_dirty(block->mr, addr, TARGET_PAGE_SIZE);
>>> -            }
>>> -        }
>>> -    }
>>> -
>>>      memory_global_dirty_log_start();
>>> +    memory_global_sync_dirty_bitmap(get_system_memory());
> 
> With the part of moving it after the memory_global_dirty_log_start() I
> agree.
> 
> With the other suggestion, I will take another look at it.  The problem
> is that vga code can also sync the kvm bitmap, and we want to get that
> notifications also.

I think I had replied to myself later... in any case, nothing that
cannot be improved after merging.  Thanks!

Paolo

Patch hide | download patch | download mbox

diff --git a/kvm-all.c b/kvm-all.c
index 78e7a88..fdab4f6 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -744,6 +746,8 @@  static void kvm_log_global_start(struct MemoryListener *listener)
 {
     int r;
 
+    /* Clear the KVM bitmap.  */
+    kvm_physical_sync_dirty_bitmap(section);
     r = kvm_set_migration_log(1);
     assert(r >= 0);
 }

On the other hand, you're doing useless work syncing the KVM bitmap to the
TCG one---even though the TCG bitmap is all-ones!  Something like this ought
to be faster, but I'm not sure whether it is conceptually right:

diff --git a/kvm-all.c b/kvm-all.c
index 78e7a88..fdab4f6 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -399,7 +399,7 @@  static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
  * @start_add: start of logged region.
  * @end_addr: end of logged region.
  */
-static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
+static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section, bool sync)
 {
     KVMState *s = kvm_state;
     unsigned long size, allocated_size = 0;
@@ -446,7 +446,9 @@  static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
             break;
         }
 
-        kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
+        if (sync) {
+            kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
+        }
         start_addr = mem->start_addr + mem->memory_size;
     }
     g_free(d.dirty_bitmap);
@@ -600,7 +602,7 @@  static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
         old = *mem;
 
         if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
-            kvm_physical_sync_dirty_bitmap(section);
+            kvm_physical_sync_dirty_bitmap(section, true);
         }
 
         /* unregister the overlapping slot */
@@ -733,7 +735,7 @@  static void kvm_log_sync(MemoryListener *listener,
 {
     int r;
 
-    r = kvm_physical_sync_dirty_bitmap(section);
+    r = kvm_physical_sync_dirty_bitmap(section, true);
     if (r < 0) {
         abort();
     }
@@ -744,6 +746,8 @@  static void kvm_log_global_start(struct MemoryListener *listener)
 {
     int r;
 
+    /* Clear the KVM bitmap.  */
+    kvm_physical_sync_dirty_bitmap(section, false);
     r = kvm_set_migration_log(1);
     assert(r >= 0);
 }