[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 13/13] migration/ram: Tolerate partially changed mappings
From: |
Peter Xu |
Subject: |
Re: [PATCH v2 13/13] migration/ram: Tolerate partially changed mappings in postcopy code |
Date: |
Mon, 24 Feb 2020 17:49:49 -0500 |
On Fri, Feb 21, 2020 at 05:42:04PM +0100, David Hildenbrand wrote:
> When we partially change mappings (esp., mmap over parts of an existing
> mmap like qemu_ram_remap() does) where we have a userfaultfd handler
> registered, the handler will implicitly be unregistered from the parts that
> changed.
>
> Trying to place pages onto mappings where there is no longer a handler
> registered will fail. Let's make sure that any waiter is woken up - we
> have to do that manually.
>
> Let's also document how UFFDIO_UNREGISTER will handle this scenario.
>
> This is mainly a preparation for RAM blocks with resizable allcoations,
> where the mapping of the invalid RAM range will change. The source will
> keep sending pages that are outside of the new (shrunk) RAM size. We have
> to treat these pages like they would have been migrated, but can
> essentially simply drop the content (ignore the placement error).
>
> Keep printing a warning on EINVAL, to avoid hiding other (programming)
> issues. ENOENT is unique.
>
> Cc: "Dr. David Alan Gilbert" <address@hidden>
> Cc: Juan Quintela <address@hidden>
> Cc: Peter Xu <address@hidden>
> Cc: Andrea Arcangeli <address@hidden>
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
> migration/postcopy-ram.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index c68caf4e42..f023830b9a 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -506,6 +506,12 @@ 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
> + * (e.g., 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.
> + */
Ideally we should still only unregister what we have registered.
After all we do have this information because we know what we
registered, we know what has unmapped (in your new resize() hook, when
postcopy_state==RUNNING).
An extreme example is when we register with pages in range [A, B),
then shrink it to [A, C), then we mapped something else within [C, B)
(note, with virtio-mem logically B can be very big and C can be very
small, it means [B, C) can cover quite some address space). Then if:
- [C, B) memory type is not compatible with uffd, or
- [C, B) could be registered with uffd again due to some other
reason (so far QEMU should not have such a reason)
Then the unregister could fail or misbehave, IMHO. Another benefit is
that...
> if (ioctl(mis->userfault_fd, UFFDIO_UNREGISTER, &range_struct)) {
> error_report("%s: userfault unregister %s", __func__,
> strerror(errno));
>
> @@ -1180,6 +1186,17 @@ int postcopy_ram_incoming_setup(MigrationIncomingState
> *mis)
> return 0;
> }
>
> +static int qemu_ufd_wake_ioctl(int userfault_fd, void *host_addr,
> + uint64_t pagesize)
> +{
> + struct uffdio_range range = {
> + .start = (uint64_t)(uintptr_t)host_addr,
> + .len = pagesize,
> + };
> +
> + return ioctl(userfault_fd, UFFDIO_WAKE, &range);
> +}
> +
> static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> void *from_addr, uint64_t pagesize, RAMBlock
> *rb)
> {
> @@ -1198,6 +1215,26 @@ static int qemu_ufd_copy_ioctl(int userfault_fd, void
> *host_addr,
> zero_struct.mode = 0;
> ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> }
> +
> + /*
> + * When the mapping gets partially changed (e.g., qemu_ram_remap())
> before
> + * we try to place a page, the userfaultfd handler will be removed for
> the
> + * changed mappings and placing pages will fail. We can safely ignore
> this,
> + * because mappings that changed on the destination don't need data from
> the
> + * source (e.g., qemu_ram_remap()). Wake up any waiter waiting for that
> page
> + * (unlikely but possible). Waking up waiters is always possible, even
> + * without a registered userfaultfd handler.
> + *
> + * Old kernels report EINVAL, new kernels report ENOENT in case there is
> + * no longer a userfaultfd handler for a mapping.
> + */
> + if (ret && (errno == ENOENT || errno == EINVAL)) {
> + if (errno == EINVAL) {
> + warn_report("%s: Failed to place page %p. Waking up any
> waiters.",
> + __func__, host_addr);
> + }
> + ret = qemu_ufd_wake_ioctl(userfault_fd, host_addr, pagesize);
... if with above information (takes notes on where we registered
uffd), I think we don't need to capture error, but we can simply skip
those outliers.
Thanks,
> + }
> if (!ret) {
> ramblock_recv_bitmap_set_range(rb, host_addr,
> pagesize / qemu_target_page_size());
> --
> 2.24.1
>
--
Peter Xu
- [PATCH v2 08/13] migration/ram: Simplify host page handling in ram_load_postcopy(), (continued)
- [PATCH v2 08/13] migration/ram: Simplify host page handling in ram_load_postcopy(), David Hildenbrand, 2020/02/21
- [PATCH v2 07/13] migration/ram: Discard RAM when growing RAM blocks after ram_postcopy_incoming_init(), David Hildenbrand, 2020/02/21
- [PATCH v2 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy(), David Hildenbrand, 2020/02/21
- [PATCH v2 10/13] migration/ram: Handle RAM block resizes during postcopy, David Hildenbrand, 2020/02/21
- [PATCH v2 11/13] migration/multifd: Print used_length of memory block, David Hildenbrand, 2020/02/21
- [PATCH v2 12/13] migration/ram: Use offset_in_ramblock() in range checks, David Hildenbrand, 2020/02/21
- [PATCH v2 13/13] migration/ram: Tolerate partially changed mappings in postcopy code, David Hildenbrand, 2020/02/21
- Re: [PATCH v2 13/13] migration/ram: Tolerate partially changed mappings in postcopy code,
Peter Xu <=
Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, Peter Xu, 2020/02/21
- Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, David Hildenbrand, 2020/02/24
- Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, Peter Xu, 2020/02/24
- Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, David Hildenbrand, 2020/02/24
- Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, David Hildenbrand, 2020/02/24
- Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, Peter Xu, 2020/02/24
- Re: [PATCH v2 00/13] migrate/ram: Fix resizing RAM blocks while migrating, David Hildenbrand, 2020/02/24