[Qemu-devel] [PATCH] migration: use call_rcu instead of synchronize_rcu

[Li Zhijian]

from commit 2ff6403, we make a mistake to call synchronize_rcu()
within rcu_read_lock()/rcu_read_unlock()

Signed-off-by: Li Zhijian <address@hidden>
Signed-off-by: Wen Congyang <address@hidden>
Signed-off-by: Li Zhijian <address@hidden>
---
 migration/ram.c | 49 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index c696814..51635e9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -221,7 +221,11 @@ static RAMBlock *last_seen_block;
 /* This is the last block from where we have sent data */
 static RAMBlock *last_sent_block;
 static ram_addr_t last_offset;
-static unsigned long *migration_bitmap;
+struct rcu_bitmap {
+    struct rcu_head rcu;
+    unsigned long *bitmap;
+};
+static struct rcu_bitmap *migration_bitmap;
 static QemuMutex migration_bitmap_mutex;
 static uint64_t migration_dirty_pages;
 static uint32_t last_version;
@@ -504,7 +508,7 @@ ram_addr_t 
migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
     unsigned long nr = base + (start >> TARGET_PAGE_BITS);
     uint64_t mr_size = TARGET_PAGE_ALIGN(memory_region_size(mr));
     unsigned long size = base + (mr_size >> TARGET_PAGE_BITS);
-    unsigned long *bitmap;
+    struct rcu_bitmap *bitmap;
 
     unsigned long next;
 
@@ -512,11 +516,11 @@ ram_addr_t 
migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
     if (ram_bulk_stage && nr > base) {
         next = nr + 1;
     } else {
-        next = find_next_bit(bitmap, size, nr);
+        next = find_next_bit(bitmap->bitmap, size, nr);
     }
 
     if (next < size) {
-        clear_bit(next, bitmap);
+        clear_bit(next, bitmap->bitmap);
         migration_dirty_pages--;
     }
     return (next - base) << TARGET_PAGE_BITS;
@@ -525,10 +529,10 @@ ram_addr_t 
migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
 /* Called with rcu_read_lock() to protect migration_bitmap */
 static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
 {
-    unsigned long *bitmap;
+    struct rcu_bitmap *bitmap;
     bitmap = atomic_rcu_read(&migration_bitmap);
     migration_dirty_pages +=
-        cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length);
+        cpu_physical_memory_sync_dirty_bitmap(bitmap->bitmap, start, length);
 }
 
 
@@ -1024,17 +1028,29 @@ void free_xbzrle_decoded_buf(void)
     xbzrle_decoded_buf = NULL;
 }
 
+static struct rcu_bitmap *rcu_bitmap_new(long bits)
+{
+    struct rcu_bitmap *bitmap = g_malloc0(sizeof(*bitmap));
+    bitmap->bitmap = bitmap_new(bits);
+    return bitmap;
+}
+
+static void rcu_bitmap_destroy(struct rcu_bitmap *bitmap)
+{
+    g_free(bitmap->bitmap);
+    g_free(bitmap);
+}
+
 static void migration_end(void)
 {
     /* caller have hold iothread lock or is in a bh, so there is
      * no writing race against this migration_bitmap
      */
-    unsigned long *bitmap = migration_bitmap;
+    struct rcu_bitmap *bitmap = migration_bitmap;
     atomic_rcu_set(&migration_bitmap, NULL);
     if (bitmap) {
         memory_global_dirty_log_stop();
-        synchronize_rcu();
-        g_free(bitmap);
+        call_rcu(bitmap, rcu_bitmap_destroy, rcu);
     }
 
     XBZRLE_cache_lock();
@@ -1071,8 +1087,8 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t 
new)
      * no writing race against this migration_bitmap
      */
     if (migration_bitmap) {
-        unsigned long *old_bitmap = migration_bitmap, *bitmap;
-        bitmap = bitmap_new(new);
+        struct rcu_bitmap *old_bitmap = migration_bitmap, *bitmap;
+        bitmap = rcu_bitmap_new(new);
 
         /* prevent migration_bitmap content from being set bit
          * by migration_bitmap_sync_range() at the same time.
@@ -1080,13 +1096,12 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t 
new)
          * at the same time.
          */
         qemu_mutex_lock(&migration_bitmap_mutex);
-        bitmap_copy(bitmap, old_bitmap, old);
-        bitmap_set(bitmap, old, new - old);
+        bitmap_copy(bitmap->bitmap, old_bitmap->bitmap, old);
+        bitmap_set(bitmap->bitmap, old, new - old);
         atomic_rcu_set(&migration_bitmap, bitmap);
         qemu_mutex_unlock(&migration_bitmap_mutex);
         migration_dirty_pages += new - old;
-        synchronize_rcu();
-        g_free(old_bitmap);
+        call_rcu(old_bitmap, rcu_bitmap_destroy, rcu);
     }
 }
 
@@ -1145,8 +1160,8 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     reset_ram_globals();
 
     ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
-    migration_bitmap = bitmap_new(ram_bitmap_pages);
-    bitmap_set(migration_bitmap, 0, ram_bitmap_pages);
+    migration_bitmap = rcu_bitmap_new(ram_bitmap_pages);
+    bitmap_set(migration_bitmap->bitmap, 0, ram_bitmap_pages);
 
     /*
      * Count the total number of pages used by ram blocks not including any
-- 
2.1.4