From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52136) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFy77-0006Rm-48 for qemu-devel@nongnu.org; Mon, 26 Jan 2015 23:50:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YFy72-000346-GK for qemu-devel@nongnu.org; Mon, 26 Jan 2015 23:50:57 -0500 Received: from ozlabs.org ([2401:3900:2:1::2]:51969) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFy71-00033Z-Uk for qemu-devel@nongnu.org; Mon, 26 Jan 2015 23:50:52 -0500 Date: Tue, 27 Jan 2015 15:38:11 +1100 From: David Gibson Message-ID: <20150127043811.GB19081@voom.fritz.box> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-38-git-send-email-dgilbert@redhat.com> <20141111011310.GC15270@voom.redhat.com> <20150114201327.GN2298@work-vm> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="R3G7APHDIzY6R/pk" Content-Disposition: inline In-Reply-To: <20150114201327.GN2298@work-vm> 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" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, cristian.klein@cs.umu.se, qemu-devel@nongnu.org, amit.shah@redhat.com, yanghy@cn.fujitsu.com --R3G7APHDIzY6R/pk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jan 14, 2015 at 08:13:27PM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (david@gibson.dropbear.id.au) wrote: > > On Fri, Oct 03, 2014 at 06:47:43PM +0100, Dr. David Alan Gilbert (git) = wrote: > > > 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 scanni= ng. > > >=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 > > > + > > > + /* > > > + * Don't break host-page chunks up with queue items > >=20 > > Why does this matter? >=20 > See the comment you make in a few patches time, it's about being able > to place the pages atomically on the destination. Hmm. But if the destination has to wait for all the pieces of a host page to arrive anyway, does it really make any difference if they're contiguous in the stream? > > > + * 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 hos= t page > > > + */ > > > + if (last_was_from_queue || (!last_sent_block) || > > > + ((last_offset & (hps - 1)) =3D=3D (hps - TARGET_PAGE_SIZ= E))) { > > > + tmpblock =3D ram_save_unqueue_page(ms, &tmpoffset, &bito= ffset); > > > } > > > - 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 bitoffs= et=3D%zx", > > > + __func__, tmpblock->idstr, tmpoffset, bitoffset); > > > + /* We're sending this page, and since it's postcopy noth= ing else > > > + * will dirty it, and we must make sure it doesn't get s= ent again. > > > + */ > > > + if (!migration_bitmap_clear_dirty(bitoffset << TARGET_PA= GE_BITS)) { > >=20 > > 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 > I've changed it to ram_addr_t as requested; it's slightly clearer but the= re > are a few places where we're dealing with the sentmap where we now need t= o shift > the other way. In the end ram_addr_t is really a scaled offset into those > bitmaps. Right, but to someone who isn't deeply familiar with the code, they're more likely to understand what the ram address means than the bitmap offset. --=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 --R3G7APHDIzY6R/pk Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUxxYzAAoJEGw4ysog2bOSfiMP/3oLofe3sO5CIZFcd5DTVzKS qkvXVKm41KhrOGopL5MhbxU+5ilZJ3Uo06JlL/muRghyJMFq6t6dTnVrritmyK4v CtsjIpv+n+0+snODw52oRvwyjsOq2WOgHUSaxxxt5tUyfA/TAOpVzslpPQzJ0hnU o/Qgt95uXuEeSvBA3HCTEnTL3vzjU8LSDxG21Zm8g3fALzrV5SvhMnA8vjwHk0q6 B5QbSgkI2vIL06sRHDRdoOC9mkFinMDZT2fun/hCEvp3P8yZS4B4fd49xdzewhFq 6HYYJ36bCxEO2eJL1hFFumd41G3EOC8L5aOsI6M2cqoP+h+gtNIZFV90WJcpR27w uMkmsLbDd4v76Q3lzMyh1TEWYZGLVxDqmQ6a7zmTUixLnI55jtV9UJiVmx1i8A91 BjnpBa+kXsoZaYOdbwDQ//2+nomwaN7mcNSvP74p1gxZluvxs0AqYdiETkaZbpYG CnGz6f+yPk7auoBb5IwzsMV9eLyyU2uNLJvkCl7hPveYeWhQKtcgWZoztU0WGo9p etmvPVe06zn7RbGI2p18NLiWrLjl9fWTw7IEYOXOmCnzHFipqrp8KCgBMcNgCGQH e4f7OViqoLETkMfhD5WmcrWOnsOF/RGMWwJ067vIEetIMBZsVDotVvq1yONZ81oK N/0/yMh+vhsVECWkjuQi =4qUq -----END PGP SIGNATURE----- --R3G7APHDIzY6R/pk--