* [RFC PATCH v4 0/5] Separate thread for VM migration
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
Following patch series deals with VCPU and iothread starvation during the
migration of a guest. Currently the iothread is responsible for performing the
guest migration. It holds qemu_mutex during the migration and doesn't allow VCPU
to enter the qemu mode and delays its return to the guest. The guest migration,
executed as an iohandler also delays the execution of other iohandlers.
In the following patch series,
The migration has been moved to a separate thread to
reduce the qemu_mutex contention and iohandler starvation.
Umesh Deshpande (5):
MRU ram list
ramlist lock
separate migration bitmap
separate thread for VM migration
synchronous migrate_cancel
arch_init.c | 26 +++++++++---
buffered_file.c | 104 ++++++++++++++++++++++++++++++++++-----------------
buffered_file.h | 3 +
cpu-all.h | 41 ++++++++++++++++++++
exec.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++--
hw/hw.h | 5 ++-
migration.c | 78 ++++++++++++++++++++------------------
migration.h | 1 +
qemu-common.h | 2 +
qemu-thread-posix.c | 10 +++++
qemu-thread.h | 1 +
savevm.c | 30 +++++++++------
12 files changed, 304 insertions(+), 97 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH v4 0/5] Separate thread for VM migration
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
Following patch series deals with VCPU and iothread starvation during the
migration of a guest. Currently the iothread is responsible for performing the
guest migration. It holds qemu_mutex during the migration and doesn't allow VCPU
to enter the qemu mode and delays its return to the guest. The guest migration,
executed as an iohandler also delays the execution of other iohandlers.
In the following patch series,
The migration has been moved to a separate thread to
reduce the qemu_mutex contention and iohandler starvation.
Umesh Deshpande (5):
MRU ram list
ramlist lock
separate migration bitmap
separate thread for VM migration
synchronous migrate_cancel
arch_init.c | 26 +++++++++---
buffered_file.c | 104 ++++++++++++++++++++++++++++++++++-----------------
buffered_file.h | 3 +
cpu-all.h | 41 ++++++++++++++++++++
exec.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++--
hw/hw.h | 5 ++-
migration.c | 78 ++++++++++++++++++++------------------
migration.h | 1 +
qemu-common.h | 2 +
qemu-thread-posix.c | 10 +++++
qemu-thread.h | 1 +
savevm.c | 30 +++++++++------
12 files changed, 304 insertions(+), 97 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC PATCH v4 1/5] MRU ram list
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 3:56 ` Umesh Deshpande
-1 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, quintela, mtosatti, Umesh Deshpande
This patch creates a new list of RAM blocks in MRU order. So that separate
locking rules can be applied to the regular RAM block list and the MRU list.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpu-all.h | 2 ++
exec.c | 17 ++++++++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index e839100..6b217a2 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -925,6 +925,7 @@ typedef struct RAMBlock {
uint32_t flags;
char idstr[256];
QLIST_ENTRY(RAMBlock) next;
+ QLIST_ENTRY(RAMBlock) next_mru;
#if defined(__linux__) && !defined(TARGET_S390X)
int fd;
#endif
@@ -933,6 +934,7 @@ typedef struct RAMBlock {
typedef struct RAMList {
uint8_t *phys_dirty;
QLIST_HEAD(ram, RAMBlock) blocks;
+ QLIST_HEAD(, RAMBlock) blocks_mru;
} RAMList;
extern RAMList ram_list;
diff --git a/exec.c b/exec.c
index 0e2ce57..c5c247c 100644
--- a/exec.c
+++ b/exec.c
@@ -113,7 +113,11 @@ static uint8_t *code_gen_ptr;
int phys_ram_fd;
static int in_migration;
-RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list) };
+RAMList ram_list = {
+ .blocks = QLIST_HEAD_INITIALIZER(ram_list),
+ .blocks_mru = QLIST_HEAD_INITIALIZER(ram_list.blocks_mru)
+};
+
#endif
CPUState *first_cpu;
@@ -2973,6 +2977,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
new_block->length = size;
QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
+ QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
last_ram_offset() >> TARGET_PAGE_BITS);
@@ -2997,6 +3002,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
+ QLIST_REMOVE(block, next_mru);
qemu_free(block);
return;
}
@@ -3010,6 +3016,7 @@ void qemu_ram_free(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
+ QLIST_REMOVE(block, next_mru);
if (block->flags & RAM_PREALLOC_MASK) {
;
} else if (mem_path) {
@@ -3113,12 +3120,12 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
{
RAMBlock *block;
- QLIST_FOREACH(block, &ram_list.blocks, next) {
+ QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
if (addr - block->offset < block->length) {
/* Move this entry to to start of the list. */
if (block != QLIST_FIRST(&ram_list.blocks)) {
- QLIST_REMOVE(block, next);
- QLIST_INSERT_HEAD(&ram_list.blocks, block, next);
+ QLIST_REMOVE(block, next_mru);
+ QLIST_INSERT_HEAD(&ram_list.blocks_mru, block, next_mru);
}
if (xen_mapcache_enabled()) {
/* We need to check if the requested address is in the RAM
@@ -3211,7 +3218,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
return 0;
}
- QLIST_FOREACH(block, &ram_list.blocks, next) {
+ QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
/* This case append when the block is not mapped. */
if (block->host == NULL) {
continue;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH v4 1/5] MRU ram list
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
This patch creates a new list of RAM blocks in MRU order. So that separate
locking rules can be applied to the regular RAM block list and the MRU list.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpu-all.h | 2 ++
exec.c | 17 ++++++++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index e839100..6b217a2 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -925,6 +925,7 @@ typedef struct RAMBlock {
uint32_t flags;
char idstr[256];
QLIST_ENTRY(RAMBlock) next;
+ QLIST_ENTRY(RAMBlock) next_mru;
#if defined(__linux__) && !defined(TARGET_S390X)
int fd;
#endif
@@ -933,6 +934,7 @@ typedef struct RAMBlock {
typedef struct RAMList {
uint8_t *phys_dirty;
QLIST_HEAD(ram, RAMBlock) blocks;
+ QLIST_HEAD(, RAMBlock) blocks_mru;
} RAMList;
extern RAMList ram_list;
diff --git a/exec.c b/exec.c
index 0e2ce57..c5c247c 100644
--- a/exec.c
+++ b/exec.c
@@ -113,7 +113,11 @@ static uint8_t *code_gen_ptr;
int phys_ram_fd;
static int in_migration;
-RAMList ram_list = { .blocks = QLIST_HEAD_INITIALIZER(ram_list) };
+RAMList ram_list = {
+ .blocks = QLIST_HEAD_INITIALIZER(ram_list),
+ .blocks_mru = QLIST_HEAD_INITIALIZER(ram_list.blocks_mru)
+};
+
#endif
CPUState *first_cpu;
@@ -2973,6 +2977,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
new_block->length = size;
QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
+ QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
ram_list.phys_dirty = qemu_realloc(ram_list.phys_dirty,
last_ram_offset() >> TARGET_PAGE_BITS);
@@ -2997,6 +3002,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
+ QLIST_REMOVE(block, next_mru);
qemu_free(block);
return;
}
@@ -3010,6 +3016,7 @@ void qemu_ram_free(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
QLIST_REMOVE(block, next);
+ QLIST_REMOVE(block, next_mru);
if (block->flags & RAM_PREALLOC_MASK) {
;
} else if (mem_path) {
@@ -3113,12 +3120,12 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
{
RAMBlock *block;
- QLIST_FOREACH(block, &ram_list.blocks, next) {
+ QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
if (addr - block->offset < block->length) {
/* Move this entry to to start of the list. */
if (block != QLIST_FIRST(&ram_list.blocks)) {
- QLIST_REMOVE(block, next);
- QLIST_INSERT_HEAD(&ram_list.blocks, block, next);
+ QLIST_REMOVE(block, next_mru);
+ QLIST_INSERT_HEAD(&ram_list.blocks_mru, block, next_mru);
}
if (xen_mapcache_enabled()) {
/* We need to check if the requested address is in the RAM
@@ -3211,7 +3218,7 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
return 0;
}
- QLIST_FOREACH(block, &ram_list.blocks, next) {
+ QLIST_FOREACH(block, &ram_list.blocks_mru, next_mru) {
/* This case append when the block is not mapped. */
if (block->host == NULL) {
continue;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v4 2/5] ramlist mutex
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 3:56 ` Umesh Deshpande
-1 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, quintela, mtosatti, Umesh Deshpande
ramlist mutex is implemented to protect the RAMBlock list traversal in the
migration thread from their addition/removal from the iothread.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
cpu-all.h | 2 ++
exec.c | 19 +++++++++++++++++++
qemu-common.h | 2 ++
3 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index 6b217a2..eab9803 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "cpu-common.h"
+#include "qemu-thread.h"
/* some important defines:
*
@@ -932,6 +933,7 @@ typedef struct RAMBlock {
} RAMBlock;
typedef struct RAMList {
+ QemuMutex mutex;
uint8_t *phys_dirty;
QLIST_HEAD(ram, RAMBlock) blocks;
QLIST_HEAD(, RAMBlock) blocks_mru;
diff --git a/exec.c b/exec.c
index c5c247c..404d8ea 100644
--- a/exec.c
+++ b/exec.c
@@ -582,6 +582,7 @@ void cpu_exec_init_all(unsigned long tb_size)
code_gen_alloc(tb_size);
code_gen_ptr = code_gen_buffer;
page_init();
+ qemu_mutex_init(&ram_list.mutex);
#if !defined(CONFIG_USER_ONLY)
io_mem_init();
#endif
@@ -2802,6 +2803,16 @@ static long gethugepagesize(const char *path)
return fs.f_bsize;
}
+void qemu_mutex_lock_ramlist(void)
+{
+ qemu_mutex_lock(&ram_list.mutex);
+}
+
+void qemu_mutex_unlock_ramlist(void)
+{
+ qemu_mutex_unlock(&ram_list.mutex);
+}
+
static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
const char *path)
@@ -2976,6 +2987,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
}
new_block->length = size;
+ qemu_mutex_lock_ramlist();
+
QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
@@ -2984,6 +2997,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
0xff, size >> TARGET_PAGE_BITS);
+ qemu_mutex_unlock_ramlist();
+
if (kvm_enabled())
kvm_setup_guest_memory(new_block->host, size);
@@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
+ qemu_mutex_lock_ramlist();
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
+ qemu_mutex_unlock_ramlist();
qemu_free(block);
return;
}
@@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
+ qemu_mutex_lock_ramlist();
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
+ qemu_mutex_unlock_ramlist();
if (block->flags & RAM_PREALLOC_MASK) {
;
} else if (mem_path) {
diff --git a/qemu-common.h b/qemu-common.h
index abd7a75..b802883 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -212,6 +212,8 @@ char *qemu_strndup(const char *str, size_t size);
void qemu_mutex_lock_iothread(void);
void qemu_mutex_unlock_iothread(void);
+void qemu_mutex_lock_ramlist(void);
+void qemu_mutex_unlock_ramlist(void);
int qemu_open(const char *name, int flags, ...);
ssize_t qemu_write_full(int fd, const void *buf, size_t count)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
ramlist mutex is implemented to protect the RAMBlock list traversal in the
migration thread from their addition/removal from the iothread.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
cpu-all.h | 2 ++
exec.c | 19 +++++++++++++++++++
qemu-common.h | 2 ++
3 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index 6b217a2..eab9803 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "cpu-common.h"
+#include "qemu-thread.h"
/* some important defines:
*
@@ -932,6 +933,7 @@ typedef struct RAMBlock {
} RAMBlock;
typedef struct RAMList {
+ QemuMutex mutex;
uint8_t *phys_dirty;
QLIST_HEAD(ram, RAMBlock) blocks;
QLIST_HEAD(, RAMBlock) blocks_mru;
diff --git a/exec.c b/exec.c
index c5c247c..404d8ea 100644
--- a/exec.c
+++ b/exec.c
@@ -582,6 +582,7 @@ void cpu_exec_init_all(unsigned long tb_size)
code_gen_alloc(tb_size);
code_gen_ptr = code_gen_buffer;
page_init();
+ qemu_mutex_init(&ram_list.mutex);
#if !defined(CONFIG_USER_ONLY)
io_mem_init();
#endif
@@ -2802,6 +2803,16 @@ static long gethugepagesize(const char *path)
return fs.f_bsize;
}
+void qemu_mutex_lock_ramlist(void)
+{
+ qemu_mutex_lock(&ram_list.mutex);
+}
+
+void qemu_mutex_unlock_ramlist(void)
+{
+ qemu_mutex_unlock(&ram_list.mutex);
+}
+
static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
const char *path)
@@ -2976,6 +2987,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
}
new_block->length = size;
+ qemu_mutex_lock_ramlist();
+
QLIST_INSERT_HEAD(&ram_list.blocks, new_block, next);
QLIST_INSERT_HEAD(&ram_list.blocks_mru, new_block, next_mru);
@@ -2984,6 +2997,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
0xff, size >> TARGET_PAGE_BITS);
+ qemu_mutex_unlock_ramlist();
+
if (kvm_enabled())
kvm_setup_guest_memory(new_block->host, size);
@@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
+ qemu_mutex_lock_ramlist();
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
+ qemu_mutex_unlock_ramlist();
qemu_free(block);
return;
}
@@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
QLIST_FOREACH(block, &ram_list.blocks, next) {
if (addr == block->offset) {
+ qemu_mutex_lock_ramlist();
QLIST_REMOVE(block, next);
QLIST_REMOVE(block, next_mru);
+ qemu_mutex_unlock_ramlist();
if (block->flags & RAM_PREALLOC_MASK) {
;
} else if (mem_path) {
diff --git a/qemu-common.h b/qemu-common.h
index abd7a75..b802883 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -212,6 +212,8 @@ char *qemu_strndup(const char *str, size_t size);
void qemu_mutex_lock_iothread(void);
void qemu_mutex_unlock_iothread(void);
+void qemu_mutex_lock_ramlist(void);
+void qemu_mutex_unlock_ramlist(void);
int qemu_open(const char *name, int flags, ...);
ssize_t qemu_write_full(int fd, const void *buf, size_t count)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v4 3/5] separate migration bitmap
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 3:56 ` Umesh Deshpande
-1 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
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 avoids
concurrent access to the qemu bitmap from iothread and migration thread.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
arch_init.c | 26 +++++++++++++++++------
cpu-all.h | 37 ++++++++++++++++++++++++++++++++++
exec.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 120 insertions(+), 7 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 484b39d..296b7d6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -123,13 +123,13 @@ static int ram_save_block(QEMUFile *f)
current_addr = block->offset + offset;
do {
- if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
+ if (migration_bitmap_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
uint8_t *p;
int cont = (block == last_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
- cpu_physical_memory_reset_dirty(current_addr,
- current_addr + TARGET_PAGE_SIZE,
- MIGRATION_DIRTY_FLAG);
+ migration_bitmap_reset_dirty(current_addr,
+ current_addr + TARGET_PAGE_SIZE,
+ MIGRATION_DIRTY_FLAG);
p = block->host + offset;
@@ -185,7 +185,7 @@ static ram_addr_t ram_save_remaining(void)
ram_addr_t addr;
for (addr = block->offset; addr < block->offset + block->length;
addr += TARGET_PAGE_SIZE) {
- if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
+ if (migration_bitmap_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
count++;
}
}
@@ -265,6 +265,8 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
return 0;
}
+ sync_migration_bitmap(0, TARGET_PHYS_ADDR_MAX);
+
if (stage == 1) {
RAMBlock *block;
bytes_transferred = 0;
@@ -276,9 +278,9 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
QLIST_FOREACH(block, &ram_list.blocks, next) {
for (addr = block->offset; addr < block->offset + block->length;
addr += TARGET_PAGE_SIZE) {
- if (!cpu_physical_memory_get_dirty(addr,
+ if (!migration_bitmap_get_dirty(addr,
MIGRATION_DIRTY_FLAG)) {
- cpu_physical_memory_set_dirty(addr);
+ migration_bitmap_set_dirty(addr);
}
}
}
@@ -298,6 +300,11 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
bytes_transferred_last = bytes_transferred;
bwidth = qemu_get_clock_ns(rt_clock);
+ if (stage != 3) {
+ qemu_mutex_lock_ramlist();
+ qemu_mutex_unlock_iothread();
+ }
+
while (!qemu_file_rate_limit(f)) {
int bytes_sent;
@@ -308,6 +315,11 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
}
}
+ if (stage != 3) {
+ qemu_mutex_lock_iothread();
+ qemu_mutex_unlock_ramlist();
+ }
+
bwidth = qemu_get_clock_ns(rt_clock) - bwidth;
bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
diff --git a/cpu-all.h b/cpu-all.h
index eab9803..e709277 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -935,6 +935,7 @@ typedef struct RAMBlock {
typedef struct RAMList {
QemuMutex mutex;
uint8_t *phys_dirty;
+ uint8_t *migration_bitmap;
QLIST_HEAD(ram, RAMBlock) blocks;
QLIST_HEAD(, RAMBlock) blocks_mru;
} RAMList;
@@ -1008,8 +1009,44 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
}
}
+
+
void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
int dirty_flags);
+
+static inline int migration_bitmap_get_dirty(ram_addr_t addr,
+ int dirty_flags)
+{
+ return ram_list.migration_bitmap[addr >> TARGET_PAGE_BITS] & dirty_flags;
+}
+
+static inline void migration_bitmap_set_dirty(ram_addr_t addr)
+{
+ ram_list.migration_bitmap[addr >> TARGET_PAGE_BITS] = 0xff;
+}
+
+static inline void migration_bitmap_mask_dirty_range(ram_addr_t start,
+ int length,
+ int dirty_flags)
+{
+ int i, mask, len;
+ uint8_t *p;
+
+ len = length >> TARGET_PAGE_BITS;
+ mask = ~dirty_flags;
+ p = ram_list.migration_bitmap + (start >> TARGET_PAGE_BITS);
+ for (i = 0; i < len; i++) {
+ p[i] &= mask;
+ }
+}
+
+
+void migration_bitmap_reset_dirty(ram_addr_t start,
+ ram_addr_t end,
+ int dirty_flags);
+
+void sync_migration_bitmap(ram_addr_t start, ram_addr_t end);
+
void cpu_tlb_update_dirty(CPUState *env);
int cpu_physical_memory_set_dirty_tracking(int enable);
diff --git a/exec.c b/exec.c
index 404d8ea..36a44c7 100644
--- a/exec.c
+++ b/exec.c
@@ -2111,6 +2111,10 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
abort();
}
+ if (kvm_enabled()) {
+ return;
+ }
+
for(env = first_cpu; env != NULL; env = env->next_cpu) {
int mmu_idx;
for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
@@ -2119,8 +2123,61 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
start1, length);
}
}
+
+}
+
+void migration_bitmap_reset_dirty(ram_addr_t start, ram_addr_t end,
+ int dirty_flags)
+{
+ unsigned long length, start1;
+
+ start &= TARGET_PAGE_MASK;
+ end = TARGET_PAGE_ALIGN(end);
+
+ length = end - start;
+ if (length == 0) {
+ return;
+ }
+
+ migration_bitmap_mask_dirty_range(start, length, dirty_flags);
+
+ /* we modify the TLB cache so that the dirty bit will be set again
+ when accessing the range */
+ start1 = (unsigned long)qemu_safe_ram_ptr(start);
+ /* Check that we don't span multiple blocks - this breaks the
+ address comparisons below. */
+ if ((unsigned long)qemu_safe_ram_ptr(end - 1) - start1
+ != (end - 1) - start) {
+ abort();
+ }
+}
+
+void sync_migration_bitmap(ram_addr_t start, ram_addr_t end)
+{
+ unsigned long length, len, i;
+ ram_addr_t addr;
+ start &= TARGET_PAGE_MASK;
+ end = TARGET_PAGE_ALIGN(end);
+
+ length = end - start;
+ if (length == 0) {
+ return;
+ }
+
+ len = length >> TARGET_PAGE_BITS;
+ for (i = 0; i < len; i++) {
+ addr = i << TARGET_PAGE_BITS;
+ if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
+ migration_bitmap_set_dirty(addr);
+ cpu_physical_memory_reset_dirty(addr, addr + TARGET_PAGE_SIZE,
+ MIGRATION_DIRTY_FLAG);
+ }
+ }
+
}
+
+
int cpu_physical_memory_set_dirty_tracking(int enable)
{
int ret = 0;
@@ -2997,6 +3054,13 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
0xff, size >> TARGET_PAGE_BITS);
+ ram_list.migration_bitmap = qemu_realloc(ram_list.phys_dirty,
+ last_ram_offset() >>
+ TARGET_PAGE_BITS);
+
+ memset(ram_list.migration_bitmap + (new_block->offset >> TARGET_PAGE_BITS),
+ 0xff, size >> TARGET_PAGE_BITS);
+
qemu_mutex_unlock_ramlist();
if (kvm_enabled())
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH v4 3/5] separate migration bitmap
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
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 avoids
concurrent access to the qemu bitmap from iothread and migration thread.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
arch_init.c | 26 +++++++++++++++++------
cpu-all.h | 37 ++++++++++++++++++++++++++++++++++
exec.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 120 insertions(+), 7 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 484b39d..296b7d6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -123,13 +123,13 @@ static int ram_save_block(QEMUFile *f)
current_addr = block->offset + offset;
do {
- if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
+ if (migration_bitmap_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
uint8_t *p;
int cont = (block == last_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
- cpu_physical_memory_reset_dirty(current_addr,
- current_addr + TARGET_PAGE_SIZE,
- MIGRATION_DIRTY_FLAG);
+ migration_bitmap_reset_dirty(current_addr,
+ current_addr + TARGET_PAGE_SIZE,
+ MIGRATION_DIRTY_FLAG);
p = block->host + offset;
@@ -185,7 +185,7 @@ static ram_addr_t ram_save_remaining(void)
ram_addr_t addr;
for (addr = block->offset; addr < block->offset + block->length;
addr += TARGET_PAGE_SIZE) {
- if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
+ if (migration_bitmap_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
count++;
}
}
@@ -265,6 +265,8 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
return 0;
}
+ sync_migration_bitmap(0, TARGET_PHYS_ADDR_MAX);
+
if (stage == 1) {
RAMBlock *block;
bytes_transferred = 0;
@@ -276,9 +278,9 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
QLIST_FOREACH(block, &ram_list.blocks, next) {
for (addr = block->offset; addr < block->offset + block->length;
addr += TARGET_PAGE_SIZE) {
- if (!cpu_physical_memory_get_dirty(addr,
+ if (!migration_bitmap_get_dirty(addr,
MIGRATION_DIRTY_FLAG)) {
- cpu_physical_memory_set_dirty(addr);
+ migration_bitmap_set_dirty(addr);
}
}
}
@@ -298,6 +300,11 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
bytes_transferred_last = bytes_transferred;
bwidth = qemu_get_clock_ns(rt_clock);
+ if (stage != 3) {
+ qemu_mutex_lock_ramlist();
+ qemu_mutex_unlock_iothread();
+ }
+
while (!qemu_file_rate_limit(f)) {
int bytes_sent;
@@ -308,6 +315,11 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
}
}
+ if (stage != 3) {
+ qemu_mutex_lock_iothread();
+ qemu_mutex_unlock_ramlist();
+ }
+
bwidth = qemu_get_clock_ns(rt_clock) - bwidth;
bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
diff --git a/cpu-all.h b/cpu-all.h
index eab9803..e709277 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -935,6 +935,7 @@ typedef struct RAMBlock {
typedef struct RAMList {
QemuMutex mutex;
uint8_t *phys_dirty;
+ uint8_t *migration_bitmap;
QLIST_HEAD(ram, RAMBlock) blocks;
QLIST_HEAD(, RAMBlock) blocks_mru;
} RAMList;
@@ -1008,8 +1009,44 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
}
}
+
+
void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
int dirty_flags);
+
+static inline int migration_bitmap_get_dirty(ram_addr_t addr,
+ int dirty_flags)
+{
+ return ram_list.migration_bitmap[addr >> TARGET_PAGE_BITS] & dirty_flags;
+}
+
+static inline void migration_bitmap_set_dirty(ram_addr_t addr)
+{
+ ram_list.migration_bitmap[addr >> TARGET_PAGE_BITS] = 0xff;
+}
+
+static inline void migration_bitmap_mask_dirty_range(ram_addr_t start,
+ int length,
+ int dirty_flags)
+{
+ int i, mask, len;
+ uint8_t *p;
+
+ len = length >> TARGET_PAGE_BITS;
+ mask = ~dirty_flags;
+ p = ram_list.migration_bitmap + (start >> TARGET_PAGE_BITS);
+ for (i = 0; i < len; i++) {
+ p[i] &= mask;
+ }
+}
+
+
+void migration_bitmap_reset_dirty(ram_addr_t start,
+ ram_addr_t end,
+ int dirty_flags);
+
+void sync_migration_bitmap(ram_addr_t start, ram_addr_t end);
+
void cpu_tlb_update_dirty(CPUState *env);
int cpu_physical_memory_set_dirty_tracking(int enable);
diff --git a/exec.c b/exec.c
index 404d8ea..36a44c7 100644
--- a/exec.c
+++ b/exec.c
@@ -2111,6 +2111,10 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
abort();
}
+ if (kvm_enabled()) {
+ return;
+ }
+
for(env = first_cpu; env != NULL; env = env->next_cpu) {
int mmu_idx;
for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
@@ -2119,8 +2123,61 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
start1, length);
}
}
+
+}
+
+void migration_bitmap_reset_dirty(ram_addr_t start, ram_addr_t end,
+ int dirty_flags)
+{
+ unsigned long length, start1;
+
+ start &= TARGET_PAGE_MASK;
+ end = TARGET_PAGE_ALIGN(end);
+
+ length = end - start;
+ if (length == 0) {
+ return;
+ }
+
+ migration_bitmap_mask_dirty_range(start, length, dirty_flags);
+
+ /* we modify the TLB cache so that the dirty bit will be set again
+ when accessing the range */
+ start1 = (unsigned long)qemu_safe_ram_ptr(start);
+ /* Check that we don't span multiple blocks - this breaks the
+ address comparisons below. */
+ if ((unsigned long)qemu_safe_ram_ptr(end - 1) - start1
+ != (end - 1) - start) {
+ abort();
+ }
+}
+
+void sync_migration_bitmap(ram_addr_t start, ram_addr_t end)
+{
+ unsigned long length, len, i;
+ ram_addr_t addr;
+ start &= TARGET_PAGE_MASK;
+ end = TARGET_PAGE_ALIGN(end);
+
+ length = end - start;
+ if (length == 0) {
+ return;
+ }
+
+ len = length >> TARGET_PAGE_BITS;
+ for (i = 0; i < len; i++) {
+ addr = i << TARGET_PAGE_BITS;
+ if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
+ migration_bitmap_set_dirty(addr);
+ cpu_physical_memory_reset_dirty(addr, addr + TARGET_PAGE_SIZE,
+ MIGRATION_DIRTY_FLAG);
+ }
+ }
+
}
+
+
int cpu_physical_memory_set_dirty_tracking(int enable)
{
int ret = 0;
@@ -2997,6 +3054,13 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
0xff, size >> TARGET_PAGE_BITS);
+ ram_list.migration_bitmap = qemu_realloc(ram_list.phys_dirty,
+ last_ram_offset() >>
+ TARGET_PAGE_BITS);
+
+ memset(ram_list.migration_bitmap + (new_block->offset >> TARGET_PAGE_BITS),
+ 0xff, size >> TARGET_PAGE_BITS);
+
qemu_mutex_unlock_ramlist();
if (kvm_enabled())
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v4 4/5] separate thread for VM migration
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 3:56 ` Umesh Deshpande
-1 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
This patch creates a separate thread for the guest migration on the source side.
migrate_cancel request from the iothread is handled asynchronously. That is,
iothread submits migrate_cancel to the migration thread and returns, while the
migration thread attends this request at the next iteration to terminate its
execution.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
buffered_file.c | 93 ++++++++++++++++++++++++++++++++++--------------------
migration.c | 77 +++++++++++++++++++++++----------------------
migration.h | 1 +
savevm.c | 5 ---
4 files changed, 99 insertions(+), 77 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 41b42c3..bdcdf42 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -16,6 +16,8 @@
#include "qemu-timer.h"
#include "qemu-char.h"
#include "buffered_file.h"
+#include "migration.h"
+#include "qemu-thread.h"
//#define DEBUG_BUFFERED_FILE
@@ -28,13 +30,14 @@ typedef struct QEMUFileBuffered
void *opaque;
QEMUFile *file;
int has_error;
+ int closed;
int freeze_output;
size_t bytes_xfer;
size_t xfer_limit;
uint8_t *buffer;
size_t buffer_size;
size_t buffer_capacity;
- QEMUTimer *timer;
+ QemuThread thread;
} QEMUFileBuffered;
#ifdef DEBUG_BUFFERED_FILE
@@ -155,14 +158,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
offset = size;
}
- if (pos == 0 && size == 0) {
- DPRINTF("file is ready\n");
- if (s->bytes_xfer <= s->xfer_limit) {
- DPRINTF("notifying client\n");
- s->put_ready(s->opaque);
- }
- }
-
return offset;
}
@@ -175,20 +170,20 @@ static int buffered_close(void *opaque)
while (!s->has_error && s->buffer_size) {
buffered_flush(s);
- if (s->freeze_output)
+ if (s->freeze_output) {
s->wait_for_unfreeze(s);
+ }
}
- ret = s->close(s->opaque);
+ s->closed = 1;
- qemu_del_timer(s->timer);
- qemu_free_timer(s->timer);
+ ret = s->close(s->opaque);
qemu_free(s->buffer);
- qemu_free(s);
return ret;
}
+
static int buffered_rate_limit(void *opaque)
{
QEMUFileBuffered *s = opaque;
@@ -228,34 +223,63 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-static void buffered_rate_tick(void *opaque)
+static void *migrate_vm(void *opaque)
{
QEMUFileBuffered *s = opaque;
+ int64_t current_time, expire_time = qemu_get_clock_ms(rt_clock) + 100;
+ struct timeval tv = { .tv_sec = 0, .tv_usec = 100000};
- if (s->has_error) {
- buffered_close(s);
- return;
- }
+ qemu_mutex_lock_iothread();
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+ while (!s->closed) {
+ if (s->freeze_output) {
+ qemu_mutex_lock_ramlist();
+ qemu_mutex_unlock_iothread();
+ s->wait_for_unfreeze(s);
+ qemu_mutex_lock_iothread();
+ qemu_mutex_unlock_ramlist();
+ s->freeze_output = 0;
+ continue;
+ }
- if (s->freeze_output)
- return;
+ if (s->has_error) {
+ break;
+ }
+
+ current_time = qemu_get_clock_ms(rt_clock);
+ if (!s->closed && (expire_time > current_time)) {
+ tv.tv_usec = 1000 * (expire_time - current_time);
+ qemu_mutex_lock_ramlist();
+ qemu_mutex_unlock_iothread();
+ select(0, NULL, NULL, NULL, &tv);
+ qemu_mutex_lock_iothread();
+ qemu_mutex_unlock_ramlist();
+ continue;
+ }
- s->bytes_xfer = 0;
+ s->bytes_xfer = 0;
+ buffered_flush(s);
- buffered_flush(s);
+ expire_time = qemu_get_clock_ms(rt_clock) + 100;
+ s->put_ready(s->opaque);
+ }
- /* Add some checks around this */
- s->put_ready(s->opaque);
+ if (s->has_error) {
+ buffered_close(s);
+ }
+ qemu_free(s);
+
+ qemu_mutex_unlock_iothread();
+
+ return NULL;
}
QEMUFile *qemu_fopen_ops_buffered(void *opaque,
- size_t bytes_per_sec,
- BufferedPutFunc *put_buffer,
- BufferedPutReadyFunc *put_ready,
- BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
- BufferedCloseFunc *close)
+ size_t bytes_per_sec,
+ BufferedPutFunc *put_buffer,
+ BufferedPutReadyFunc *put_ready,
+ BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
+ BufferedCloseFunc *close)
{
QEMUFileBuffered *s;
@@ -267,15 +291,14 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
s->put_ready = put_ready;
s->wait_for_unfreeze = wait_for_unfreeze;
s->close = close;
+ s->closed = 0;
s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
- buffered_get_rate_limit);
-
- s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
+ buffered_get_rate_limit);
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+ qemu_thread_create(&s->thread, migrate_vm, s);
return s->file;
}
diff --git a/migration.c b/migration.c
index af3a1f2..b6ba690 100644
--- a/migration.c
+++ b/migration.c
@@ -34,6 +34,8 @@
/* Migration speed throttling */
static int64_t max_throttle = (32 << 20);
+static int change_speed;
+
static MigrationState *current_migration;
static NotifierList migration_state_notifiers =
@@ -141,18 +143,13 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
int64_t d;
- FdMigrationState *s;
d = qdict_get_int(qdict, "value");
if (d < 0) {
d = 0;
}
max_throttle = d;
-
- s = migrate_to_fms(current_migration);
- if (s && s->file) {
- qemu_file_set_rate_limit(s->file, max_throttle);
- }
+ change_speed = 1;
return 0;
}
@@ -284,8 +281,6 @@ int migrate_fd_cleanup(FdMigrationState *s)
{
int ret = 0;
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-
if (s->file) {
DPRINTF("closing file\n");
if (qemu_fclose(s->file) != 0) {
@@ -307,14 +302,6 @@ int migrate_fd_cleanup(FdMigrationState *s)
return ret;
}
-void migrate_fd_put_notify(void *opaque)
-{
- FdMigrationState *s = opaque;
-
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
- qemu_file_put_notify(s->file);
-}
-
ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
{
FdMigrationState *s = opaque;
@@ -327,9 +314,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
if (ret == -1)
ret = -(s->get_error(s));
- if (ret == -EAGAIN) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
- } else if (ret < 0) {
+ if (ret < 0 && ret != -EAGAIN) {
if (s->mon) {
monitor_resume(s->mon);
}
@@ -340,36 +325,59 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
return ret;
}
-void migrate_fd_connect(FdMigrationState *s)
+static void migrate_fd_set_speed(void)
{
- int ret;
+ FdMigrationState *s = migrate_to_fms(current_migration);
+ if (s && s->file && change_speed) {
+ qemu_file_set_rate_limit(s->file, max_throttle);
+ change_speed = 0;
+ }
+}
+
+static void migrate_fd_terminate(FdMigrationState *s)
+{
+ notifier_list_notify(&migration_state_notifiers);
+ qemu_savevm_state_cancel(s->mon, s->file);
+ migrate_fd_cleanup(s);
+}
+
+void migrate_fd_connect(FdMigrationState *s)
+{
+ s->begin = 1;
s->file = qemu_fopen_ops_buffered(s,
s->bandwidth_limit,
migrate_fd_put_buffer,
migrate_fd_put_ready,
migrate_fd_wait_for_unfreeze,
migrate_fd_close);
-
- DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
- s->mig_state.shared);
- if (ret < 0) {
- DPRINTF("failed, %d\n", ret);
- migrate_fd_error(s);
- return;
- }
-
- migrate_fd_put_ready(s);
}
void migrate_fd_put_ready(void *opaque)
{
FdMigrationState *s = opaque;
+ int ret;
if (s->state != MIG_STATE_ACTIVE) {
DPRINTF("put_ready returning because of non-active state\n");
+ if (s->state == MIG_STATE_CANCELLED) {
+ migrate_fd_terminate(s);
+ }
return;
+ } else {
+ migrate_fd_set_speed();
+ }
+
+ if (s->begin) {
+ DPRINTF("beginning savevm\n");
+ ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
+ s->mig_state.shared);
+ if (ret < 0) {
+ DPRINTF("failed, %d\n", ret);
+ migrate_fd_error(s);
+ return;
+ }
+ s->begin = 0;
}
DPRINTF("iterate\n");
@@ -415,10 +423,6 @@ void migrate_fd_cancel(MigrationState *mig_state)
DPRINTF("cancelling migration\n");
s->state = MIG_STATE_CANCELLED;
- notifier_list_notify(&migration_state_notifiers);
- qemu_savevm_state_cancel(s->mon, s->file);
-
- migrate_fd_cleanup(s);
}
void migrate_fd_release(MigrationState *mig_state)
@@ -458,7 +462,6 @@ int migrate_fd_close(void *opaque)
{
FdMigrationState *s = opaque;
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
return s->close(s);
}
diff --git a/migration.h b/migration.h
index 050c56c..4f50df8 100644
--- a/migration.h
+++ b/migration.h
@@ -45,6 +45,7 @@ struct FdMigrationState
int fd;
Monitor *mon;
int state;
+ int begin;
int (*get_error)(struct FdMigrationState*);
int (*close)(struct FdMigrationState*);
int (*write)(struct FdMigrationState*, const void *, size_t);
diff --git a/savevm.c b/savevm.c
index 8139bc7..f54f555 100644
--- a/savevm.c
+++ b/savevm.c
@@ -481,11 +481,6 @@ int qemu_fclose(QEMUFile *f)
return ret;
}
-void qemu_file_put_notify(QEMUFile *f)
-{
- f->put_buffer(f->opaque, NULL, 0, 0);
-}
-
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH v4 4/5] separate thread for VM migration
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
This patch creates a separate thread for the guest migration on the source side.
migrate_cancel request from the iothread is handled asynchronously. That is,
iothread submits migrate_cancel to the migration thread and returns, while the
migration thread attends this request at the next iteration to terminate its
execution.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
buffered_file.c | 93 ++++++++++++++++++++++++++++++++++--------------------
migration.c | 77 +++++++++++++++++++++++----------------------
migration.h | 1 +
savevm.c | 5 ---
4 files changed, 99 insertions(+), 77 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index 41b42c3..bdcdf42 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -16,6 +16,8 @@
#include "qemu-timer.h"
#include "qemu-char.h"
#include "buffered_file.h"
+#include "migration.h"
+#include "qemu-thread.h"
//#define DEBUG_BUFFERED_FILE
@@ -28,13 +30,14 @@ typedef struct QEMUFileBuffered
void *opaque;
QEMUFile *file;
int has_error;
+ int closed;
int freeze_output;
size_t bytes_xfer;
size_t xfer_limit;
uint8_t *buffer;
size_t buffer_size;
size_t buffer_capacity;
- QEMUTimer *timer;
+ QemuThread thread;
} QEMUFileBuffered;
#ifdef DEBUG_BUFFERED_FILE
@@ -155,14 +158,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
offset = size;
}
- if (pos == 0 && size == 0) {
- DPRINTF("file is ready\n");
- if (s->bytes_xfer <= s->xfer_limit) {
- DPRINTF("notifying client\n");
- s->put_ready(s->opaque);
- }
- }
-
return offset;
}
@@ -175,20 +170,20 @@ static int buffered_close(void *opaque)
while (!s->has_error && s->buffer_size) {
buffered_flush(s);
- if (s->freeze_output)
+ if (s->freeze_output) {
s->wait_for_unfreeze(s);
+ }
}
- ret = s->close(s->opaque);
+ s->closed = 1;
- qemu_del_timer(s->timer);
- qemu_free_timer(s->timer);
+ ret = s->close(s->opaque);
qemu_free(s->buffer);
- qemu_free(s);
return ret;
}
+
static int buffered_rate_limit(void *opaque)
{
QEMUFileBuffered *s = opaque;
@@ -228,34 +223,63 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
-static void buffered_rate_tick(void *opaque)
+static void *migrate_vm(void *opaque)
{
QEMUFileBuffered *s = opaque;
+ int64_t current_time, expire_time = qemu_get_clock_ms(rt_clock) + 100;
+ struct timeval tv = { .tv_sec = 0, .tv_usec = 100000};
- if (s->has_error) {
- buffered_close(s);
- return;
- }
+ qemu_mutex_lock_iothread();
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+ while (!s->closed) {
+ if (s->freeze_output) {
+ qemu_mutex_lock_ramlist();
+ qemu_mutex_unlock_iothread();
+ s->wait_for_unfreeze(s);
+ qemu_mutex_lock_iothread();
+ qemu_mutex_unlock_ramlist();
+ s->freeze_output = 0;
+ continue;
+ }
- if (s->freeze_output)
- return;
+ if (s->has_error) {
+ break;
+ }
+
+ current_time = qemu_get_clock_ms(rt_clock);
+ if (!s->closed && (expire_time > current_time)) {
+ tv.tv_usec = 1000 * (expire_time - current_time);
+ qemu_mutex_lock_ramlist();
+ qemu_mutex_unlock_iothread();
+ select(0, NULL, NULL, NULL, &tv);
+ qemu_mutex_lock_iothread();
+ qemu_mutex_unlock_ramlist();
+ continue;
+ }
- s->bytes_xfer = 0;
+ s->bytes_xfer = 0;
+ buffered_flush(s);
- buffered_flush(s);
+ expire_time = qemu_get_clock_ms(rt_clock) + 100;
+ s->put_ready(s->opaque);
+ }
- /* Add some checks around this */
- s->put_ready(s->opaque);
+ if (s->has_error) {
+ buffered_close(s);
+ }
+ qemu_free(s);
+
+ qemu_mutex_unlock_iothread();
+
+ return NULL;
}
QEMUFile *qemu_fopen_ops_buffered(void *opaque,
- size_t bytes_per_sec,
- BufferedPutFunc *put_buffer,
- BufferedPutReadyFunc *put_ready,
- BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
- BufferedCloseFunc *close)
+ size_t bytes_per_sec,
+ BufferedPutFunc *put_buffer,
+ BufferedPutReadyFunc *put_ready,
+ BufferedWaitForUnfreezeFunc *wait_for_unfreeze,
+ BufferedCloseFunc *close)
{
QEMUFileBuffered *s;
@@ -267,15 +291,14 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
s->put_ready = put_ready;
s->wait_for_unfreeze = wait_for_unfreeze;
s->close = close;
+ s->closed = 0;
s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
- buffered_get_rate_limit);
-
- s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
+ buffered_get_rate_limit);
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 100);
+ qemu_thread_create(&s->thread, migrate_vm, s);
return s->file;
}
diff --git a/migration.c b/migration.c
index af3a1f2..b6ba690 100644
--- a/migration.c
+++ b/migration.c
@@ -34,6 +34,8 @@
/* Migration speed throttling */
static int64_t max_throttle = (32 << 20);
+static int change_speed;
+
static MigrationState *current_migration;
static NotifierList migration_state_notifiers =
@@ -141,18 +143,13 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
int64_t d;
- FdMigrationState *s;
d = qdict_get_int(qdict, "value");
if (d < 0) {
d = 0;
}
max_throttle = d;
-
- s = migrate_to_fms(current_migration);
- if (s && s->file) {
- qemu_file_set_rate_limit(s->file, max_throttle);
- }
+ change_speed = 1;
return 0;
}
@@ -284,8 +281,6 @@ int migrate_fd_cleanup(FdMigrationState *s)
{
int ret = 0;
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-
if (s->file) {
DPRINTF("closing file\n");
if (qemu_fclose(s->file) != 0) {
@@ -307,14 +302,6 @@ int migrate_fd_cleanup(FdMigrationState *s)
return ret;
}
-void migrate_fd_put_notify(void *opaque)
-{
- FdMigrationState *s = opaque;
-
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
- qemu_file_put_notify(s->file);
-}
-
ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
{
FdMigrationState *s = opaque;
@@ -327,9 +314,7 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
if (ret == -1)
ret = -(s->get_error(s));
- if (ret == -EAGAIN) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
- } else if (ret < 0) {
+ if (ret < 0 && ret != -EAGAIN) {
if (s->mon) {
monitor_resume(s->mon);
}
@@ -340,36 +325,59 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
return ret;
}
-void migrate_fd_connect(FdMigrationState *s)
+static void migrate_fd_set_speed(void)
{
- int ret;
+ FdMigrationState *s = migrate_to_fms(current_migration);
+ if (s && s->file && change_speed) {
+ qemu_file_set_rate_limit(s->file, max_throttle);
+ change_speed = 0;
+ }
+}
+
+static void migrate_fd_terminate(FdMigrationState *s)
+{
+ notifier_list_notify(&migration_state_notifiers);
+ qemu_savevm_state_cancel(s->mon, s->file);
+ migrate_fd_cleanup(s);
+}
+
+void migrate_fd_connect(FdMigrationState *s)
+{
+ s->begin = 1;
s->file = qemu_fopen_ops_buffered(s,
s->bandwidth_limit,
migrate_fd_put_buffer,
migrate_fd_put_ready,
migrate_fd_wait_for_unfreeze,
migrate_fd_close);
-
- DPRINTF("beginning savevm\n");
- ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
- s->mig_state.shared);
- if (ret < 0) {
- DPRINTF("failed, %d\n", ret);
- migrate_fd_error(s);
- return;
- }
-
- migrate_fd_put_ready(s);
}
void migrate_fd_put_ready(void *opaque)
{
FdMigrationState *s = opaque;
+ int ret;
if (s->state != MIG_STATE_ACTIVE) {
DPRINTF("put_ready returning because of non-active state\n");
+ if (s->state == MIG_STATE_CANCELLED) {
+ migrate_fd_terminate(s);
+ }
return;
+ } else {
+ migrate_fd_set_speed();
+ }
+
+ if (s->begin) {
+ DPRINTF("beginning savevm\n");
+ ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
+ s->mig_state.shared);
+ if (ret < 0) {
+ DPRINTF("failed, %d\n", ret);
+ migrate_fd_error(s);
+ return;
+ }
+ s->begin = 0;
}
DPRINTF("iterate\n");
@@ -415,10 +423,6 @@ void migrate_fd_cancel(MigrationState *mig_state)
DPRINTF("cancelling migration\n");
s->state = MIG_STATE_CANCELLED;
- notifier_list_notify(&migration_state_notifiers);
- qemu_savevm_state_cancel(s->mon, s->file);
-
- migrate_fd_cleanup(s);
}
void migrate_fd_release(MigrationState *mig_state)
@@ -458,7 +462,6 @@ int migrate_fd_close(void *opaque)
{
FdMigrationState *s = opaque;
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
return s->close(s);
}
diff --git a/migration.h b/migration.h
index 050c56c..4f50df8 100644
--- a/migration.h
+++ b/migration.h
@@ -45,6 +45,7 @@ struct FdMigrationState
int fd;
Monitor *mon;
int state;
+ int begin;
int (*get_error)(struct FdMigrationState*);
int (*close)(struct FdMigrationState*);
int (*write)(struct FdMigrationState*, const void *, size_t);
diff --git a/savevm.c b/savevm.c
index 8139bc7..f54f555 100644
--- a/savevm.c
+++ b/savevm.c
@@ -481,11 +481,6 @@ int qemu_fclose(QEMUFile *f)
return ret;
}
-void qemu_file_put_notify(QEMUFile *f)
-{
- f->put_buffer(f->opaque, NULL, 0, 0);
-}
-
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [RFC PATCH v3 5/5] Making iothread block for migrate_cancel
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 3:56 ` Umesh Deshpande
-1 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, quintela, mtosatti, Umesh Deshpande
Following patch makes iothread wait until the migration thread responds to the
migrate_cancel request and terminates its execution.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
buffered_file.c | 13 ++++++++++++-
buffered_file.h | 3 +++
hw/hw.h | 5 ++++-
migration.c | 1 +
qemu-thread-posix.c | 10 ++++++++++
qemu-thread.h | 1 +
savevm.c | 31 +++++++++++++++++++++----------
7 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index bdcdf42..405b17f 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -223,6 +223,16 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
+static void buffered_wait_for_cancel(void *opaque)
+{
+ QEMUFileBuffered *s = opaque;
+ QemuThread thread = s->thread;
+
+ qemu_mutex_unlock_iothread();
+ qemu_thread_join(thread);
+ qemu_mutex_lock_iothread();
+}
+
static void *migrate_vm(void *opaque)
{
QEMUFileBuffered *s = opaque;
@@ -296,7 +306,8 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
- buffered_get_rate_limit);
+ buffered_get_rate_limit,
+ buffered_wait_for_cancel);
qemu_thread_create(&s->thread, migrate_vm, s);
diff --git a/buffered_file.h b/buffered_file.h
index 98d358b..413cc9f 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -20,6 +20,9 @@ typedef ssize_t (BufferedPutFunc)(void *opaque, const void *data, size_t size);
typedef void (BufferedPutReadyFunc)(void *opaque);
typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
typedef int (BufferedCloseFunc)(void *opaque);
+typedef void (BufferedWaitForCancelFunc)(void *opaque);
+
+void wait_for_cancel(void *opaque);
QEMUFile *qemu_fopen_ops_buffered(void *opaque, size_t xfer_limit,
BufferedPutFunc *put_buffer,
diff --git a/hw/hw.h b/hw/hw.h
index 9dd7096..e1d5ea8 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -41,13 +41,15 @@ typedef int (QEMUFileRateLimit)(void *opaque);
*/
typedef int64_t (QEMUFileSetRateLimit)(void *opaque, int64_t new_rate);
typedef int64_t (QEMUFileGetRateLimit)(void *opaque);
+typedef void (QEMUFileWaitForCancel)(void *opaque);
QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
QEMUFileGetBufferFunc *get_buffer,
QEMUFileCloseFunc *close,
QEMUFileRateLimit *rate_limit,
QEMUFileSetRateLimit *set_rate_limit,
- QEMUFileGetRateLimit *get_rate_limit);
+ QEMUFileGetRateLimit *get_rate_limit,
+ QEMUFileWaitForCancel *wait_for_cancel);
QEMUFile *qemu_fopen(const char *filename, const char *mode);
QEMUFile *qemu_fdopen(int fd, const char *mode);
QEMUFile *qemu_fopen_socket(int fd);
@@ -56,6 +58,7 @@ QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
int qemu_stdio_fd(QEMUFile *f);
void qemu_fflush(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
+void qemu_wait_for_cancel(QEMUFile *f);
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
void qemu_put_byte(QEMUFile *f, int v);
diff --git a/migration.c b/migration.c
index b6ba690..0c5a484 100644
--- a/migration.c
+++ b/migration.c
@@ -423,6 +423,7 @@ void migrate_fd_cancel(MigrationState *mig_state)
DPRINTF("cancelling migration\n");
s->state = MIG_STATE_CANCELLED;
+ qemu_wait_for_cancel(s->file);
}
void migrate_fd_release(MigrationState *mig_state)
diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
index 2bd02ef..0d18b35 100644
--- a/qemu-thread-posix.c
+++ b/qemu-thread-posix.c
@@ -115,6 +115,16 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
error_exit(err, __func__);
}
+void qemu_thread_join(QemuThread thread)
+{
+ int err;
+
+ err = pthread_join(thread.thread, NULL);
+ if (err) {
+ error_exit(err, __func__);
+ }
+}
+
void qemu_thread_create(QemuThread *thread,
void *(*start_routine)(void*),
void *arg)
diff --git a/qemu-thread.h b/qemu-thread.h
index 0a73d50..909529f 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -30,6 +30,7 @@ void qemu_cond_destroy(QemuCond *cond);
void qemu_cond_signal(QemuCond *cond);
void qemu_cond_broadcast(QemuCond *cond);
void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex);
+void qemu_thread_join(QemuThread thread);
void qemu_thread_create(QemuThread *thread,
void *(*start_routine)(void*),
diff --git a/savevm.c b/savevm.c
index f54f555..8003411 100644
--- a/savevm.c
+++ b/savevm.c
@@ -164,6 +164,7 @@ struct QEMUFile {
QEMUFileRateLimit *rate_limit;
QEMUFileSetRateLimit *set_rate_limit;
QEMUFileGetRateLimit *get_rate_limit;
+ QEMUFileWaitForCancel *wait_for_cancel;
void *opaque;
int is_write;
@@ -261,10 +262,10 @@ QEMUFile *qemu_popen(FILE *stdio_file, const char *mode)
if(mode[0] == 'r') {
s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_pclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
} else {
s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_pclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
return s->file;
}
@@ -310,10 +311,10 @@ QEMUFile *qemu_fdopen(int fd, const char *mode)
if(mode[0] == 'r') {
s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
} else {
s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
return s->file;
@@ -328,7 +329,7 @@ QEMUFile *qemu_fopen_socket(int fd)
s->fd = fd;
s->file = qemu_fopen_ops(s, NULL, socket_get_buffer, socket_close,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
return s->file;
}
@@ -366,10 +367,10 @@ QEMUFile *qemu_fopen(const char *filename, const char *mode)
if(mode[0] == 'w') {
s->file = qemu_fopen_ops(s, file_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
} else {
s->file = qemu_fopen_ops(s, NULL, file_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
return s->file;
fail:
@@ -398,8 +399,9 @@ static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
{
if (is_writable)
return qemu_fopen_ops(bs, block_put_buffer, NULL, bdrv_fclose,
- NULL, NULL, NULL);
- return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
+ return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL, NULL,
+ NULL, NULL);
}
QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
@@ -407,7 +409,8 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
QEMUFileCloseFunc *close,
QEMUFileRateLimit *rate_limit,
QEMUFileSetRateLimit *set_rate_limit,
- QEMUFileGetRateLimit *get_rate_limit)
+ QEMUFileGetRateLimit *get_rate_limit,
+ QEMUFileWaitForCancel *wait_for_cancel)
{
QEMUFile *f;
@@ -420,6 +423,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
f->rate_limit = rate_limit;
f->set_rate_limit = set_rate_limit;
f->get_rate_limit = get_rate_limit;
+ f->wait_for_cancel = wait_for_cancel;
f->is_write = 0;
return f;
@@ -481,6 +485,13 @@ int qemu_fclose(QEMUFile *f)
return ret;
}
+void qemu_wait_for_cancel(QEMUFile *f)
+{
+ if (f->wait_for_cancel) {
+ f->wait_for_cancel(f->opaque);
+ }
+}
+
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH v3 5/5] Making iothread block for migrate_cancel
@ 2011-08-17 3:56 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-17 3:56 UTC (permalink / raw)
To: kvm, qemu-devel; +Cc: pbonzini, mtosatti, Umesh Deshpande, quintela
Following patch makes iothread wait until the migration thread responds to the
migrate_cancel request and terminates its execution.
Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
---
buffered_file.c | 13 ++++++++++++-
buffered_file.h | 3 +++
hw/hw.h | 5 ++++-
migration.c | 1 +
qemu-thread-posix.c | 10 ++++++++++
qemu-thread.h | 1 +
savevm.c | 31 +++++++++++++++++++++----------
7 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/buffered_file.c b/buffered_file.c
index bdcdf42..405b17f 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -223,6 +223,16 @@ static int64_t buffered_get_rate_limit(void *opaque)
return s->xfer_limit;
}
+static void buffered_wait_for_cancel(void *opaque)
+{
+ QEMUFileBuffered *s = opaque;
+ QemuThread thread = s->thread;
+
+ qemu_mutex_unlock_iothread();
+ qemu_thread_join(thread);
+ qemu_mutex_lock_iothread();
+}
+
static void *migrate_vm(void *opaque)
{
QEMUFileBuffered *s = opaque;
@@ -296,7 +306,8 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
- buffered_get_rate_limit);
+ buffered_get_rate_limit,
+ buffered_wait_for_cancel);
qemu_thread_create(&s->thread, migrate_vm, s);
diff --git a/buffered_file.h b/buffered_file.h
index 98d358b..413cc9f 100644
--- a/buffered_file.h
+++ b/buffered_file.h
@@ -20,6 +20,9 @@ typedef ssize_t (BufferedPutFunc)(void *opaque, const void *data, size_t size);
typedef void (BufferedPutReadyFunc)(void *opaque);
typedef void (BufferedWaitForUnfreezeFunc)(void *opaque);
typedef int (BufferedCloseFunc)(void *opaque);
+typedef void (BufferedWaitForCancelFunc)(void *opaque);
+
+void wait_for_cancel(void *opaque);
QEMUFile *qemu_fopen_ops_buffered(void *opaque, size_t xfer_limit,
BufferedPutFunc *put_buffer,
diff --git a/hw/hw.h b/hw/hw.h
index 9dd7096..e1d5ea8 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -41,13 +41,15 @@ typedef int (QEMUFileRateLimit)(void *opaque);
*/
typedef int64_t (QEMUFileSetRateLimit)(void *opaque, int64_t new_rate);
typedef int64_t (QEMUFileGetRateLimit)(void *opaque);
+typedef void (QEMUFileWaitForCancel)(void *opaque);
QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
QEMUFileGetBufferFunc *get_buffer,
QEMUFileCloseFunc *close,
QEMUFileRateLimit *rate_limit,
QEMUFileSetRateLimit *set_rate_limit,
- QEMUFileGetRateLimit *get_rate_limit);
+ QEMUFileGetRateLimit *get_rate_limit,
+ QEMUFileWaitForCancel *wait_for_cancel);
QEMUFile *qemu_fopen(const char *filename, const char *mode);
QEMUFile *qemu_fdopen(int fd, const char *mode);
QEMUFile *qemu_fopen_socket(int fd);
@@ -56,6 +58,7 @@ QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
int qemu_stdio_fd(QEMUFile *f);
void qemu_fflush(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
+void qemu_wait_for_cancel(QEMUFile *f);
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
void qemu_put_byte(QEMUFile *f, int v);
diff --git a/migration.c b/migration.c
index b6ba690..0c5a484 100644
--- a/migration.c
+++ b/migration.c
@@ -423,6 +423,7 @@ void migrate_fd_cancel(MigrationState *mig_state)
DPRINTF("cancelling migration\n");
s->state = MIG_STATE_CANCELLED;
+ qemu_wait_for_cancel(s->file);
}
void migrate_fd_release(MigrationState *mig_state)
diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
index 2bd02ef..0d18b35 100644
--- a/qemu-thread-posix.c
+++ b/qemu-thread-posix.c
@@ -115,6 +115,16 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
error_exit(err, __func__);
}
+void qemu_thread_join(QemuThread thread)
+{
+ int err;
+
+ err = pthread_join(thread.thread, NULL);
+ if (err) {
+ error_exit(err, __func__);
+ }
+}
+
void qemu_thread_create(QemuThread *thread,
void *(*start_routine)(void*),
void *arg)
diff --git a/qemu-thread.h b/qemu-thread.h
index 0a73d50..909529f 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -30,6 +30,7 @@ void qemu_cond_destroy(QemuCond *cond);
void qemu_cond_signal(QemuCond *cond);
void qemu_cond_broadcast(QemuCond *cond);
void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex);
+void qemu_thread_join(QemuThread thread);
void qemu_thread_create(QemuThread *thread,
void *(*start_routine)(void*),
diff --git a/savevm.c b/savevm.c
index f54f555..8003411 100644
--- a/savevm.c
+++ b/savevm.c
@@ -164,6 +164,7 @@ struct QEMUFile {
QEMUFileRateLimit *rate_limit;
QEMUFileSetRateLimit *set_rate_limit;
QEMUFileGetRateLimit *get_rate_limit;
+ QEMUFileWaitForCancel *wait_for_cancel;
void *opaque;
int is_write;
@@ -261,10 +262,10 @@ QEMUFile *qemu_popen(FILE *stdio_file, const char *mode)
if(mode[0] == 'r') {
s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_pclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
} else {
s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_pclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
return s->file;
}
@@ -310,10 +311,10 @@ QEMUFile *qemu_fdopen(int fd, const char *mode)
if(mode[0] == 'r') {
s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
} else {
s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
return s->file;
@@ -328,7 +329,7 @@ QEMUFile *qemu_fopen_socket(int fd)
s->fd = fd;
s->file = qemu_fopen_ops(s, NULL, socket_get_buffer, socket_close,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
return s->file;
}
@@ -366,10 +367,10 @@ QEMUFile *qemu_fopen(const char *filename, const char *mode)
if(mode[0] == 'w') {
s->file = qemu_fopen_ops(s, file_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
} else {
s->file = qemu_fopen_ops(s, NULL, file_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
}
return s->file;
fail:
@@ -398,8 +399,9 @@ static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
{
if (is_writable)
return qemu_fopen_ops(bs, block_put_buffer, NULL, bdrv_fclose,
- NULL, NULL, NULL);
- return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL, NULL, NULL);
+ NULL, NULL, NULL, NULL);
+ return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL, NULL,
+ NULL, NULL);
}
QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
@@ -407,7 +409,8 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
QEMUFileCloseFunc *close,
QEMUFileRateLimit *rate_limit,
QEMUFileSetRateLimit *set_rate_limit,
- QEMUFileGetRateLimit *get_rate_limit)
+ QEMUFileGetRateLimit *get_rate_limit,
+ QEMUFileWaitForCancel *wait_for_cancel)
{
QEMUFile *f;
@@ -420,6 +423,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
f->rate_limit = rate_limit;
f->set_rate_limit = set_rate_limit;
f->get_rate_limit = get_rate_limit;
+ f->wait_for_cancel = wait_for_cancel;
f->is_write = 0;
return f;
@@ -481,6 +485,13 @@ int qemu_fclose(QEMUFile *f)
return ret;
}
+void qemu_wait_for_cancel(QEMUFile *f)
+{
+ if (f->wait_for_cancel) {
+ f->wait_for_cancel(f->opaque);
+ }
+}
+
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
{
int l;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 6:28 ` Paolo Bonzini
-1 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-17 6:28 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
> @@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
>
> QLIST_FOREACH(block,&ram_list.blocks, next) {
> if (addr == block->offset) {
> + qemu_mutex_lock_ramlist();
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> + qemu_mutex_unlock_ramlist();
> qemu_free(block);
> return;
> }
> @@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
>
> QLIST_FOREACH(block,&ram_list.blocks, next) {
> if (addr == block->offset) {
> + qemu_mutex_lock_ramlist();
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> + qemu_mutex_unlock_ramlist();
> if (block->flags& RAM_PREALLOC_MASK) {
> ;
> } else if (mem_path) {
You must protect the whole QLIST_FOREACH. Otherwise looks good.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-17 6:28 ` Paolo Bonzini
0 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-17 6:28 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
> @@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
>
> QLIST_FOREACH(block,&ram_list.blocks, next) {
> if (addr == block->offset) {
> + qemu_mutex_lock_ramlist();
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> + qemu_mutex_unlock_ramlist();
> qemu_free(block);
> return;
> }
> @@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
>
> QLIST_FOREACH(block,&ram_list.blocks, next) {
> if (addr == block->offset) {
> + qemu_mutex_lock_ramlist();
> QLIST_REMOVE(block, next);
> QLIST_REMOVE(block, next_mru);
> + qemu_mutex_unlock_ramlist();
> if (block->flags& RAM_PREALLOC_MASK) {
> ;
> } else if (mem_path) {
You must protect the whole QLIST_FOREACH. Otherwise looks good.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 4/5] separate thread for VM migration
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 7:13 ` Paolo Bonzini
-1 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-17 7:13 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
> + qemu_mutex_lock_ramlist();
Taken locks: iothread, ramlist
> + qemu_mutex_unlock_iothread();
Taken locks: ramlist
> + s->wait_for_unfreeze(s);
> + qemu_mutex_lock_iothread();
Taken locks: ramlist, iothread
You'd have a deadlock here if hypothetically you had two migrations at
the same time.
> + qemu_mutex_unlock_ramlist();
But in general, why this locking? The buffered file need not know
anything about the ram list and its mutex. Only ram_save_live needs to
hold the ramlist lock---starting just before sort_ram_list and ending
just after the end of stage 3. That should be part of patch 2.
Actually buffered_file.c should ideally not even take the iothread lock!
The code there is only copying data from a private buffer to a file
descriptor; neither is shared. It's migrate_fd_put_buffer that should
take care of locking. This is an example of why keeping the separation
of QEMUBufferedFile is a good idea at least for now.
I am still kind of unconvinced about calling qemu_fclose from the
migration thread. You still have one instance of cancellation in the
iothread when migrate_fd_release is called. Ideally, as soon as
migration finishes or has an error you could trigger a bottom half that
closes the file (which in turn joins the thread). Migration state
notifiers should also be run only from the iothread. Failure to do so
(or in general lack of a policy of what runs where) can lead to very
difficult bugs. Not so much hard to debug in this case (we have a
global lock, so things cannot go _that_ bad), but hard to fix without
redoing everything.
However, this patch is a good start (with locking fixed). It should
takes several incremental steps before getting there, including
incredible simplification if you take into account that migration can
block and wait_for_unfreeze can disappear. In the end it probably
should be committed as a single patch, but I'm liking the patches more
and more.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 4/5] separate thread for VM migration
@ 2011-08-17 7:13 ` Paolo Bonzini
0 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-17 7:13 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
> + qemu_mutex_lock_ramlist();
Taken locks: iothread, ramlist
> + qemu_mutex_unlock_iothread();
Taken locks: ramlist
> + s->wait_for_unfreeze(s);
> + qemu_mutex_lock_iothread();
Taken locks: ramlist, iothread
You'd have a deadlock here if hypothetically you had two migrations at
the same time.
> + qemu_mutex_unlock_ramlist();
But in general, why this locking? The buffered file need not know
anything about the ram list and its mutex. Only ram_save_live needs to
hold the ramlist lock---starting just before sort_ram_list and ending
just after the end of stage 3. That should be part of patch 2.
Actually buffered_file.c should ideally not even take the iothread lock!
The code there is only copying data from a private buffer to a file
descriptor; neither is shared. It's migrate_fd_put_buffer that should
take care of locking. This is an example of why keeping the separation
of QEMUBufferedFile is a good idea at least for now.
I am still kind of unconvinced about calling qemu_fclose from the
migration thread. You still have one instance of cancellation in the
iothread when migrate_fd_release is called. Ideally, as soon as
migration finishes or has an error you could trigger a bottom half that
closes the file (which in turn joins the thread). Migration state
notifiers should also be run only from the iothread. Failure to do so
(or in general lack of a policy of what runs where) can lead to very
difficult bugs. Not so much hard to debug in this case (we have a
global lock, so things cannot go _that_ bad), but hard to fix without
redoing everything.
However, this patch is a good start (with locking fixed). It should
takes several incremental steps before getting there, including
incredible simplification if you take into account that migration can
block and wait_for_unfreeze can disappear. In the end it probably
should be committed as a single patch, but I'm liking the patches more
and more.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v3 5/5] Making iothread block for migrate_cancel
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-17 7:14 ` Paolo Bonzini
-1 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-17 7:14 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: kvm, qemu-devel, quintela, mtosatti
On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> index 2bd02ef..0d18b35 100644
> --- a/qemu-thread-posix.c
> +++ b/qemu-thread-posix.c
> @@ -115,6 +115,16 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
> error_exit(err, __func__);
> }
>
> +void qemu_thread_join(QemuThread thread)
Should take a pointer.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v3 5/5] Making iothread block for migrate_cancel
@ 2011-08-17 7:14 ` Paolo Bonzini
0 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-17 7:14 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> index 2bd02ef..0d18b35 100644
> --- a/qemu-thread-posix.c
> +++ b/qemu-thread-posix.c
> @@ -115,6 +115,16 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)
> error_exit(err, __func__);
> }
>
> +void qemu_thread_join(QemuThread thread)
Should take a pointer.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-17 6:28 ` [Qemu-devel] " Paolo Bonzini
@ 2011-08-19 6:20 ` Umesh Deshpande
-1 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-19 6:20 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, qemu-devel, quintela, mtosatti
On 08/17/2011 02:28 AM, Paolo Bonzini wrote:
> On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
>> @@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
>>
>> QLIST_FOREACH(block,&ram_list.blocks, next) {
>> if (addr == block->offset) {
>> + qemu_mutex_lock_ramlist();
>> QLIST_REMOVE(block, next);
>> QLIST_REMOVE(block, next_mru);
>> + qemu_mutex_unlock_ramlist();
>> qemu_free(block);
>> return;
>> }
>> @@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
>>
>> QLIST_FOREACH(block,&ram_list.blocks, next) {
>> if (addr == block->offset) {
>> + qemu_mutex_lock_ramlist();
>> QLIST_REMOVE(block, next);
>> QLIST_REMOVE(block, next_mru);
>> + qemu_mutex_unlock_ramlist();
>> if (block->flags& RAM_PREALLOC_MASK) {
>> ;
>> } else if (mem_path) {
>
> You must protect the whole QLIST_FOREACH. Otherwise looks good.
Or, is it okay to convert all the ramblock list traversals in exec.c
(under iothread) to mru traversals, and probably it makes sense as the
original list was also maintained in the mru order, whereas the sequence
of blocks doesn't matter for the migration code. This way we don't have
to acquire the mutex for block list traversals.
- Umesh
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-19 6:20 ` Umesh Deshpande
0 siblings, 0 replies; 30+ messages in thread
From: Umesh Deshpande @ 2011-08-19 6:20 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/17/2011 02:28 AM, Paolo Bonzini wrote:
> On 08/16/2011 08:56 PM, Umesh Deshpande wrote:
>> @@ -3001,8 +3016,10 @@ void qemu_ram_free_from_ptr(ram_addr_t addr)
>>
>> QLIST_FOREACH(block,&ram_list.blocks, next) {
>> if (addr == block->offset) {
>> + qemu_mutex_lock_ramlist();
>> QLIST_REMOVE(block, next);
>> QLIST_REMOVE(block, next_mru);
>> + qemu_mutex_unlock_ramlist();
>> qemu_free(block);
>> return;
>> }
>> @@ -3015,8 +3032,10 @@ void qemu_ram_free(ram_addr_t addr)
>>
>> QLIST_FOREACH(block,&ram_list.blocks, next) {
>> if (addr == block->offset) {
>> + qemu_mutex_lock_ramlist();
>> QLIST_REMOVE(block, next);
>> QLIST_REMOVE(block, next_mru);
>> + qemu_mutex_unlock_ramlist();
>> if (block->flags& RAM_PREALLOC_MASK) {
>> ;
>> } else if (mem_path) {
>
> You must protect the whole QLIST_FOREACH. Otherwise looks good.
Or, is it okay to convert all the ramblock list traversals in exec.c
(under iothread) to mru traversals, and probably it makes sense as the
original list was also maintained in the mru order, whereas the sequence
of blocks doesn't matter for the migration code. This way we don't have
to acquire the mutex for block list traversals.
- Umesh
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-19 6:20 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-22 6:48 ` Paolo Bonzini
-1 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-22 6:48 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: kvm, qemu-devel, quintela, mtosatti
On 08/19/2011 08:20 AM, Umesh Deshpande wrote:
> Or, is it okay to convert all the ramblock list traversals in exec.c
> (under iothread) to mru traversals, and probably it makes sense as the
> original list was also maintained in the mru order, whereas the sequence
> of blocks doesn't matter for the migration code. This way we don't have
> to acquire the mutex for block list traversals.
I'm not sure... as I said, the MRU list is on a fast path and
restricting it to that fast path keeps us honest. Also, the non-MRU
list is almost never accessed outside the migration thread, so the mutex
shouldn't be heavily contended anyway. You can also think about (not
too clever) ways to keep the mutex unlocked while not doing ram_save_live.
BTW, actually the migration code tries to migrate the largest blocks
first (because usually all the blocks after the first are small and
easily migrated during the _complete pass), so the order does somewhat
matter for migration.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-22 6:48 ` Paolo Bonzini
0 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-22 6:48 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: mtosatti, qemu-devel, kvm, quintela
On 08/19/2011 08:20 AM, Umesh Deshpande wrote:
> Or, is it okay to convert all the ramblock list traversals in exec.c
> (under iothread) to mru traversals, and probably it makes sense as the
> original list was also maintained in the mru order, whereas the sequence
> of blocks doesn't matter for the migration code. This way we don't have
> to acquire the mutex for block list traversals.
I'm not sure... as I said, the MRU list is on a fast path and
restricting it to that fast path keeps us honest. Also, the non-MRU
list is almost never accessed outside the migration thread, so the mutex
shouldn't be heavily contended anyway. You can also think about (not
too clever) ways to keep the mutex unlocked while not doing ram_save_live.
BTW, actually the migration code tries to migrate the largest blocks
first (because usually all the blocks after the first are small and
easily migrated during the _complete pass), so the order does somewhat
matter for migration.
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
@ 2011-08-23 9:15 ` Marcelo Tosatti
-1 siblings, 0 replies; 30+ messages in thread
From: Marcelo Tosatti @ 2011-08-23 9:15 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: kvm, qemu-devel, pbonzini, quintela
On Tue, Aug 16, 2011 at 11:56:37PM -0400, Umesh Deshpande wrote:
> ramlist mutex is implemented to protect the RAMBlock list traversal in the
> migration thread from their addition/removal from the iothread.
>
> Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
> ---
> cpu-all.h | 2 ++
> exec.c | 19 +++++++++++++++++++
> qemu-common.h | 2 ++
> 3 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 6b217a2..eab9803 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -21,6 +21,7 @@
>
> #include "qemu-common.h"
> #include "cpu-common.h"
> +#include "qemu-thread.h"
>
> /* some important defines:
> *
> @@ -932,6 +933,7 @@ typedef struct RAMBlock {
> } RAMBlock;
>
> typedef struct RAMList {
> + QemuMutex mutex;
> uint8_t *phys_dirty;
> QLIST_HEAD(ram, RAMBlock) blocks;
> QLIST_HEAD(, RAMBlock) blocks_mru;
A comment on what the mutex protects would be good.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-23 9:15 ` Marcelo Tosatti
0 siblings, 0 replies; 30+ messages in thread
From: Marcelo Tosatti @ 2011-08-23 9:15 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: pbonzini, qemu-devel, kvm, quintela
On Tue, Aug 16, 2011 at 11:56:37PM -0400, Umesh Deshpande wrote:
> ramlist mutex is implemented to protect the RAMBlock list traversal in the
> migration thread from their addition/removal from the iothread.
>
> Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
> ---
> cpu-all.h | 2 ++
> exec.c | 19 +++++++++++++++++++
> qemu-common.h | 2 ++
> 3 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 6b217a2..eab9803 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -21,6 +21,7 @@
>
> #include "qemu-common.h"
> #include "cpu-common.h"
> +#include "qemu-thread.h"
>
> /* some important defines:
> *
> @@ -932,6 +933,7 @@ typedef struct RAMBlock {
> } RAMBlock;
>
> typedef struct RAMList {
> + QemuMutex mutex;
> uint8_t *phys_dirty;
> QLIST_HEAD(ram, RAMBlock) blocks;
> QLIST_HEAD(, RAMBlock) blocks_mru;
A comment on what the mutex protects would be good.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-23 9:15 ` [Qemu-devel] " Marcelo Tosatti
@ 2011-08-23 9:17 ` Marcelo Tosatti
-1 siblings, 0 replies; 30+ messages in thread
From: Marcelo Tosatti @ 2011-08-23 9:17 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: pbonzini, qemu-devel, kvm, quintela
On Tue, Aug 23, 2011 at 06:15:33AM -0300, Marcelo Tosatti wrote:
> On Tue, Aug 16, 2011 at 11:56:37PM -0400, Umesh Deshpande wrote:
> > ramlist mutex is implemented to protect the RAMBlock list traversal in the
> > migration thread from their addition/removal from the iothread.
> >
> > Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
> > ---
> > cpu-all.h | 2 ++
> > exec.c | 19 +++++++++++++++++++
> > qemu-common.h | 2 ++
> > 3 files changed, 23 insertions(+), 0 deletions(-)
> >
> > diff --git a/cpu-all.h b/cpu-all.h
> > index 6b217a2..eab9803 100644
> > --- a/cpu-all.h
> > +++ b/cpu-all.h
> > @@ -21,6 +21,7 @@
> >
> > #include "qemu-common.h"
> > #include "cpu-common.h"
> > +#include "qemu-thread.h"
> >
> > /* some important defines:
> > *
> > @@ -932,6 +933,7 @@ typedef struct RAMBlock {
> > } RAMBlock;
> >
> > typedef struct RAMList {
> > + QemuMutex mutex;
> > uint8_t *phys_dirty;
> > QLIST_HEAD(ram, RAMBlock) blocks;
> > QLIST_HEAD(, RAMBlock) blocks_mru;
>
> A comment on what the mutex protects would be good.
And on the lock ordering.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-23 9:17 ` Marcelo Tosatti
0 siblings, 0 replies; 30+ messages in thread
From: Marcelo Tosatti @ 2011-08-23 9:17 UTC (permalink / raw)
To: Umesh Deshpande; +Cc: pbonzini, qemu-devel, kvm, quintela
On Tue, Aug 23, 2011 at 06:15:33AM -0300, Marcelo Tosatti wrote:
> On Tue, Aug 16, 2011 at 11:56:37PM -0400, Umesh Deshpande wrote:
> > ramlist mutex is implemented to protect the RAMBlock list traversal in the
> > migration thread from their addition/removal from the iothread.
> >
> > Signed-off-by: Umesh Deshpande <udeshpan@redhat.com>
> > ---
> > cpu-all.h | 2 ++
> > exec.c | 19 +++++++++++++++++++
> > qemu-common.h | 2 ++
> > 3 files changed, 23 insertions(+), 0 deletions(-)
> >
> > diff --git a/cpu-all.h b/cpu-all.h
> > index 6b217a2..eab9803 100644
> > --- a/cpu-all.h
> > +++ b/cpu-all.h
> > @@ -21,6 +21,7 @@
> >
> > #include "qemu-common.h"
> > #include "cpu-common.h"
> > +#include "qemu-thread.h"
> >
> > /* some important defines:
> > *
> > @@ -932,6 +933,7 @@ typedef struct RAMBlock {
> > } RAMBlock;
> >
> > typedef struct RAMList {
> > + QemuMutex mutex;
> > uint8_t *phys_dirty;
> > QLIST_HEAD(ram, RAMBlock) blocks;
> > QLIST_HEAD(, RAMBlock) blocks_mru;
>
> A comment on what the mutex protects would be good.
And on the lock ordering.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-23 9:17 ` [Qemu-devel] " Marcelo Tosatti
@ 2011-08-23 11:41 ` Paolo Bonzini
-1 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-23 11:41 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Umesh Deshpande, kvm, qemu-devel, quintela
On 08/23/2011 11:17 AM, Marcelo Tosatti wrote:
>>> > > typedef struct RAMList {
>>> > > + QemuMutex mutex;
>>> > > uint8_t *phys_dirty;
>>> > > QLIST_HEAD(ram, RAMBlock) blocks;
>>> > > QLIST_HEAD(, RAMBlock) blocks_mru;
>> >
>> > A comment on what the mutex protects would be good.
Indeed, especially because Umesh wanted to use the ramlist+iothread
combo as a rw-lock: iothread = read-lock for the I/O thread, ramlist =
read-lock for the migration thread, together = exclusive (write) lock.
But I think I talked him out of this. :) It's not a bad idea in
general, it just sounds like overkill in this case.
> And on the lock ordering.
I think when only two locks are involved, we can always assume iothread
is outer and the other is inner. Do you agree?
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-23 11:41 ` Paolo Bonzini
0 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2011-08-23 11:41 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Umesh Deshpande, qemu-devel, kvm, quintela
On 08/23/2011 11:17 AM, Marcelo Tosatti wrote:
>>> > > typedef struct RAMList {
>>> > > + QemuMutex mutex;
>>> > > uint8_t *phys_dirty;
>>> > > QLIST_HEAD(ram, RAMBlock) blocks;
>>> > > QLIST_HEAD(, RAMBlock) blocks_mru;
>> >
>> > A comment on what the mutex protects would be good.
Indeed, especially because Umesh wanted to use the ramlist+iothread
combo as a rw-lock: iothread = read-lock for the I/O thread, ramlist =
read-lock for the migration thread, together = exclusive (write) lock.
But I think I talked him out of this. :) It's not a bad idea in
general, it just sounds like overkill in this case.
> And on the lock ordering.
I think when only two locks are involved, we can always assume iothread
is outer and the other is inner. Do you agree?
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC PATCH v4 2/5] ramlist mutex
2011-08-23 11:41 ` [Qemu-devel] " Paolo Bonzini
@ 2011-08-23 12:16 ` Marcelo Tosatti
-1 siblings, 0 replies; 30+ messages in thread
From: Marcelo Tosatti @ 2011-08-23 12:16 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Umesh Deshpande, qemu-devel, kvm, quintela
On Tue, Aug 23, 2011 at 01:41:48PM +0200, Paolo Bonzini wrote:
> On 08/23/2011 11:17 AM, Marcelo Tosatti wrote:
> >>>> > typedef struct RAMList {
> >>>> > + QemuMutex mutex;
> >>>> > uint8_t *phys_dirty;
> >>>> > QLIST_HEAD(ram, RAMBlock) blocks;
> >>>> > QLIST_HEAD(, RAMBlock) blocks_mru;
> >>>
> >>> A comment on what the mutex protects would be good.
>
> Indeed, especially because Umesh wanted to use the ramlist+iothread
> combo as a rw-lock: iothread = read-lock for the I/O thread, ramlist
> = read-lock for the migration thread, together = exclusive (write)
> lock. But I think I talked him out of this. :) It's not a bad idea
> in general, it just sounds like overkill in this case.
>
> >And on the lock ordering.
>
> I think when only two locks are involved, we can always assume
> iothread is outer and the other is inner. Do you agree?
>
> Paolo
Yep.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v4 2/5] ramlist mutex
@ 2011-08-23 12:16 ` Marcelo Tosatti
0 siblings, 0 replies; 30+ messages in thread
From: Marcelo Tosatti @ 2011-08-23 12:16 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Umesh Deshpande, qemu-devel, kvm, quintela
On Tue, Aug 23, 2011 at 01:41:48PM +0200, Paolo Bonzini wrote:
> On 08/23/2011 11:17 AM, Marcelo Tosatti wrote:
> >>>> > typedef struct RAMList {
> >>>> > + QemuMutex mutex;
> >>>> > uint8_t *phys_dirty;
> >>>> > QLIST_HEAD(ram, RAMBlock) blocks;
> >>>> > QLIST_HEAD(, RAMBlock) blocks_mru;
> >>>
> >>> A comment on what the mutex protects would be good.
>
> Indeed, especially because Umesh wanted to use the ramlist+iothread
> combo as a rw-lock: iothread = read-lock for the I/O thread, ramlist
> = read-lock for the migration thread, together = exclusive (write)
> lock. But I think I talked him out of this. :) It's not a bad idea
> in general, it just sounds like overkill in this case.
>
> >And on the lock ordering.
>
> I think when only two locks are involved, we can always assume
> iothread is outer and the other is inner. Do you agree?
>
> Paolo
Yep.
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2011-08-23 12:38 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-17 3:56 [RFC PATCH v4 0/5] Separate thread for VM migration Umesh Deshpande
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
2011-08-17 3:56 ` [RFC PATCH v4 1/5] MRU ram list Umesh Deshpande
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
2011-08-17 3:56 ` [RFC PATCH v4 2/5] ramlist mutex Umesh Deshpande
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
2011-08-17 6:28 ` Paolo Bonzini
2011-08-17 6:28 ` [Qemu-devel] " Paolo Bonzini
2011-08-19 6:20 ` Umesh Deshpande
2011-08-19 6:20 ` [Qemu-devel] " Umesh Deshpande
2011-08-22 6:48 ` Paolo Bonzini
2011-08-22 6:48 ` [Qemu-devel] " Paolo Bonzini
2011-08-23 9:15 ` Marcelo Tosatti
2011-08-23 9:15 ` [Qemu-devel] " Marcelo Tosatti
2011-08-23 9:17 ` Marcelo Tosatti
2011-08-23 9:17 ` [Qemu-devel] " Marcelo Tosatti
2011-08-23 11:41 ` Paolo Bonzini
2011-08-23 11:41 ` [Qemu-devel] " Paolo Bonzini
2011-08-23 12:16 ` Marcelo Tosatti
2011-08-23 12:16 ` [Qemu-devel] " Marcelo Tosatti
2011-08-17 3:56 ` [RFC PATCH v4 3/5] separate migration bitmap Umesh Deshpande
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
2011-08-17 3:56 ` [RFC PATCH v4 4/5] separate thread for VM migration Umesh Deshpande
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
2011-08-17 7:13 ` Paolo Bonzini
2011-08-17 7:13 ` [Qemu-devel] " Paolo Bonzini
2011-08-17 3:56 ` [RFC PATCH v3 5/5] Making iothread block for migrate_cancel Umesh Deshpande
2011-08-17 3:56 ` [Qemu-devel] " Umesh Deshpande
2011-08-17 7:14 ` Paolo Bonzini
2011-08-17 7:14 ` [Qemu-devel] " Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.