* [Qemu-devel] [PATCH v2 1/2] migration: protect migration_bitmap
2015-06-26 10:09 [Qemu-devel] [PATCH v2 0/2] avoid a hotplug operation leading migration's source side abort Li Zhijian
@ 2015-06-26 10:09 ` Li Zhijian
2015-06-26 10:09 ` [Qemu-devel] [PATCH v2 2/2] migration: extend migration_bitmap Li Zhijian
2015-07-01 8:34 ` [Qemu-devel] [PATCH v2 0/2] avoid a hotplug operation leading migration's source side abort Juan Quintela
2 siblings, 0 replies; 5+ messages in thread
From: Li Zhijian @ 2015-06-26 10:09 UTC (permalink / raw)
To: qemu-devel, pbonzini, quintela, amit.shah; +Cc: Li Zhijian
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
migration/ram.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 57368e1..4754aa9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -222,6 +222,7 @@ static RAMBlock *last_seen_block;
static RAMBlock *last_sent_block;
static ram_addr_t last_offset;
static unsigned long *migration_bitmap;
+static QemuMutex migration_bitmap_mutex;
static uint64_t migration_dirty_pages;
static uint32_t last_version;
static bool ram_bulk_stage;
@@ -494,6 +495,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
return 1;
}
+/* Called with rcu_read_lock() to protect migration_bitmap */
static inline
ram_addr_t migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
ram_addr_t start)
@@ -502,26 +504,31 @@ 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;
unsigned long next;
+ bitmap = atomic_rcu_read(&migration_bitmap);
if (ram_bulk_stage && nr > base) {
next = nr + 1;
} else {
- next = find_next_bit(migration_bitmap, size, nr);
+ next = find_next_bit(bitmap, size, nr);
}
if (next < size) {
- clear_bit(next, migration_bitmap);
+ clear_bit(next, bitmap);
migration_dirty_pages--;
}
return (next - base) << TARGET_PAGE_BITS;
}
+/* 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;
+ bitmap = atomic_rcu_read(&migration_bitmap);
migration_dirty_pages +=
- cpu_physical_memory_sync_dirty_bitmap(migration_bitmap, start, length);
+ cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length);
}
@@ -1017,10 +1024,15 @@ void free_xbzrle_decoded_buf(void)
static void migration_end(void)
{
- if (migration_bitmap) {
+ unsigned long *bitmap;
+ qemu_mutex_lock(&migration_bitmap_mutex);
+ bitmap = migration_bitmap;
+ atomic_rcu_set(&migration_bitmap, NULL);
+ qemu_mutex_unlock(&migration_bitmap_mutex);
+ if (bitmap) {
memory_global_dirty_log_stop();
- g_free(migration_bitmap);
- migration_bitmap = NULL;
+ synchronize_rcu();
+ g_free(bitmap);
}
XBZRLE_cache_lock();
@@ -1067,6 +1079,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
dirty_rate_high_cnt = 0;
bitmap_sync_count = 0;
migration_bitmap_sync_init();
+ qemu_mutex_init(&migration_bitmap_mutex);
if (migrate_use_xbzrle()) {
XBZRLE_cache_lock();
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH v2 2/2] migration: extend migration_bitmap
2015-06-26 10:09 [Qemu-devel] [PATCH v2 0/2] avoid a hotplug operation leading migration's source side abort Li Zhijian
2015-06-26 10:09 ` [Qemu-devel] [PATCH v2 1/2] migration: protect migration_bitmap Li Zhijian
@ 2015-06-26 10:09 ` Li Zhijian
2015-07-01 10:27 ` Juan Quintela
2015-07-01 8:34 ` [Qemu-devel] [PATCH v2 0/2] avoid a hotplug operation leading migration's source side abort Juan Quintela
2 siblings, 1 reply; 5+ messages in thread
From: Li Zhijian @ 2015-06-26 10:09 UTC (permalink / raw)
To: qemu-devel, pbonzini, quintela, amit.shah; +Cc: Li Zhijian
Prevously, if we hotplug a device(e.g. device_add e1000) during
migration is processing in source side, qemu will add a new ram
block but migration_bitmap is not extended.
In this case, migration_bitmap will overflow and lead qemu abort
unexpectedly.
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
exec.c | 5 +++++
include/exec/exec-all.h | 1 +
migration/ram.c | 15 +++++++++++++++
3 files changed, 21 insertions(+)
diff --git a/exec.c b/exec.c
index f7883d2..1702544 100644
--- a/exec.c
+++ b/exec.c
@@ -1401,6 +1401,11 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
}
}
+ new_ram_size = MAX(old_ram_size,
+ (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS);
+ if (new_ram_size > old_ram_size) {
+ migration_bitmap_extend(old_ram_size, new_ram_size);
+ }
/* Keep the list sorted from biggest to smallest block. Unlike QTAILQ,
* QLIST (which has an RCU-friendly variant) does not have insertion at
* tail, so save the last element in last_block.
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 2573e8c..dd9be44 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -385,4 +385,5 @@ static inline bool cpu_can_do_io(CPUState *cpu)
return cpu->can_do_io != 0;
}
+void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
#endif
diff --git a/migration/ram.c b/migration/ram.c
index 4754aa9..3e16ef8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1063,6 +1063,21 @@ static void reset_ram_globals(void)
#define MAX_WAIT 50 /* ms, half buffered_file limit */
+void migration_bitmap_extend(ram_addr_t old, ram_addr_t new)
+{
+ qemu_mutex_lock(&migration_bitmap_mutex);
+ if (migration_bitmap) {
+ unsigned long *old_bitmap = migration_bitmap, *bitmap;
+ bitmap = bitmap_new(new);
+ bitmap_copy(bitmap, old_bitmap, old);
+ bitmap_set(bitmap, old, new - old);
+ atomic_rcu_set(&migration_bitmap, bitmap);
+ migration_dirty_pages += new - old;
+ synchronize_rcu();
+ g_free(old_bitmap);
+ }
+ qemu_mutex_unlock(&migration_bitmap_mutex);
+}
/* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
* long-running RCU critical section. When rcu-reclaims in the code
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/2] migration: extend migration_bitmap
2015-06-26 10:09 ` [Qemu-devel] [PATCH v2 2/2] migration: extend migration_bitmap Li Zhijian
@ 2015-07-01 10:27 ` Juan Quintela
0 siblings, 0 replies; 5+ messages in thread
From: Juan Quintela @ 2015-07-01 10:27 UTC (permalink / raw)
To: Li Zhijian; +Cc: amit.shah, pbonzini, qemu-devel
Li Zhijian <lizhijian@cn.fujitsu.com> wrote:
> Prevously, if we hotplug a device(e.g. device_add e1000) during
> migration is processing in source side, qemu will add a new ram
> block but migration_bitmap is not extended.
> In this case, migration_bitmap will overflow and lead qemu abort
> unexpectedly.
>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> exec.c | 5 +++++
> include/exec/exec-all.h | 1 +
> migration/ram.c | 15 +++++++++++++++
> 3 files changed, 21 insertions(+)
>
> diff --git a/exec.c b/exec.c
> index f7883d2..1702544 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1401,6 +1401,11 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
> }
> }
>
> + new_ram_size = MAX(old_ram_size,
> + (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS);
> + if (new_ram_size > old_ram_size) {
> + migration_bitmap_extend(old_ram_size, new_ram_size);
> + }
> /* Keep the list sorted from biggest to smallest block. Unlike QTAILQ,
> * QLIST (which has an RCU-friendly variant) does not have insertion at
> * tail, so save the last element in last_block.
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 2573e8c..dd9be44 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -385,4 +385,5 @@ static inline bool cpu_can_do_io(CPUState *cpu)
> return cpu->can_do_io != 0;
> }
>
> +void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
Had to protect this one with
#if !defined(CONFIG_USER_ONLY)
void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
#endif
> #endif
> diff --git a/migration/ram.c b/migration/ram.c
> index 4754aa9..3e16ef8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1063,6 +1063,21 @@ static void reset_ram_globals(void)
>
> #define MAX_WAIT 50 /* ms, half buffered_file limit */
>
> +void migration_bitmap_extend(ram_addr_t old, ram_addr_t new)
> +{
> + qemu_mutex_lock(&migration_bitmap_mutex);
> + if (migration_bitmap) {
> + unsigned long *old_bitmap = migration_bitmap, *bitmap;
> + bitmap = bitmap_new(new);
> + bitmap_copy(bitmap, old_bitmap, old);
> + bitmap_set(bitmap, old, new - old);
> + atomic_rcu_set(&migration_bitmap, bitmap);
> + migration_dirty_pages += new - old;
> + synchronize_rcu();
> + g_free(old_bitmap);
> + }
> + qemu_mutex_unlock(&migration_bitmap_mutex);
> +}
>
> /* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> * long-running RCU critical section. When rcu-reclaims in the code
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] avoid a hotplug operation leading migration's source side abort
2015-06-26 10:09 [Qemu-devel] [PATCH v2 0/2] avoid a hotplug operation leading migration's source side abort Li Zhijian
2015-06-26 10:09 ` [Qemu-devel] [PATCH v2 1/2] migration: protect migration_bitmap Li Zhijian
2015-06-26 10:09 ` [Qemu-devel] [PATCH v2 2/2] migration: extend migration_bitmap Li Zhijian
@ 2015-07-01 8:34 ` Juan Quintela
2 siblings, 0 replies; 5+ messages in thread
From: Juan Quintela @ 2015-07-01 8:34 UTC (permalink / raw)
To: Li Zhijian; +Cc: amit.shah, pbonzini, qemu-devel
Li Zhijian <lizhijian@cn.fujitsu.com> wrote:
> qemu migration's source side will exit unexpectedly when we hotplug a deivce
> during a migration is processing.
> we can reproduced it easily by following step
> 1. do something with dirty memory requently(like memtester) in guest
> 2. startup a background migration with '-d' option
> 3. hotplug a device(device_add e1000,id=mye1000)
> 4. stop step.1, let guest idle so that migration can complete fastly
Applied, thanks.
>
> something unexpectedly occurs like below:
> *** Error in `/home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64': free(): invalid pointer: 0x00007fff5c010b20 ***
> ======= Backtrace: =========
> /lib64/libc.so.6(+0x7d1fd)[0x7ffff5ad41fd]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x1e29c2)[0x5555557369c2]
> /lib64/libglib-2.0.so.0(g_free+0xf)[0x7ffff6aaa5af]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x139454)[0x55555568d454]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x13a232)[0x55555568e232]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x13a2f1)[0x55555568e2f1]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0xec914)[0x555555640914]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0xc7e7e)[0x55555561be7e]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0xc7f0f)[0x55555561bf0f]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0xf01c9)[0x5555556441c9]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3541d4)[0x5555558a81d4]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3a5cf6)[0x5555558f9cf6]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3b5809)[0x555555909809]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3a6067)[0x5555558fa067]
> /lib64/libglib-2.0.so.0(g_main_context_dispatch+0x15a)[0x7ffff6aa49ba]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3b3c6f)[0x555555907c6f]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3b3d4c)[0x555555907d4c]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x3b3e0b)[0x555555907e0b]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x1df701)[0x555555733701]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x1e6fed)[0x55555573afed]
> /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ffff5a78af5]
> /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64(+0x93729)[0x5555555e7729]
> ======= Memory map: ========
> 555555554000-555555b04000 r-xp 00000000 08:04 14111744 /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64
> 555555d03000-555555dcc000 r--p 005af000 08:04 14111744 /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64
> 555555dcc000-555555e42000 rw-p 00678000 08:04 14111744 /home/lizj/workspace/qemu/x86_64-softmmu/qemu-system-x86_64
> 555555e42000-55555affc000 rw-p 00000000 00:00 0 [heap]
> snip...
>
> Changes from v1
> do bitmap_set after bitmap_copy
>
> Li Zhijian (2):
> migration: protect migration_bitmap
> migration: extend migration_bitmap
>
> exec.c | 5 +++++
> include/exec/exec-all.h | 1 +
> migration/ram.c | 40 ++++++++++++++++++++++++++++++++++------
> 3 files changed, 40 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread