From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Send a 'wake' request on a userfaultfd for a shared process.
The address in the clients address space is specified together
with the RAMBlock it was resolved to.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/postcopy-ram.c | 26 ++++++++++++++++++++++++++
 migration/postcopy-ram.h |  6 ++++++
 migration/trace-events   |  1 +
 3 files changed, 33 insertions(+)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 277ff749a0..67deae7e1c 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -534,6 +534,25 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
     return 0;
 }
 
+int postcopy_wake_shared(struct PostCopyFD *pcfd,
+                         uint64_t client_addr,
+                         RAMBlock *rb)
+{
+    size_t pagesize = qemu_ram_pagesize(rb);
+    struct uffdio_range range;
+    int ret;
+    trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
+    range.start = client_addr & ~(pagesize - 1);
+    range.len = pagesize;
+    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
+    if (ret) {
+        error_report("%s: Failed to wake: %zx in %s (%s)",
+                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
+                     strerror(errno));
+    }
+    return ret;
+}
+
 /*
  * Callback from shared fault handlers to ask for a page,
  * the page must be specified by a RAMBlock and an offset in that rb
@@ -951,6 +970,13 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
     return NULL;
 }
 
+int postcopy_wake_shared(struct PostCopyFD *pcfd,
+                         uint64_t client_addr,
+                         RAMBlock *rb)
+{
+    assert(0);
+    return -1;
+}
 #endif
 
 /* ------------------------------------------------------------------------- */
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 4c63f20df4..2e3dd844d5 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -162,6 +162,12 @@ struct PostCopyFD {
  */
 void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
 void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
+/* Notify a client ufd that a page is available
+ * Note: The 'client_address' is in the address space of the client
+ * program not QEMU
+ */
+int postcopy_wake_shared(struct PostCopyFD *pcfd, uint64_t client_addr,
+                         RAMBlock *rb);
 /* Callback from shared fault handlers to ask for a page */
 int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
                                  uint64_t client_addr, uint64_t offset);
diff --git a/migration/trace-events b/migration/trace-events
index 7c910b5479..b0acaaa8a0 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -199,6 +199,7 @@ postcopy_ram_incoming_cleanup_entry(void) ""
 postcopy_ram_incoming_cleanup_exit(void) ""
 postcopy_ram_incoming_cleanup_join(void) ""
 postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset 0x%"PRIx64
+postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64" in %s"
 
 save_xbzrle_page_skipping(void) ""
 save_xbzrle_page_overflow(void) ""
-- 
2.14.3

On Fri, Feb 16, 2018 at 01:16:15PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Send a 'wake' request on a userfaultfd for a shared process.
> The address in the clients address space is specified together
> with the RAMBlock it was resolved to.

I think it's "providing a helper to send WAKE to uffd" rather than
really sending it.

Otherwise it looks good to me.  Thanks,

> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/postcopy-ram.c | 26 ++++++++++++++++++++++++++
>  migration/postcopy-ram.h |  6 ++++++
>  migration/trace-events   |  1 +
>  3 files changed, 33 insertions(+)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 277ff749a0..67deae7e1c 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -534,6 +534,25 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
>      return 0;
>  }
>  
> +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> +                         uint64_t client_addr,
> +                         RAMBlock *rb)
> +{
> +    size_t pagesize = qemu_ram_pagesize(rb);
> +    struct uffdio_range range;
> +    int ret;
> +    trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> +    range.start = client_addr & ~(pagesize - 1);
> +    range.len = pagesize;
> +    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
> +    if (ret) {
> +        error_report("%s: Failed to wake: %zx in %s (%s)",
> +                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
> +                     strerror(errno));
> +    }
> +    return ret;
> +}
> +
>  /*
>   * Callback from shared fault handlers to ask for a page,
>   * the page must be specified by a RAMBlock and an offset in that rb
> @@ -951,6 +970,13 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>      return NULL;
>  }
>  
> +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> +                         uint64_t client_addr,
> +                         RAMBlock *rb)
> +{
> +    assert(0);
> +    return -1;
> +}
>  #endif
>  
>  /* ------------------------------------------------------------------------- */
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index 4c63f20df4..2e3dd844d5 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -162,6 +162,12 @@ struct PostCopyFD {
>   */
>  void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
>  void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
> +/* Notify a client ufd that a page is available
> + * Note: The 'client_address' is in the address space of the client
> + * program not QEMU
> + */
> +int postcopy_wake_shared(struct PostCopyFD *pcfd, uint64_t client_addr,
> +                         RAMBlock *rb);
>  /* Callback from shared fault handlers to ask for a page */
>  int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
>                                   uint64_t client_addr, uint64_t offset);
> diff --git a/migration/trace-events b/migration/trace-events
> index 7c910b5479..b0acaaa8a0 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -199,6 +199,7 @@ postcopy_ram_incoming_cleanup_entry(void) ""
>  postcopy_ram_incoming_cleanup_exit(void) ""
>  postcopy_ram_incoming_cleanup_join(void) ""
>  postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset 0x%"PRIx64
> +postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64" in %s"
>  
>  save_xbzrle_page_skipping(void) ""
>  save_xbzrle_page_overflow(void) ""
> -- 
> 2.14.3
> 

-- 
Peter Xu

* Peter Xu (peterx@redhat.com) wrote:
> On Fri, Feb 16, 2018 at 01:16:15PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Send a 'wake' request on a userfaultfd for a shared process.
> > The address in the clients address space is specified together
> > with the RAMBlock it was resolved to.
> 
> I think it's "providing a helper to send WAKE to uffd" rather than
> really sending it.
> 
> Otherwise it looks good to me.  Thanks,

Reworded to:

postcopy: helper for waking shared

Provide a helper to send a 'wake' request on a userfaultfd for
a shared process.  
The address in the clients address space is specified together
with the RAMBlock it was resolved to.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  migration/postcopy-ram.c | 26 ++++++++++++++++++++++++++
> >  migration/postcopy-ram.h |  6 ++++++
> >  migration/trace-events   |  1 +
> >  3 files changed, 33 insertions(+)
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 277ff749a0..67deae7e1c 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -534,6 +534,25 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
> >      return 0;
> >  }
> >  
> > +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> > +                         uint64_t client_addr,
> > +                         RAMBlock *rb)
> > +{
> > +    size_t pagesize = qemu_ram_pagesize(rb);
> > +    struct uffdio_range range;
> > +    int ret;
> > +    trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> > +    range.start = client_addr & ~(pagesize - 1);
> > +    range.len = pagesize;
> > +    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
> > +    if (ret) {
> > +        error_report("%s: Failed to wake: %zx in %s (%s)",
> > +                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
> > +                     strerror(errno));
> > +    }
> > +    return ret;
> > +}
> > +
> >  /*
> >   * Callback from shared fault handlers to ask for a page,
> >   * the page must be specified by a RAMBlock and an offset in that rb
> > @@ -951,6 +970,13 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> >      return NULL;
> >  }
> >  
> > +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> > +                         uint64_t client_addr,
> > +                         RAMBlock *rb)
> > +{
> > +    assert(0);
> > +    return -1;
> > +}
> >  #endif
> >  
> >  /* ------------------------------------------------------------------------- */
> > diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> > index 4c63f20df4..2e3dd844d5 100644
> > --- a/migration/postcopy-ram.h
> > +++ b/migration/postcopy-ram.h
> > @@ -162,6 +162,12 @@ struct PostCopyFD {
> >   */
> >  void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
> >  void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
> > +/* Notify a client ufd that a page is available
> > + * Note: The 'client_address' is in the address space of the client
> > + * program not QEMU
> > + */
> > +int postcopy_wake_shared(struct PostCopyFD *pcfd, uint64_t client_addr,
> > +                         RAMBlock *rb);
> >  /* Callback from shared fault handlers to ask for a page */
> >  int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
> >                                   uint64_t client_addr, uint64_t offset);
> > diff --git a/migration/trace-events b/migration/trace-events
> > index 7c910b5479..b0acaaa8a0 100644
> > --- a/migration/trace-events
> > +++ b/migration/trace-events
> > @@ -199,6 +199,7 @@ postcopy_ram_incoming_cleanup_entry(void) ""
> >  postcopy_ram_incoming_cleanup_exit(void) ""
> >  postcopy_ram_incoming_cleanup_join(void) ""
> >  postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset 0x%"PRIx64
> > +postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64" in %s"
> >  
> >  save_xbzrle_page_skipping(void) ""
> >  save_xbzrle_page_overflow(void) ""
> > -- 
> > 2.14.3
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Hi

On Fri, Feb 16, 2018 at 2:16 PM, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Send a 'wake' request on a userfaultfd for a shared process.
> The address in the clients address space is specified together
> with the RAMBlock it was resolved to.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  migration/postcopy-ram.c | 26 ++++++++++++++++++++++++++
>  migration/postcopy-ram.h |  6 ++++++
>  migration/trace-events   |  1 +
>  3 files changed, 33 insertions(+)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 277ff749a0..67deae7e1c 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -534,6 +534,25 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
>      return 0;
>  }
>
> +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> +                         uint64_t client_addr,
> +                         RAMBlock *rb)
> +{
> +    size_t pagesize = qemu_ram_pagesize(rb);
> +    struct uffdio_range range;
> +    int ret;
> +    trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> +    range.start = client_addr & ~(pagesize - 1);
> +    range.len = pagesize;
> +    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
> +    if (ret) {
> +        error_report("%s: Failed to wake: %zx in %s (%s)",
> +                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
> +                     strerror(errno));
> +    }
> +    return ret;
> +}
> +
>  /*
>   * Callback from shared fault handlers to ask for a page,
>   * the page must be specified by a RAMBlock and an offset in that rb
> @@ -951,6 +970,13 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>      return NULL;
>  }
>
> +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> +                         uint64_t client_addr,
> +                         RAMBlock *rb)
> +{
> +    assert(0);
> +    return -1;
> +}
>  #endif
>
>  /* ------------------------------------------------------------------------- */
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index 4c63f20df4..2e3dd844d5 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -162,6 +162,12 @@ struct PostCopyFD {
>   */
>  void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
>  void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
> +/* Notify a client ufd that a page is available
> + * Note: The 'client_address' is in the address space of the client
> + * program not QEMU
> + */

Any reason not to follow the classic function declaration / API
documentation style:
/**
 * func:
 * @arg: blah
 *
 * Lorem ipsum...
 */

(future documentation tooling will hopefully parse it etc)

> +int postcopy_wake_shared(struct PostCopyFD *pcfd, uint64_t client_addr,
> +                         RAMBlock *rb);
>  /* Callback from shared fault handlers to ask for a page */
>  int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
>                                   uint64_t client_addr, uint64_t offset);
> diff --git a/migration/trace-events b/migration/trace-events
> index 7c910b5479..b0acaaa8a0 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -199,6 +199,7 @@ postcopy_ram_incoming_cleanup_entry(void) ""
>  postcopy_ram_incoming_cleanup_exit(void) ""
>  postcopy_ram_incoming_cleanup_join(void) ""
>  postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset 0x%"PRIx64
> +postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64" in %s"
>
>  save_xbzrle_page_skipping(void) ""
>  save_xbzrle_page_overflow(void) ""
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

* Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> Hi
> 
> On Fri, Feb 16, 2018 at 2:16 PM, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Send a 'wake' request on a userfaultfd for a shared process.
> > The address in the clients address space is specified together
> > with the RAMBlock it was resolved to.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks.

> 
> > ---
> >  migration/postcopy-ram.c | 26 ++++++++++++++++++++++++++
> >  migration/postcopy-ram.h |  6 ++++++
> >  migration/trace-events   |  1 +
> >  3 files changed, 33 insertions(+)
> >
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 277ff749a0..67deae7e1c 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -534,6 +534,25 @@ static int ram_block_enable_notify(const char *block_name, void *host_addr,
> >      return 0;
> >  }
> >
> > +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> > +                         uint64_t client_addr,
> > +                         RAMBlock *rb)
> > +{
> > +    size_t pagesize = qemu_ram_pagesize(rb);
> > +    struct uffdio_range range;
> > +    int ret;
> > +    trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> > +    range.start = client_addr & ~(pagesize - 1);
> > +    range.len = pagesize;
> > +    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
> > +    if (ret) {
> > +        error_report("%s: Failed to wake: %zx in %s (%s)",
> > +                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
> > +                     strerror(errno));
> > +    }
> > +    return ret;
> > +}
> > +
> >  /*
> >   * Callback from shared fault handlers to ask for a page,
> >   * the page must be specified by a RAMBlock and an offset in that rb
> > @@ -951,6 +970,13 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
> >      return NULL;
> >  }
> >
> > +int postcopy_wake_shared(struct PostCopyFD *pcfd,
> > +                         uint64_t client_addr,
> > +                         RAMBlock *rb)
> > +{
> > +    assert(0);
> > +    return -1;
> > +}
> >  #endif
> >
> >  /* ------------------------------------------------------------------------- */
> > diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> > index 4c63f20df4..2e3dd844d5 100644
> > --- a/migration/postcopy-ram.h
> > +++ b/migration/postcopy-ram.h
> > @@ -162,6 +162,12 @@ struct PostCopyFD {
> >   */
> >  void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
> >  void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
> > +/* Notify a client ufd that a page is available
> > + * Note: The 'client_address' is in the address space of the client
> > + * program not QEMU
> > + */
> 
> Any reason not to follow the classic function declaration / API
> documentation style:
> /**
>  * func:
>  * @arg: blah
>  *
>  * Lorem ipsum...
>  */
> 
> (future documentation tooling will hopefully parse it etc)

I've added it; it's not something I generally do except for big external
interfaces.

Dave

> 
> > +int postcopy_wake_shared(struct PostCopyFD *pcfd, uint64_t client_addr,
> > +                         RAMBlock *rb);
> >  /* Callback from shared fault handlers to ask for a page */
> >  int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
> >                                   uint64_t client_addr, uint64_t offset);
> > diff --git a/migration/trace-events b/migration/trace-events
> > index 7c910b5479..b0acaaa8a0 100644
> > --- a/migration/trace-events
> > +++ b/migration/trace-events
> > @@ -199,6 +199,7 @@ postcopy_ram_incoming_cleanup_entry(void) ""
> >  postcopy_ram_incoming_cleanup_exit(void) ""
> >  postcopy_ram_incoming_cleanup_join(void) ""
> >  postcopy_request_shared_page(const char *sharer, const char *rb, uint64_t rb_offset) "for %s in %s offset 0x%"PRIx64
> > +postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64" in %s"
> >
> >  save_xbzrle_page_skipping(void) ""
> >  save_xbzrle_page_overflow(void) ""
> > --
> > 2.14.3
> >
> >
> 
> 
> 
> -- 
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK