From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44035) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xo14d-0000PZ-Le for qemu-devel@nongnu.org; Mon, 10 Nov 2014 21:20:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xo14X-0004oD-Py for qemu-devel@nongnu.org; Mon, 10 Nov 2014 21:20:51 -0500 Received: from ozlabs.org ([103.22.144.67]:40964) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xo14X-0004na-6Q for qemu-devel@nongnu.org; Mon, 10 Nov 2014 21:20:45 -0500 Date: Tue, 11 Nov 2014 12:13:10 +1100 From: David Gibson Message-ID: <20141111011310.GC15270@voom.redhat.com> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-38-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Bu8it7iiRSEf40bY" Content-Disposition: inline In-Reply-To: <1412358473-31398-38-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the post-copy queue 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, lilei@linux.vnet.ibm.com, quintela@redhat.com, cristian.klein@cs.umu.se, qemu-devel@nongnu.org, amit.shah@redhat.com, yanghy@cn.fujitsu.com --Bu8it7iiRSEf40bY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 03, 2014 at 06:47:43PM +0100, Dr. David Alan Gilbert (git) wrot= e: > From: "Dr. David Alan Gilbert" >=20 > When transmitting RAM pages, consume pages that have been queued by > MIG_RPCOMM_REQPAGE commands and send them ahead of normal page scanning. >=20 > Note: > a) After a queued page the linear walk carries on from after the > unqueued page; there is a reasonable chance that the destination > was about to ask for other closeby pages anyway. >=20 > b) We have to be careful of any assumptions that the page walking > code makes, in particular it does some short cuts on its first linear > walk that break as soon as we do a queued page. >=20 > Signed-off-by: Dr. David Alan Gilbert > --- > arch_init.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++----= ------ > 1 file changed, 125 insertions(+), 24 deletions(-) >=20 > diff --git a/arch_init.c b/arch_init.c > index 72f9e17..a945990 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -331,6 +331,7 @@ static RAMBlock *last_seen_block; > /* This is the last block from where we have sent data */ > static RAMBlock *last_sent_block; > static ram_addr_t last_offset; > +static bool last_was_from_queue; > static unsigned long *migration_bitmap; > static uint64_t migration_dirty_pages; > static uint32_t last_version; > @@ -460,6 +461,19 @@ static inline bool migration_bitmap_set_dirty(ram_ad= dr_t addr) > return ret; > } > =20 > +static inline bool migration_bitmap_clear_dirty(ram_addr_t addr) > +{ > + bool ret; > + int nr =3D addr >> TARGET_PAGE_BITS; > + > + ret =3D test_and_clear_bit(nr, migration_bitmap); > + > + if (ret) { > + migration_dirty_pages--; > + } > + return ret; > +} > + > static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t len= gth) > { > ram_addr_t addr; > @@ -660,6 +674,39 @@ static int ram_save_page(QEMUFile *f, RAMBlock* bloc= k, ram_addr_t offset, > } > =20 > /* > + * Unqueue a page from the queue fed by postcopy page requests > + * > + * Returns: The RAMBlock* to transmit from (or NULL if the queue is em= pty) > + * ms: MigrationState in > + * offset: the byte offset within the RAMBlock for the start of the p= age > + * bitoffset: global offset in the dirty/sent bitmaps > + */ > +static RAMBlock *ram_save_unqueue_page(MigrationState *ms, ram_addr_t *o= ffset, > + unsigned long *bitoffset) > +{ > + RAMBlock *result =3D NULL; > + qemu_mutex_lock(&ms->src_page_req_mutex); > + if (!QSIMPLEQ_EMPTY(&ms->src_page_requests)) { > + struct MigrationSrcPageRequest *entry =3D > + QSIMPLEQ_FIRST(&ms->src_page_request= s); > + result =3D entry->rb; > + *offset =3D entry->offset; > + *bitoffset =3D (entry->offset + entry->rb->offset) >> TARGET_PAG= E_BITS; > + > + if (entry->len > TARGET_PAGE_SIZE) { > + entry->len -=3D TARGET_PAGE_SIZE; > + entry->offset +=3D TARGET_PAGE_SIZE; > + } else { > + QSIMPLEQ_REMOVE_HEAD(&ms->src_page_requests, next_req); > + g_free(entry); > + } > + } > + qemu_mutex_unlock(&ms->src_page_req_mutex); > + > + return result; > +} > + > +/* > * 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) > @@ -720,44 +767,97 @@ int ram_save_queue_pages(MigrationState *ms, const = char *rbname, > =20 > static int ram_find_and_save_block(QEMUFile *f, bool last_stage) > { > + MigrationState *ms =3D migrate_get_current(); > RAMBlock *block =3D last_seen_block; > + RAMBlock *tmpblock; > ram_addr_t offset =3D last_offset; > + ram_addr_t tmpoffset; > bool complete_round =3D false; > int bytes_sent =3D 0; > - MemoryRegion *mr; > unsigned long bitoffset; > + unsigned long hps =3D sysconf(_SC_PAGESIZE); > =20 > - if (!block) > + if (!block) { > block =3D QTAILQ_FIRST(&ram_list.blocks); > + last_was_from_queue =3D false; > + } > =20 > - while (true) { > - mr =3D block->mr; > - offset =3D migration_bitmap_find_and_reset_dirty(mr, offset, &bi= toffset); > - if (complete_round && block =3D=3D last_seen_block && > - offset >=3D last_offset) { > - break; > + while (true) { /* Until we send a block or run out of stuff to send = */ > + tmpblock =3D NULL; > + > + /* > + * Don't break host-page chunks up with queue items Why does this matter? > + * so only unqueue if, > + * a) The last item came from the queue anyway > + * b) The last sent item was the last target-page in a host pa= ge > + */ > + if (last_was_from_queue || (!last_sent_block) || > + ((last_offset & (hps - 1)) =3D=3D (hps - TARGET_PAGE_SIZE)))= { > + tmpblock =3D ram_save_unqueue_page(ms, &tmpoffset, &bitoffse= t); > } > - if (offset >=3D block->length) { > - offset =3D 0; > - block =3D QTAILQ_NEXT(block, next); > - if (!block) { > - block =3D QTAILQ_FIRST(&ram_list.blocks); > - complete_round =3D true; > - ram_bulk_stage =3D false; > + > + if (tmpblock) { > + /* We've got a block from the postcopy queue */ > + DPRINTF("%s: Got postcopy item '%s' offset=3D%zx bitoffset= =3D%zx", > + __func__, tmpblock->idstr, tmpoffset, bitoffset); > + /* We're sending this page, and since it's postcopy nothing = else > + * will dirty it, and we must make sure it doesn't get sent = again. > + */ > + if (!migration_bitmap_clear_dirty(bitoffset << TARGET_PAGE_B= ITS)) { Ugh.. that's kind of subtle. I think it would be clearer if you work in terms of a ram_addr_t throughout, rather than "bitoffset" whose meaning is not terribly obvious. --=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 --Bu8it7iiRSEf40bY Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIbBAEBAgAGBQJUYWKmAAoJEGw4ysog2bOS29EP+O1JhgxBV1LXokpN9e+a2fV1 tiuitd2sfp1wCgwUg130qFGHhU19qtHe1Hn4B4GvhgEnpXPncaKB4UCWZvsJY0ZL CoR5Kq2mXRv9oVjKAAR0qoeINFZLLcIB/xnZkD2Stl58yyTpaLvM2S7tNGK2psds EgKPjft0283iXopb9PHY8STcBxVJRKfBBIQHrZmY2/xBDpxe+RsM9Fx8paLXgpvP 7rFhrX7mlOD5XxKzu+9O+YM2i0uT0aR7roilY9TXE8myHS7dNE/SVv/S6H3sRqDc aKy4a9ZkVbt0HOBm2n1zyyP7EjBdNvQn9XOBiBpvY61s4zZ8auZ/hYIcjnosgS5i 8b9/1x0+K7UzjDFCC33U4sNG4ry8TMSMLarKXhJm57QQy2uK0304jOJsnpTCwg7w iS6Ht1juI4QcVyOGeDOMOQu/WNCqQkMxzEpw/i1qixUjkQktHH0NXUjIJu7uligp 5TFOpcvVM2t8u1p3oG9Qi+neNHnpVvlGA9SiJvmZnspOx0HinwJNLnxBKikFNVjc uLr+Ub/DUecFQSrzdWvZJZQ08BMr/CYq6RnoWQ4Hsji6eHpCGounJlW+K6YfJshD f0Mbj9VfE4/t6HBg+4tpF/Qjdj0r0+7pE23CSZqtsdF9b0XOapq+1ABkrNWvlfpe FkROwfPlQVBhTgyrpgA= =uDF6 -----END PGP SIGNATURE----- --Bu8it7iiRSEf40bY--