* Peter Xu (pet...@redhat.com) wrote:
> On Fri, Feb 16, 2018 at 01:16:16PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > 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 <dgilb...@redhat.com>
> > ---
> > 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?
The 'waker' function we call knows that, we don't; see the
'vhost_user_postcopy_waker' in the next patch, and it hunts down whether
the address the waker is called for is one it's responsible for.
Also note that a shared page might be shared between multiple other
programs - not just one. In our case that could be two vhost-user
devices wired to two separate processes.
> 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.
Hmm can you explain that a bit more?
> 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 <pet...@redhat.com>
Thanks.
Dave
>
> > /*
> > * 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
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
