From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:57028)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1YapFY-0006tM-M5
	for qemu-devel@nongnu.org; Wed, 25 Mar 2015 13:37:53 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1YapFV-00039x-6e
	for qemu-devel@nongnu.org; Wed, 25 Mar 2015 13:37:52 -0400
Received: from mx1.redhat.com ([209.132.183.28]:33163)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1YapFU-00039o-Uq
	for qemu-devel@nongnu.org; Wed, 25 Mar 2015 13:37:49 -0400
Date: Wed, 25 Mar 2015 17:37:34 +0000
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Message-ID: <20150325173733.GJ2313@work-vm>
References: <1424883128-9841-1-git-send-email-dgilbert@redhat.com>
	<1424883128-9841-34-git-send-email-dgilbert@redhat.com>
	<20150324015307.GX25043@voom.fritz.box>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150324015307.GX25043@voom.fritz.box>
Subject: Re: [Qemu-devel] [PATCH v5 33/45] Page request: Process incoming
	page request
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: David Gibson <david@gibson.dropbear.id.au>
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

* David Gibson (david@gibson.dropbear.id.au) wrote:
> On Wed, Feb 25, 2015 at 04:51:56PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > On receiving MIG_RPCOMM_REQ_PAGES look up the address and
> > queue the page.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  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(-)
> > 
> > diff --git a/arch_init.c b/arch_init.c
> > index d2c4457..9d8fc6b 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c

<snip>

> >  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 = getpagesize();
> > +    if (start & (our_host_ps-1)) {
> > +        long roundings = start & (our_host_ps-1);
> > +        start -= roundings;
> > +        len += roundings;
> > +    }
> > +    if (len & (our_host_ps-1)) {
> > +        long roundings = len & (our_host_ps-1);
> > +        len -= roundings;
> > +        len += 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.

In principal the request you get from the destination should already
be nicely aligned; but of course you can't actually trust it, so you
have to at least test for alignment.

Since the code has to send whole host pages to keep the
destination happy, it expects the requests that come out of the queue
to be host page aligned.

At the moment we're only supporting matching page sizes, if we wanted
to support mismatches then it probably needs to round to the size of
destination host page sizes.

Dave

> > +    if (ram_save_queue_pages(ms, rbname, start, len)) {
> > +        source_return_path_bad(ms);
> > +    }
> >  }
> >  
> >  /*
> > 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"
> >  
> >  # 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 nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")"
> >  migrate_send_rp_message(int cmd, uint16_t len) "cmd=%d, len=%d"
> > -migrate_handle_rp_req_pages(size_t start, size_t len) "at %zx for len %zx"
> > +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) ""
> 
> -- 
> 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


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK