Skip to content

Commit

Permalink
migration: Send requested page directly in rp-return thread
Browse files Browse the repository at this point in the history
With all the facilities ready, send the requested page directly in the
rp-return thread rather than queuing it in the request queue, if and only
if postcopy preempt is enabled.  It can achieve so because it uses separate
channel for sending urgent pages.  The only shared data is bitmap and it's
protected by the bitmap_mutex.

Note that since we're moving the ownership of the urgent channel from the
migration thread to rp thread it also means the rp thread is responsible
for managing the qemufile, e.g. properly close it when pausing migration
happens.  For this, let migration_release_from_dst_file to cover shutdown
of the urgent channel too, renaming it as migration_release_dst_files() to
better show what it does.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
  • Loading branch information
xzpeter authored and Juan Quintela committed Nov 15, 2022
1 parent 366e80d commit d03beba
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 16 deletions.
35 changes: 19 additions & 16 deletions migration/migration.c
Expand Up @@ -2868,8 +2868,11 @@ static int migrate_handle_rp_resume_ack(MigrationState *s, uint32_t value)
return 0;
}

/* Release ms->rp_state.from_dst_file in a safe way */
static void migration_release_from_dst_file(MigrationState *ms)
/*
* Release ms->rp_state.from_dst_file (and postcopy_qemufile_src if
* existed) in a safe way.
*/
static void migration_release_dst_files(MigrationState *ms)
{
QEMUFile *file;

Expand All @@ -2882,6 +2885,18 @@ static void migration_release_from_dst_file(MigrationState *ms)
ms->rp_state.from_dst_file = NULL;
}

/*
* Do the same to postcopy fast path socket too if there is. No
* locking needed because this qemufile should only be managed by
* return path thread.
*/
if (ms->postcopy_qemufile_src) {
migration_ioc_unregister_yank_from_file(ms->postcopy_qemufile_src);
qemu_file_shutdown(ms->postcopy_qemufile_src);
qemu_fclose(ms->postcopy_qemufile_src);
ms->postcopy_qemufile_src = NULL;
}

qemu_fclose(file);
}

Expand Down Expand Up @@ -3026,7 +3041,7 @@ static void *source_return_path_thread(void *opaque)
* Maybe there is something we can do: it looks like a
* network down issue, and we pause for a recovery.
*/
migration_release_from_dst_file(ms);
migration_release_dst_files(ms);
rp = NULL;
if (postcopy_pause_return_path_thread(ms)) {
/*
Expand All @@ -3044,7 +3059,7 @@ static void *source_return_path_thread(void *opaque)
}

trace_source_return_path_thread_end();
migration_release_from_dst_file(ms);
migration_release_dst_files(ms);
rcu_unregister_thread();
return NULL;
}
Expand Down Expand Up @@ -3567,18 +3582,6 @@ static MigThrError postcopy_pause(MigrationState *s)
qemu_file_shutdown(file);
qemu_fclose(file);

/*
* Do the same to postcopy fast path socket too if there is. No
* locking needed because no racer as long as we do this before setting
* status to paused.
*/
if (s->postcopy_qemufile_src) {
migration_ioc_unregister_yank_from_file(s->postcopy_qemufile_src);
qemu_file_shutdown(s->postcopy_qemufile_src);
qemu_fclose(s->postcopy_qemufile_src);
s->postcopy_qemufile_src = NULL;
}

migrate_set_state(&s->state, s->state,
MIGRATION_STATUS_POSTCOPY_PAUSED);

Expand Down
112 changes: 112 additions & 0 deletions migration/ram.c
Expand Up @@ -574,6 +574,8 @@ static QemuThread *decompress_threads;
static QemuMutex decomp_done_lock;
static QemuCond decomp_done_cond;

static int ram_save_host_page_urgent(PageSearchStatus *pss);

static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
ram_addr_t offset, uint8_t *source_buf);

Expand All @@ -588,6 +590,16 @@ static void pss_init(PageSearchStatus *pss, RAMBlock *rb, ram_addr_t page)
pss->complete_round = false;
}

/*
* Check whether two PSSs are actively sending the same page. Return true
* if it is, false otherwise.
*/
static bool pss_overlap(PageSearchStatus *pss1, PageSearchStatus *pss2)
{
return pss1->host_page_sending && pss2->host_page_sending &&
(pss1->host_page_start == pss2->host_page_start);
}

static void *do_data_compress(void *opaque)
{
CompressParam *param = opaque;
Expand Down Expand Up @@ -2288,6 +2300,57 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len)
return -1;
}

/*
* When with postcopy preempt, we send back the page directly in the
* rp-return thread.
*/
if (postcopy_preempt_active()) {
ram_addr_t page_start = start >> TARGET_PAGE_BITS;
size_t page_size = qemu_ram_pagesize(ramblock);
PageSearchStatus *pss = &ram_state->pss[RAM_CHANNEL_POSTCOPY];
int ret = 0;

qemu_mutex_lock(&rs->bitmap_mutex);

pss_init(pss, ramblock, page_start);
/*
* Always use the preempt channel, and make sure it's there. It's
* safe to access without lock, because when rp-thread is running
* we should be the only one who operates on the qemufile
*/
pss->pss_channel = migrate_get_current()->postcopy_qemufile_src;
pss->postcopy_requested = true;
assert(pss->pss_channel);

/*
* It must be either one or multiple of host page size. Just
* assert; if something wrong we're mostly split brain anyway.
*/
assert(len % page_size == 0);
while (len) {
if (ram_save_host_page_urgent(pss)) {
error_report("%s: ram_save_host_page_urgent() failed: "
"ramblock=%s, start_addr=0x"RAM_ADDR_FMT,
__func__, ramblock->idstr, start);
ret = -1;
break;
}
/*
* NOTE: after ram_save_host_page_urgent() succeeded, pss->page
* will automatically be moved and point to the next host page
* we're going to send, so no need to update here.
*
* Normally QEMU never sends >1 host page in requests, so
* logically we don't even need that as the loop should only
* run once, but just to be consistent.
*/
len -= page_size;
};
qemu_mutex_unlock(&rs->bitmap_mutex);

return ret;
}

struct RAMSrcPageRequest *new_entry =
g_new0(struct RAMSrcPageRequest, 1);
new_entry->rb = ramblock;
Expand Down Expand Up @@ -2565,6 +2628,55 @@ static void pss_host_page_finish(PageSearchStatus *pss)
pss->host_page_start = pss->host_page_end = 0;
}

/*
* Send an urgent host page specified by `pss'. Need to be called with
* bitmap_mutex held.
*
* Returns 0 if save host page succeeded, false otherwise.
*/
static int ram_save_host_page_urgent(PageSearchStatus *pss)
{
bool page_dirty, sent = false;
RAMState *rs = ram_state;
int ret = 0;

trace_postcopy_preempt_send_host_page(pss->block->idstr, pss->page);
pss_host_page_prepare(pss);

/*
* If precopy is sending the same page, let it be done in precopy, or
* we could send the same page in two channels and none of them will
* receive the whole page.
*/
if (pss_overlap(pss, &ram_state->pss[RAM_CHANNEL_PRECOPY])) {
trace_postcopy_preempt_hit(pss->block->idstr,
pss->page << TARGET_PAGE_BITS);
return 0;
}

do {
page_dirty = migration_bitmap_clear_dirty(rs, pss->block, pss->page);

if (page_dirty) {
/* Be strict to return code; it must be 1, or what else? */
if (ram_save_target_page(rs, pss) != 1) {
error_report_once("%s: ram_save_target_page failed", __func__);
ret = -1;
goto out;
}
sent = true;
}
pss_find_next_dirty(pss);
} while (pss_within_range(pss));
out:
pss_host_page_finish(pss);
/* For urgent requests, flush immediately if sent */
if (sent) {
qemu_fflush(pss->pss_channel);
}
return ret;
}

/**
* ram_save_host_page: save a whole host page
*
Expand Down

0 comments on commit d03beba

Please sign in to comment.