Re: [Qemu-devel] [PATCH v5 33/45] Page request: Process incoming page request

[Dr. David Alan Gilbert]

* David Gibson (address@hidden) wrote:
> On Wed, Feb 25, 2015 at 04:51:56PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > On receiving MIG_RPCOMM_REQ_PAGES look up the address and
> > queue the page.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  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 / address@hidden / Manchester, UK