* Peter Xu (pet...@redhat.com) wrote: > On Mon, Mar 05, 2018 at 07:55:13PM +0000, Dr. David Alan Gilbert wrote: > > * 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. > > For vhost-user devices, they should be always responsible for mostly > all RAM exported on the guest? If so, they will always be notified to > wake up if a page is copied?
Right; but this patch isn't vhost-user specific; this is more general. > Here I was thinking not only about responsible ranges - It was about > whether each PostcopyFD could note down the faulted addresses that > were waiting to be service. Then when we do the wake up, we could > possibly skip notifying the PostcopyFD when the copied page is not > covering any of the faulted addresses on that PostcopyFD? Yes, that would be possible - in this case I made that the job of the device that had registered (i.e. the waker method) rather than the core postcopy code. > > 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. > > Yeah, but the idea still stands IMHO - we can notify only those > PostcopyFDs that have faulted on the page already and skip the rest. > For sure there can be more than one candidate for the wakeup, since > there can be multiple PostcopyFDs that captured page fault on the same > page (or even, same address). > > > > > > 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? > > Basically above was what I thought - to record the faulted addresses > with specific PostcopyFD when page fault happened, then we may know > which page(s) will a PostcopyFD need. But when with that, we'll > possibly need a lock to protect the information (or any other sync > method). OK, but I think you're suggesting building a whole new data structure to know which ones need notifying; that sounds like a lot of extra complexity for not much gain. Dave > (Hope I didn't miss anything important along the way) > > Thanks, > > -- > Peter Xu -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK