* Peter Xu (pet...@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 <pet...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilb...@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 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK