From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGLI4-0003ef-KQ for qemu-devel@nongnu.org; Wed, 28 Jan 2015 00:35:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGLI3-0001Vl-DK for qemu-devel@nongnu.org; Wed, 28 Jan 2015 00:35:48 -0500 Received: from ozlabs.org ([2401:3900:2:1::2]:56737) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGLI2-0001VX-Pd for qemu-devel@nongnu.org; Wed, 28 Jan 2015 00:35:47 -0500 Date: Wed, 28 Jan 2015 16:33:46 +1100 From: David Gibson Message-ID: <20150128053346.GC14681@voom> 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> <20150127043811.GB19081@voom.fritz.box> <20150127094010.GA2425@work-vm> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="H8ygTp4AXg6deix2" Content-Disposition: inline In-Reply-To: <20150127094010.GA2425@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 --H8ygTp4AXg6deix2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 27, 2015 at 09:40:12AM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (david@gibson.dropbear.id.au) wrote: > > 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 (g= it) 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 sc= anning. > > > > >=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 walki= ng > > > > > code makes, in particular it does some short cuts on its first li= near > > > > > 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. > >=20 > > 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? >=20 > The problem is knowing where to put the arriving target-pages until you've > got a full host-page; you've got to put the arriving TP into a temporary > until you have the full set, if they're not contiguous in the stream > then you have to have multiple temporarys dealing with the set of outstan= ding > host pages that you've not got the full set for; and you've still got to = be > careful on the sending side to have a bounded-number of host-pages on the= run > at any time. Making that bound 1 makes the code simpler. Ah, right, I see your point. > > > > > + * 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 page > > > > > + */ > > > > > + 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, &= bitoffset); > > > > > } > > > > > - 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 bit= offset=3D%zx", > > > > > + __func__, tmpblock->idstr, tmpoffset, bitoff= set); > > > > > + /* We're sending this page, and since it's postcopy = nothing else > > > > > + * will dirty it, and we must make sure it doesn't g= et sent again. > > > > > + */ > > > > > + if (!migration_bitmap_clear_dirty(bitoffset << TARGE= T_PAGE_BITS)) { > > > >=20 > > > > Ugh.. that's kind of subtle. I think it would be clearer if you wo= rk > > > > 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= there > > > are a few places where we're dealing with the sentmap where we now ne= ed to shift > > > the other way. In the end ram_addr_t is really a scaled offset into = those > > > bitmaps. > >=20 > > 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 > Fair enough. >=20 > Dave >=20 > >=20 >=20 >=20 --=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 --H8ygTp4AXg6deix2 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUyHS6AAoJEGw4ysog2bOSuQEP/0Ad78csgWZbr7pDCmJNwo0T ZjHHaDCt2WWuKJYyh8QHJR+W8nDdreus1Jl9pyuGGddvmxmq79uJUlF/R627CR8r sIFyDS2v0L2HC1V4hs8o4EazRFxBIDNZKfW5f3qYegGE0AVmduojCcPMJDEggAdW aTdmIGznuJ5z/jv8Rb4NXShUuBNPh49PLViL4bIygQ4Gl+dolZKF//vOkvWRh7cg 7GK+qoparrTpjvK6Y6Mr453asTPaKKpG5ELq4L4dWyfu1bxSPdtusPs2aewmufv5 JYzZhlMU2In45+4qZ6MT1On7xM5PNkXVnQHE/jvVoMz9wZvqHFs2Z8JbUmhKqoxa yXHDZugC6hIy+8RhV+bwSf772+XTw6C52ZDyztItvxPN8uB8iGaqDP3bYw8P6uS0 mgkrSUpZjob0YL0Yhfc1xwqcb5nCWbKO+gqu1w7BPN8YhWNn8tXwx/x7ILkQYs0O t5wwO/HcXVztUBqMBy3fZaIBHD1S3ce9MLmynCSP0D+PcWPJSBYtsvTZOMkOTVyk lpLpbJZvxW33cnKWbzgInB9Dk3cewlnjJfKKhaHQtR7vVQpvbNDGprVhxeUu/IeF HPqjedZGlV38E/tC3qmrs2QxC9Pz/6Sbl+Sje/jLN6WWVwzzI3dsCTjw+lwSuvcK bUZONHHsWQGaggOW55Q2 =1nv6 -----END PGP SIGNATURE----- --H8ygTp4AXg6deix2--