Re: [Qemu-devel] [PATCH v3 20/29] postcopy: postcopy_notify_shared_wake

[Peter Xu]

On Fri, Feb 16, 2018 at 01:16:16PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> Add a hook to allow a client userfaultfd to be 'woken'
> when a page arrives, and a walker that calls that
> hook for relevant clients given a RAMBlock and offset.
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  migration/postcopy-ram.c | 16 ++++++++++++++++
>  migration/postcopy-ram.h | 10 ++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 67deae7e1c..879711968c 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -824,6 +824,22 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void 
> *host_addr,
>      return ret;
>  }
>  
> +int postcopy_notify_shared_wake(RAMBlock *rb, uint64_t offset)
> +{
> +    int i;
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    GArray *pcrfds = mis->postcopy_remote_fds;
> +
> +    for (i = 0; i < pcrfds->len; i++) {
> +        struct PostCopyFD *cur = &g_array_index(pcrfds, struct PostCopyFD, 
> i);
> +        int ret = cur->waker(cur, rb, offset);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +    return 0;
> +}
> +

We should know that which FD needs what pages, right?  If with that
information, we can only notify the ones who have page faulted on
exactly the same page?  Otherwise we do UFFDIO_WAKE once for each
client when a page is ready, even if the clients have not page faulted
at all?

But for the first version, I think it's fine.  And I believe if we
maintain the faulted addresses we need some way to sync between the
wake thread and fault thread too.  And I totally have no idea on how
this difference will be any kind of bottle neck at all, since I guess
the network link should still be the postcopy bottleneck considering
that 10g is mostly what we have now (or even, 1g).

Reviewed-by: Peter Xu <address@hidden>

>  /*
>   * Place a host page (from) at (host) atomically
>   * returns 0 on success
> diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> index 2e3dd844d5..2b71cf958e 100644
> --- a/migration/postcopy-ram.h
> +++ b/migration/postcopy-ram.h
> @@ -146,6 +146,10 @@ struct PostCopyFD;
>  
>  /* ufd is a pointer to the struct uffd_msg *TODO: more Portable! */
>  typedef int (*pcfdhandler)(struct PostCopyFD *pcfd, void *ufd);
> +/* Notification to wake, either on place or on reception of
> + * a fault on something that's already arrived (race)
> + */
> +typedef int (*pcfdwake)(struct PostCopyFD *pcfd, RAMBlock *rb, uint64_t 
> offset);
>  
>  struct PostCopyFD {
>      int fd;
> @@ -153,6 +157,8 @@ struct PostCopyFD {
>      void *data;
>      /* Handler to be called whenever we get a poll event */
>      pcfdhandler handler;
> +    /* Notification to wake shared client */
> +    pcfdwake waker;
>      /* A string to use in error messages */
>      const char *idstr;
>  };
> @@ -162,6 +168,10 @@ struct PostCopyFD {
>   */
>  void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
>  void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
> +/* Call each of the shared 'waker's registerd telling them of
> + * availability of a block.
> + */
> +int postcopy_notify_shared_wake(RAMBlock *rb, uint64_t offset);
>  /* Notify a client ufd that a page is available
>   * Note: The 'client_address' is in the address space of the client
>   * program not QEMU
> -- 
> 2.14.3
> 

-- 
Peter Xu