From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43159) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaEMe-0002X1-2e for qemu-devel@nongnu.org; Mon, 23 Mar 2015 22:14:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaEMY-0000xv-Pv for qemu-devel@nongnu.org; Mon, 23 Mar 2015 22:14:44 -0400 Received: from ozlabs.org ([103.22.144.67]:48295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaEMY-0000xb-4m for qemu-devel@nongnu.org; Mon, 23 Mar 2015 22:14:38 -0400 Date: Tue, 24 Mar 2015 12:53:07 +1100 From: David Gibson Message-ID: <20150324015307.GX25043@voom.fritz.box> References: <1424883128-9841-1-git-send-email-dgilbert@redhat.com> <1424883128-9841-34-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="95puKF9o0QNvO5PE" Content-Disposition: inline In-Reply-To: <1424883128-9841-34-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 33/45] Page request: Process incoming page request List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, qemu-devel@nongnu.org, amit.shah@redhat.com, pbonzini@redhat.com, yanghy@cn.fujitsu.com --95puKF9o0QNvO5PE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 25, 2015 at 04:51:56PM +0000, Dr. David Alan Gilbert (git) wrot= e: > From: "Dr. David Alan Gilbert" >=20 > On receiving MIG_RPCOMM_REQ_PAGES look up the address and > queue the page. >=20 > Signed-off-by: Dr. David Alan Gilbert > --- > arch_init.c | 55 +++++++++++++++++++++++++++++++++++++= ++++++ > include/exec/cpu-all.h | 2 -- > include/migration/migration.h | 21 +++++++++++++++++ > include/qemu/typedefs.h | 1 + > migration/migration.c | 33 +++++++++++++++++++++++++- > trace-events | 3 ++- > 6 files changed, 111 insertions(+), 4 deletions(-) >=20 > diff --git a/arch_init.c b/arch_init.c > index d2c4457..9d8fc6b 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -669,6 +669,61 @@ static int ram_save_page(QEMUFile *f, RAMBlock* bloc= k, ram_addr_t offset, > } > =20 > /* > + * Queue the pages for transmission, e.g. a request from postcopy destin= ation > + * ms: MigrationStatus in which the queue is held > + * rbname: The RAMBlock the request is for - may be NULL (to mean reus= e last) > + * start: Offset from the start of the RAMBlock > + * len: Length (in bytes) to send > + * Return: 0 on success > + */ > +int ram_save_queue_pages(MigrationState *ms, const char *rbname, > + ram_addr_t start, ram_addr_t len) > +{ > + RAMBlock *ramblock; > + > + if (!rbname) { > + /* Reuse last RAMBlock */ > + ramblock =3D ms->last_req_rb; > + > + if (!ramblock) { > + /* > + * Shouldn't happen, we can't reuse the last RAMBlock if > + * it's the 1st request. > + */ > + error_report("ram_save_queue_pages no previous block"); > + return -1; > + } > + } else { > + ramblock =3D ram_find_block(rbname); > + > + if (!ramblock) { > + /* We shouldn't be asked for a non-existent RAMBlock */ > + error_report("ram_save_queue_pages no block '%s'", rbname); > + return -1; > + } > + } > + trace_ram_save_queue_pages(ramblock->idstr, start, len); > + if (start+len > ramblock->used_length) { > + error_report("%s request overrun start=3D%zx len=3D%zx blocklen= =3D%zx", > + __func__, start, len, ramblock->used_length); > + return -1; > + } > + > + struct MigrationSrcPageRequest *new_entry =3D > + g_malloc0(sizeof(struct MigrationSrcPageRequest)); > + new_entry->rb =3D ramblock; > + new_entry->offset =3D start; > + new_entry->len =3D len; > + ms->last_req_rb =3D ramblock; > + > + qemu_mutex_lock(&ms->src_page_req_mutex); > + QSIMPLEQ_INSERT_TAIL(&ms->src_page_requests, new_entry, next_req); > + qemu_mutex_unlock(&ms->src_page_req_mutex); > + > + return 0; > +} > + > +/* > * ram_find_and_save_block: Finds a page to send and sends it to f > * > * Returns: The number of bytes written. > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h > index 2c48286..3088000 100644 > --- a/include/exec/cpu-all.h > +++ b/include/exec/cpu-all.h > @@ -265,8 +265,6 @@ CPUArchState *cpu_copy(CPUArchState *env); > =20 > /* memory API */ > =20 > -typedef struct RAMBlock RAMBlock; > - > struct RAMBlock { > struct MemoryRegion *mr; > uint8_t *host; > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 2c15d63..b1c7cad 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -100,6 +100,18 @@ MigrationIncomingState *migration_incoming_get_curre= nt(void); > MigrationIncomingState *migration_incoming_state_new(QEMUFile *f); > void migration_incoming_state_destroy(void); > =20 > +/* > + * An outstanding page request, on the source, having been received > + * and queued > + */ > +struct MigrationSrcPageRequest { > + RAMBlock *rb; > + hwaddr offset; > + hwaddr len; > + > + QSIMPLEQ_ENTRY(MigrationSrcPageRequest) next_req; > +}; > + > struct MigrationState > { > int64_t bandwidth_limit; > @@ -142,6 +154,12 @@ struct MigrationState > * of the postcopy phase > */ > unsigned long *sentmap; > + > + /* Queue of outstanding page requests from the destination */ > + QemuMutex src_page_req_mutex; > + QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_r= equests; > + /* The RAMBlock used in the last src_page_request */ > + RAMBlock *last_req_rb; > }; > =20 > void process_incoming_migration(QEMUFile *f); > @@ -276,6 +294,9 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t = block_offset, > ram_addr_t offset, size_t size, > int *bytes_sent); > =20 > +int ram_save_queue_pages(MigrationState *ms, const char *rbname, > + ram_addr_t start, ram_addr_t len); > + > PostcopyState postcopy_state_get(MigrationIncomingState *mis); > =20 > /* Set the state and return the old state */ > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > index 0651275..396044d 100644 > --- a/include/qemu/typedefs.h > +++ b/include/qemu/typedefs.h > @@ -73,6 +73,7 @@ typedef struct QEMUSGList QEMUSGList; > typedef struct QEMUSizedBuffer QEMUSizedBuffer; > typedef struct QEMUTimerListGroup QEMUTimerListGroup; > typedef struct QEMUTimer QEMUTimer; > +typedef struct RAMBlock RAMBlock; > typedef struct Range Range; > typedef struct SerialState SerialState; > typedef struct SHPCDevice SHPCDevice; > diff --git a/migration/migration.c b/migration/migration.c > index 2e9d0dd..939f426 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -26,6 +26,8 @@ > #include "qemu/thread.h" > #include "qmp-commands.h" > #include "trace.h" > +#include "exec/memory.h" > +#include "exec/address-spaces.h" > =20 > enum MigrationPhase { > MIG_STATE_ERROR =3D -1, > @@ -495,6 +497,15 @@ static void migrate_fd_cleanup(void *opaque) > =20 > migrate_fd_cleanup_src_rp(s); > =20 > + /* This queue generally should be empty - but in the case of a failed > + * migration might have some droppings in. > + */ > + struct MigrationSrcPageRequest *mspr, *next_mspr; > + QSIMPLEQ_FOREACH_SAFE(mspr, &s->src_page_requests, next_req, next_ms= pr) { > + QSIMPLEQ_REMOVE_HEAD(&s->src_page_requests, next_req); > + g_free(mspr); > + } > + > if (s->file) { > trace_migrate_fd_cleanup(); > qemu_mutex_unlock_iothread(); > @@ -613,6 +624,9 @@ MigrationState *migrate_init(const MigrationParams *p= arams) > s->state =3D MIG_STATE_SETUP; > trace_migrate_set_state(MIG_STATE_SETUP); > =20 > + qemu_mutex_init(&s->src_page_req_mutex); > + QSIMPLEQ_INIT(&s->src_page_requests); > + > s->total_time =3D qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > return s; > } > @@ -826,7 +840,24 @@ static void source_return_path_bad(MigrationState *s) > static void migrate_handle_rp_req_pages(MigrationState *ms, const char* = rbname, > ram_addr_t start, ram_addr_t len) > { > - trace_migrate_handle_rp_req_pages(start, len); > + trace_migrate_handle_rp_req_pages(rbname, start, len); > + > + /* Round everything up to our host page size */ > + long our_host_ps =3D getpagesize(); > + if (start & (our_host_ps-1)) { > + long roundings =3D start & (our_host_ps-1); > + start -=3D roundings; > + len +=3D roundings; > + } > + if (len & (our_host_ps-1)) { > + long roundings =3D len & (our_host_ps-1); > + len -=3D roundings; > + len +=3D our_host_ps; > + } Why is it necessary to round out to host page size on the source? I understand why the host page size is relevant on the destination, due to the userfaultfd and atomic populate constraints, but not on the source. > + if (ram_save_queue_pages(ms, rbname, start, len)) { > + source_return_path_bad(ms); > + } > } > =20 > /* > diff --git a/trace-events b/trace-events > index 9bedee4..8a0d70d 100644 > --- a/trace-events > +++ b/trace-events > @@ -1218,6 +1218,7 @@ migration_bitmap_sync_start(void) "" > migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64"" > migration_throttle(void) "" > ram_postcopy_send_discard_bitmap(void) "" > +ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: = start: %zx len: %zx" > =20 > # hw/display/qxl.c > disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d" > @@ -1404,7 +1405,7 @@ migrate_fd_error(void) "" > migrate_fd_cancel(void) "" > migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t non= post) "pending size %" PRIu64 " max %" PRIu64 " (post=3D%" PRIu64 " nonpost= =3D%" PRIu64 ")" > migrate_send_rp_message(int cmd, uint16_t len) "cmd=3D%d, len=3D%d" > -migrate_handle_rp_req_pages(size_t start, size_t len) "at %zx for len %z= x" > +migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len= ) "in %s at %zx len %zx" > migration_thread_after_loop(void) "" > migration_thread_file_err(void) "" > migration_thread_setup_complete(void) "" --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --95puKF9o0QNvO5PE Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVEMODAAoJEGw4ysog2bOSUcoP/1mwWbxJ1+RsZPDcHPqA57gs upMvbOgEpWg7UjdIEuahryNnkvGb+fOOWtGGwLwYT0ID1tkvxy+94NUgv/l4btKW jWXeKRrqQVPShlfOUthxFtl7+R8TFCtb6ZAodItpkRmVT4PqhoXjdtVjt4uFHN90 dZ7Q7OphlE1hR9piNsn9t++QZRO/ZfgEopgbR9bMVn0ilOE5X4+J82WLLzEUnJ2c PV2kngDP6xZ02precWSjAFD8HlHCtR/ET3aJ3moaxur7Q4Je3gcYAvAmadqGLvpU vlMnNon995ttfmhahdK+zoA4zLs5rkQkNmU0k6J1joLTqBpitIOJg/Zj6ZmsioYl JniBHB1IGm2FgZXDISr9l7IbqxU24ERokPvpEcBZM7EmHUEmtZ1R7bKw7hfBtXxF LiO8G4jeSLCPoFgkyDLmUOAydjg6HO3nXVuYfNllmO85HTSBK8SoZRGCZeNClrdt YjQP4bMjTA1OEdHsJv35UtnVNaccW1UhP8kWryxi9qFqYSVoWBTQmGd+mHAhAGGT l8xbUaVZtOYm+uAmh3k5QCbl68vggoeY5ZRYYTP0EknuGpR1nsx2qKNOicrvLupy nu5ySh2X1iKg8RRh13G6lpLnZ0PuPDA1uqCPICot9i1LyN1uYlz8CJcNts+4Q1WV QW31/fSFoog/p17EMMwe =mIRc -----END PGP SIGNATURE----- --95puKF9o0QNvO5PE--