| Message ID | 20220119080929.39485-5-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Postcopy Preemption | expand |
* Peter Xu (peterx@redhat.com) wrote: > Add a helper to detect whether postcopy has pending request. > > Since at it, cleanup the code a bit, e.g. in unqueue_page() we shouldn't need > to check it again on queue empty because we're the only one (besides cleanup > code, which should never run during this process) that will take a request off > the list, so the request list can only grow but not shrink under the hood. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/ram.c | 45 ++++++++++++++++++++++++++++----------------- > 1 file changed, 28 insertions(+), 17 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 94b0ad4234..dc6ba041fa 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -354,6 +354,12 @@ static RAMState *ram_state; > > static NotifierWithReturnList precopy_notifier_list; > > +/* Whether postcopy has queued requests? */ > +static bool postcopy_has_request(RAMState *rs) > +{ > + return !QSIMPLEQ_EMPTY_ATOMIC(&rs->src_page_requests); > +} > + > void precopy_infrastructure_init(void) > { > notifier_with_return_list_init(&precopy_notifier_list); > @@ -1533,28 +1539,33 @@ static bool find_dirty_block(RAMState *rs, PageSearchStatus *pss, bool *again) > */ > static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset) > { > + struct RAMSrcPageRequest *entry; > RAMBlock *block = NULL; > > - if (QSIMPLEQ_EMPTY_ATOMIC(&rs->src_page_requests)) { > + if (!postcopy_has_request(rs)) { > return NULL; > } > > QEMU_LOCK_GUARD(&rs->src_page_req_mutex); > - if (!QSIMPLEQ_EMPTY(&rs->src_page_requests)) { > - struct RAMSrcPageRequest *entry = > - QSIMPLEQ_FIRST(&rs->src_page_requests); > - block = entry->rb; > - *offset = entry->offset; > - > - if (entry->len > TARGET_PAGE_SIZE) { > - entry->len -= TARGET_PAGE_SIZE; > - entry->offset += TARGET_PAGE_SIZE; > - } else { > - memory_region_unref(block->mr); > - QSIMPLEQ_REMOVE_HEAD(&rs->src_page_requests, next_req); > - g_free(entry); > - migration_consume_urgent_request(); > - } > + > + /* > + * This should _never_ change even after we take the lock, because no one > + * should be taking anything off the request list other than us. > + */ > + assert(postcopy_has_request(rs)); > + > + entry = QSIMPLEQ_FIRST(&rs->src_page_requests); > + block = entry->rb; > + *offset = entry->offset; > + > + if (entry->len > TARGET_PAGE_SIZE) { > + entry->len -= TARGET_PAGE_SIZE; > + entry->offset += TARGET_PAGE_SIZE; > + } else { > + memory_region_unref(block->mr); > + QSIMPLEQ_REMOVE_HEAD(&rs->src_page_requests, next_req); > + g_free(entry); > + migration_consume_urgent_request(); > } > > return block; > @@ -2996,7 +3007,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) > t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > i = 0; > while ((ret = qemu_file_rate_limit(f)) == 0 || > - !QSIMPLEQ_EMPTY(&rs->src_page_requests)) { > + postcopy_has_request(rs)) { > int pages; > > if (qemu_file_get_error(f)) { > -- > 2.32.0 >
Peter Xu <peterx@redhat.com> wrote: > Add a helper to detect whether postcopy has pending request. > > Since at it, cleanup the code a bit, e.g. in unqueue_page() we shouldn't need > to check it again on queue empty because we're the only one (besides cleanup > code, which should never run during this process) that will take a request off > the list, so the request list can only grow but not shrink under the hood. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> queued
diff --git a/migration/ram.c b/migration/ram.c index 94b0ad4234..dc6ba041fa 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -354,6 +354,12 @@ static RAMState *ram_state; static NotifierWithReturnList precopy_notifier_list; +/* Whether postcopy has queued requests? */ +static bool postcopy_has_request(RAMState *rs) +{ + return !QSIMPLEQ_EMPTY_ATOMIC(&rs->src_page_requests); +} + void precopy_infrastructure_init(void) { notifier_with_return_list_init(&precopy_notifier_list); @@ -1533,28 +1539,33 @@ static bool find_dirty_block(RAMState *rs, PageSearchStatus *pss, bool *again) */ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset) { + struct RAMSrcPageRequest *entry; RAMBlock *block = NULL; - if (QSIMPLEQ_EMPTY_ATOMIC(&rs->src_page_requests)) { + if (!postcopy_has_request(rs)) { return NULL; } QEMU_LOCK_GUARD(&rs->src_page_req_mutex); - if (!QSIMPLEQ_EMPTY(&rs->src_page_requests)) { - struct RAMSrcPageRequest *entry = - QSIMPLEQ_FIRST(&rs->src_page_requests); - block = entry->rb; - *offset = entry->offset; - - if (entry->len > TARGET_PAGE_SIZE) { - entry->len -= TARGET_PAGE_SIZE; - entry->offset += TARGET_PAGE_SIZE; - } else { - memory_region_unref(block->mr); - QSIMPLEQ_REMOVE_HEAD(&rs->src_page_requests, next_req); - g_free(entry); - migration_consume_urgent_request(); - } + + /* + * This should _never_ change even after we take the lock, because no one + * should be taking anything off the request list other than us. + */ + assert(postcopy_has_request(rs)); + + entry = QSIMPLEQ_FIRST(&rs->src_page_requests); + block = entry->rb; + *offset = entry->offset; + + if (entry->len > TARGET_PAGE_SIZE) { + entry->len -= TARGET_PAGE_SIZE; + entry->offset += TARGET_PAGE_SIZE; + } else { + memory_region_unref(block->mr); + QSIMPLEQ_REMOVE_HEAD(&rs->src_page_requests, next_req); + g_free(entry); + migration_consume_urgent_request(); } return block; @@ -2996,7 +3007,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); i = 0; while ((ret = qemu_file_rate_limit(f)) == 0 || - !QSIMPLEQ_EMPTY(&rs->src_page_requests)) { + postcopy_has_request(rs)) { int pages; if (qemu_file_get_error(f)) {
Add a helper to detect whether postcopy has pending request. Since at it, cleanup the code a bit, e.g. in unqueue_page() we shouldn't need to check it again on queue empty because we're the only one (besides cleanup code, which should never run during this process) that will take a request off the list, so the request list can only grow but not shrink under the hood. Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/ram.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-)