Hi,
> -----Original Message----- > From: Daniel Cho [mailto:daniel...@qnap.com] > Sent: Tuesday, February 25, 2020 10:53 AM > To: Zhanghailiang <zhang.zhanghaili...@huawei.com> > Cc: qemu-devel@nongnu.org; quint...@redhat.com; Dr. David Alan Gilbert > <dgilb...@redhat.com> > Subject: Re: [PATCH V2 4/8] COLO: Optimize memory back-up process > > Hi Hailiang, > > With version 2, the code in migration/ram.c > > + if (migration_incoming_colo_enabled()) { > + if (migration_incoming_in_colo_state()) { > + /* In COLO stage, put all pages into cache > temporarily */ > + host = colo_cache_from_block_offset(block, addr); > + } else { > + /* > + * In migration stage but before COLO stage, > + * Put all pages into both cache and SVM's memory. > + */ > + host_bak = colo_cache_from_block_offset(block, > addr); > + } > } > if (!host) { > error_report("Illegal RAM offset " RAM_ADDR_FMT, > addr); > ret = -EINVAL; > break; > } > > host = colo_cache_from_block_offset(block, addr); > host_bak = colo_cache_from_block_offset(block, addr); > Does it cause the "if(!host)" will go break if the condition goes > "host_bak = colo_cache_from_block_offset(block, addr);" ? > That will not happen, you may have missed this parts. @@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f) RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) { RAMBlock *block = ram_block_from_stream(f, flags); + host = host_from_ram_block_offset(block, addr); /* We have given host a value unconditionally. > Best regards, > Daniel Cho > > zhanghailiang <zhang.zhanghaili...@huawei.com> 於 2020年2月24日 週 > 一 下午2:55寫道: > > > > This patch will reduce the downtime of VM for the initial process, > > Privously, we copied all these memory in preparing stage of COLO > > while we need to stop VM, which is a time-consuming process. > > Here we optimize it by a trick, back-up every page while in migration > > process while COLO is enabled, though it affects the speed of the > > migration, but it obviously reduce the downtime of back-up all SVM'S > > memory in COLO preparing stage. > > > > Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> > > --- > > migration/colo.c | 3 +++ > > migration/ram.c | 68 > +++++++++++++++++++++++++++++++++++------------- > > migration/ram.h | 1 + > > 3 files changed, 54 insertions(+), 18 deletions(-) > > > > diff --git a/migration/colo.c b/migration/colo.c > > index 93c5a452fb..44942c4e23 100644 > > --- a/migration/colo.c > > +++ b/migration/colo.c > > @@ -26,6 +26,7 @@ > > #include "qemu/main-loop.h" > > #include "qemu/rcu.h" > > #include "migration/failover.h" > > +#include "migration/ram.h" > > #ifdef CONFIG_REPLICATION > > #include "replication.h" > > #endif > > @@ -845,6 +846,8 @@ void *colo_process_incoming_thread(void > *opaque) > > */ > > qemu_file_set_blocking(mis->from_src_file, true); > > > > + colo_incoming_start_dirty_log(); > > + > > bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE); > > fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc)); > > object_unref(OBJECT(bioc)); > > diff --git a/migration/ram.c b/migration/ram.c > > index ed23ed1c7c..ebf9e6ba51 100644 > > --- a/migration/ram.c > > +++ b/migration/ram.c > > @@ -2277,6 +2277,7 @@ static void ram_list_init_bitmaps(void) > > * dirty_memory[DIRTY_MEMORY_MIGRATION] don't > include the whole > > * guest memory. > > */ > > + > > block->bmap = bitmap_new(pages); > > bitmap_set(block->bmap, 0, pages); > > block->clear_bmap_shift = shift; > > @@ -2986,7 +2987,6 @@ int colo_init_ram_cache(void) > > } > > return -errno; > > } > > - memcpy(block->colo_cache, block->host, > block->used_length); > > } > > } > > > > @@ -3000,19 +3000,36 @@ int colo_init_ram_cache(void) > > > > RAMBLOCK_FOREACH_NOT_IGNORED(block) { > > unsigned long pages = block->max_length >> > TARGET_PAGE_BITS; > > - > > block->bmap = bitmap_new(pages); > > - bitmap_set(block->bmap, 0, pages); > > } > > } > > - ram_state = g_new0(RAMState, 1); > > - ram_state->migration_dirty_pages = 0; > > - qemu_mutex_init(&ram_state->bitmap_mutex); > > - memory_global_dirty_log_start(); > > > > + ram_state_init(&ram_state); > > return 0; > > } > > > > +/* TODO: duplicated with ram_init_bitmaps */ > > +void colo_incoming_start_dirty_log(void) > > +{ > > + RAMBlock *block = NULL; > > + /* For memory_global_dirty_log_start below. */ > > + qemu_mutex_lock_iothread(); > > + qemu_mutex_lock_ramlist(); > > + > > + memory_global_dirty_log_sync(); > > + WITH_RCU_READ_LOCK_GUARD() { > > + RAMBLOCK_FOREACH_NOT_IGNORED(block) { > > + ramblock_sync_dirty_bitmap(ram_state, block); > > + /* Discard this dirty bitmap record */ > > + bitmap_zero(block->bmap, block->max_length >> > TARGET_PAGE_BITS); > > + } > > + memory_global_dirty_log_start(); > > + } > > + ram_state->migration_dirty_pages = 0; > > + qemu_mutex_unlock_ramlist(); > > + qemu_mutex_unlock_iothread(); > > +} > > + > > /* It is need to hold the global lock to call this helper */ > > void colo_release_ram_cache(void) > > { > > @@ -3032,9 +3049,7 @@ void colo_release_ram_cache(void) > > } > > } > > } > > - qemu_mutex_destroy(&ram_state->bitmap_mutex); > > - g_free(ram_state); > > - ram_state = NULL; > > + ram_state_cleanup(&ram_state); > > } > > > > /** > > @@ -3302,7 +3317,6 @@ static void colo_flush_ram_cache(void) > > ramblock_sync_dirty_bitmap(ram_state, block); > > } > > } > > - > > > trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages); > > WITH_RCU_READ_LOCK_GUARD() { > > block = QLIST_FIRST_RCU(&ram_list.blocks); > > @@ -3348,7 +3362,7 @@ static int ram_load_precopy(QEMUFile *f) > > > > while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) { > > ram_addr_t addr, total_ram_bytes; > > - void *host = NULL; > > + void *host = NULL, *host_bak = NULL; > > uint8_t ch; > > > > /* > > @@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f) > > RAM_SAVE_FLAG_COMPRESS_PAGE | > RAM_SAVE_FLAG_XBZRLE)) { > > RAMBlock *block = ram_block_from_stream(f, flags); > > > > + host = host_from_ram_block_offset(block, addr); > > /* > > - * After going into COLO, we should load the Page into > colo_cache. > > + * After going into COLO stage, we should not load the page > > + * into SVM's memory diretly, we put them into colo_cache > firstly. > > + * NOTE: We need to keep a copy of SVM's ram in > colo_cache. > > + * Privously, we copied all these memory in preparing stage > of COLO > > + * while we need to stop VM, which is a time-consuming > process. > > + * Here we optimize it by a trick, back-up every page while > in > > + * migration process while COLO is enabled, though it > affects the > > + * speed of the migration, but it obviously reduce the > downtime of > > + * back-up all SVM'S memory in COLO preparing stage. > > */ > > - if (migration_incoming_in_colo_state()) { > > - host = colo_cache_from_block_offset(block, addr); > > - } else { > > - host = host_from_ram_block_offset(block, addr); > > + if (migration_incoming_colo_enabled()) { > > + if (migration_incoming_in_colo_state()) { > > + /* In COLO stage, put all pages into cache > temporarily */ > > + host = colo_cache_from_block_offset(block, addr); > > + } else { > > + /* > > + * In migration stage but before COLO stage, > > + * Put all pages into both cache and SVM's > memory. > > + */ > > + host_bak = colo_cache_from_block_offset(block, > addr); > > + } > > } > > if (!host) { > > error_report("Illegal RAM offset " RAM_ADDR_FMT, > addr); > > ret = -EINVAL; > > break; > > } > > - > > if (!migration_incoming_in_colo_state()) { > > ramblock_recv_bitmap_set(block, host); > > } > > @@ -3506,6 +3535,9 @@ static int ram_load_precopy(QEMUFile *f) > > if (!ret) { > > ret = qemu_file_get_error(f); > > } > > + if (!ret && host_bak) { > > + memcpy(host_bak, host, TARGET_PAGE_SIZE); > > + } > > } > > > > ret |= wait_for_decompress_done(); > > diff --git a/migration/ram.h b/migration/ram.h > > index a553d40751..5ceaff7cb4 100644 > > --- a/migration/ram.h > > +++ b/migration/ram.h > > @@ -66,5 +66,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, > RAMBlock *rb); > > /* ram cache */ > > int colo_init_ram_cache(void); > > void colo_release_ram_cache(void); > > +void colo_incoming_start_dirty_log(void); > > > > #endif > > -- > > 2.21.0 > > > >