On 19.02.20 17:17, David Hildenbrand wrote: > When we partially change mappings (e.g., mmap over parts of an existing > mmap) where we have a userfaultfd handler registered, the handler will > implicitly be unregistered from the parts that changed. This is e.g., the > case when doing a qemu_ram_remap(), but is also a preparation for RAM > blocks with resizable allocations and we're shrinking RAM blocks. > > When the mapping is changed and the handler is removed, any waiters are > woken up. Trying to place pages will fail. We can simply ignore erors > due to that when placing pages - as the mapping changed on the migration > destination, also the content is stale. E.g., after shrinking a RAM > block, nobody should be using that memory. After doing a > qemu_ram_remap(), the old memory is expected to have vanished. > > Let's tolerate such errors (but still warn for now) when placing pages. > Also, add a comment why unregistering will continue to work even though > the mapping might have changed. > > Cc: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > Cc: Juan Quintela <quint...@redhat.com> > Cc: Peter Xu <pet...@redhat.com> > Cc: Andrea Arcangeli <aarca...@redhat.com> > Signed-off-by: David Hildenbrand <da...@redhat.com> > --- > migration/postcopy-ram.c | 43 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 6 deletions(-) > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index c68caf4e42..df9d27c004 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -506,6 +506,13 @@ static int cleanup_range(RAMBlock *rb, void *opaque) > range_struct.start = (uintptr_t)host_addr; > range_struct.len = length; > > + /* > + * In case the mapping was partially changed since we enabled userfault > + * (esp. when whrinking RAM blocks and we have resizable allocations, or > + * via qemu_ram_remap()), the userfaultfd handler was already removed for > + * the mappings that changed. Unregistering will, however, still work and > + * ignore mappings without a registered handler. > + */ > if (ioctl(mis->userfault_fd, UFFDIO_UNREGISTER, &range_struct)) { > error_report("%s: userfault unregister %s", __func__, > strerror(errno)); > > @@ -1239,10 +1246,28 @@ int postcopy_place_page(MigrationIncomingState *mis, > void *host, void *from, > */ > if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) { > int e = errno; > - error_report("%s: %s copy host: %p from: %p (size: %zd)", > - __func__, strerror(e), host, from, pagesize); > > - return -e; > + /* > + * When the mapping gets partially changed before we try to place a > page > + * (esp. when whrinking RAM blocks and we have resizable > allocations, or > + * via qemu_ram_remap()), the userfaultfd handler will be removed and > + * placing pages will fail. In that case, any waiter was already > woken > + * up when the mapping was changed. We can safely ignore this, as > + * mappings that change once we're running on the destination imply > + * that memory of these mappings vanishes. Let's still print a > warning > + * for now. > + *
After talking to Andrea, on mapping changes, no waiter will be woken up automatically. We have to do an UFFDIO_WAKE, which even works when there is no longer a handler registered for that reason. Interesting stuff :) -- Thanks, David / dhildenb