Patchwork [02/41] fix migration sync

login
register
mail settings
Submitter Paolo Bonzini
Date Sept. 25, 2012, 8:45 a.m.
Message ID <50616F38.1000201@redhat.com>
Download mbox | patch
Permalink /patch/186741/
State New
Headers show

Comments

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?
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

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);
 }