| Message ID | 20200903152646.93336-6-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/postcopy: Sync faulted addresses after network recovered | expand |
* Peter Xu (peterx@redhat.com) wrote: > We synchronize the requested pages right after a postcopy recovery happens. > This helps to synchronize the prioritized pages on source so that the faulted > threads can be served faster. > > Reported-by: Xiaohui Li <xiaohli@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > migration/savevm.c | 56 ++++++++++++++++++++++++++++++++++++++++++ > migration/trace-events | 1 + > 2 files changed, 57 insertions(+) > > diff --git a/migration/savevm.c b/migration/savevm.c > index 304d98ff78..f998dd230d 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2011,6 +2011,48 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis) > return LOADVM_QUIT; > } > > +/* We must be with page_request_mutex held */ > +static gboolean postcopy_sync_page_req(gpointer key, gpointer value, > + gpointer data) > +{ > + MigrationIncomingState *mis = data; > + void *host_addr = (void *) key; > + ram_addr_t rb_offset; > + RAMBlock *rb; > + int ret; > + > + rb = qemu_ram_block_from_host(host_addr, true, &rb_offset); > + if (!rb) { > + /* > + * This should _never_ happen. However be nice for a migrating VM to > + * not crash/assert. Post an error (note: intended to not use *_once > + * because we do want to see all the illegal addresses; and this can > + * never be triggered by the guest so we're safe) and move on next. > + */ > + error_report("%s: illegal host addr %p", __func__, host_addr); > + /* Try the next entry */ > + return FALSE; > + } > + > + ret = migrate_send_rp_message_req_pages(mis, rb, rb_offset); > + if (ret) { > + /* Refer to above comment - just try our best to continue */ > + error_report("%s: send rp message failed for addr %p", > + __func__, host_addr); > + } > + > + trace_postcopy_page_req_sync((uint64_t)host_addr); > + > + return FALSE; > +} OK, Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> but... > +static void migrate_send_rp_req_pages_pending(MigrationIncomingState *mis) > +{ > + qemu_mutex_lock(&mis->page_request_mutex); > + g_tree_foreach(mis->page_requested, postcopy_sync_page_req, mis); > + qemu_mutex_unlock(&mis->page_request_mutex); could have used the lock macro there. Dave > +} > + > static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis) > { > if (mis->state != MIGRATION_STATUS_POSTCOPY_RECOVER) { > @@ -2033,6 +2075,20 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis) > /* Tell source that "we are ready" */ > migrate_send_rp_resume_ack(mis, MIGRATION_RESUME_ACK_VALUE); > > + /* > + * After a postcopy recovery, the source should have lost the postcopy > + * queue, or potentially the requested pages could have been lost during > + * the network down phase. Let's re-sync with the source VM by re-sending > + * all the pending pages that we eagerly need, so these threads won't get > + * blocked too long due to the recovery. > + * > + * Without this procedure, the faulted destination VM threads (waiting for > + * page requests right before the postcopy is interrupted) can keep hanging > + * until the pages are sent by the source during the background copying of > + * pages, or another thread faulted on the same address accidentally. > + */ > + migrate_send_rp_req_pages_pending(mis); > + > return 0; > } > > diff --git a/migration/trace-events b/migration/trace-events > index b89ce02cb0..54a6dd2761 100644 > --- a/migration/trace-events > +++ b/migration/trace-events > @@ -49,6 +49,7 @@ vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s" > vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s" > postcopy_pause_incoming(void) "" > postcopy_pause_incoming_continued(void) "" > +postcopy_page_req_sync(uint64_t host_addr) "sync page req 0x%"PRIx64 > > # vmstate.c > vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d" > -- > 2.26.2 >
diff --git a/migration/savevm.c b/migration/savevm.c index 304d98ff78..f998dd230d 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2011,6 +2011,48 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis) return LOADVM_QUIT; } +/* We must be with page_request_mutex held */ +static gboolean postcopy_sync_page_req(gpointer key, gpointer value, + gpointer data) +{ + MigrationIncomingState *mis = data; + void *host_addr = (void *) key; + ram_addr_t rb_offset; + RAMBlock *rb; + int ret; + + rb = qemu_ram_block_from_host(host_addr, true, &rb_offset); + if (!rb) { + /* + * This should _never_ happen. However be nice for a migrating VM to + * not crash/assert. Post an error (note: intended to not use *_once + * because we do want to see all the illegal addresses; and this can + * never be triggered by the guest so we're safe) and move on next. + */ + error_report("%s: illegal host addr %p", __func__, host_addr); + /* Try the next entry */ + return FALSE; + } + + ret = migrate_send_rp_message_req_pages(mis, rb, rb_offset); + if (ret) { + /* Refer to above comment - just try our best to continue */ + error_report("%s: send rp message failed for addr %p", + __func__, host_addr); + } + + trace_postcopy_page_req_sync((uint64_t)host_addr); + + return FALSE; +} + +static void migrate_send_rp_req_pages_pending(MigrationIncomingState *mis) +{ + qemu_mutex_lock(&mis->page_request_mutex); + g_tree_foreach(mis->page_requested, postcopy_sync_page_req, mis); + qemu_mutex_unlock(&mis->page_request_mutex); +} + static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis) { if (mis->state != MIGRATION_STATUS_POSTCOPY_RECOVER) { @@ -2033,6 +2075,20 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis) /* Tell source that "we are ready" */ migrate_send_rp_resume_ack(mis, MIGRATION_RESUME_ACK_VALUE); + /* + * After a postcopy recovery, the source should have lost the postcopy + * queue, or potentially the requested pages could have been lost during + * the network down phase. Let's re-sync with the source VM by re-sending + * all the pending pages that we eagerly need, so these threads won't get + * blocked too long due to the recovery. + * + * Without this procedure, the faulted destination VM threads (waiting for + * page requests right before the postcopy is interrupted) can keep hanging + * until the pages are sent by the source during the background copying of + * pages, or another thread faulted on the same address accidentally. + */ + migrate_send_rp_req_pages_pending(mis); + return 0; } diff --git a/migration/trace-events b/migration/trace-events index b89ce02cb0..54a6dd2761 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -49,6 +49,7 @@ vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s" vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s" postcopy_pause_incoming(void) "" postcopy_pause_incoming_continued(void) "" +postcopy_page_req_sync(uint64_t host_addr) "sync page req 0x%"PRIx64 # vmstate.c vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
We synchronize the requested pages right after a postcopy recovery happens. This helps to synchronize the prioritized pages on source so that the faulted threads can be served faster. Reported-by: Xiaohui Li <xiaohli@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/savevm.c | 56 ++++++++++++++++++++++++++++++++++++++++++ migration/trace-events | 1 + 2 files changed, 57 insertions(+)