[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC 20/21] migration: Handle page faults using UFFDIO_CONTINU
From: |
Juan Quintela |
Subject: |
Re: [PATCH RFC 20/21] migration: Handle page faults using UFFDIO_CONTINUE |
Date: |
Wed, 01 Feb 2023 20:24:40 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Peter Xu <peterx@redhat.com> wrote:
> Teach QEMU to be able to handle page faults using UFFDIO_CONTINUE for
> hugetlbfs double mapped ranges.
>
> To copy the data, we need to use the mirror buffer created per ramblock by
> a raw memcpy(), then we can kick the faulted threads using UFFDIO_CONTINUE
> by installing the pgtables.
>
> Move trace_postcopy_place_page(host) upper so that it'll dump something for
> either UFFDIO_COPY or UFFDIO_CONTINUE.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/postcopy-ram.c | 55 ++++++++++++++++++++++++++++++++++++++--
> migration/trace-events | 4 +--
> 2 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 8a2259581e..c4bd338e22 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1350,6 +1350,43 @@ int postcopy_notify_shared_wake(RAMBlock *rb, uint64_t
> offset)
> return 0;
> }
>
> +/* Returns the mirror_host addr for a specific host address in ramblock */
> +static inline void *migration_ram_get_mirror_addr(RAMBlock *rb, void *host)
> +{
> + return (void *)((__u64)rb->host_mirror + ((__u64)host -
> (__u64)rb->host));
This is gross :-(
I hate this C miss-feature.
What about:
return (char *)rb->host_mirror + (char*)host - (char*)rb->host;
But I don't know if it (much) clearer. And no, I don't remember if we
ever need more parents.
gcc used to do "the right" thing on void * arithmetic, but it is not in
the standard, and I don't know what is worse.
> +}
> +
> +static int
> +qemu_uffd_continue(MigrationIncomingState *mis, RAMBlock *rb, void *host,
> + void *from)
> +{
> + void *mirror_addr = migration_ram_get_mirror_addr(rb, host);
> + /* Doublemap uses small host page size */
> + uint64_t psize = qemu_real_host_page_size();
> + struct uffdio_continue req;
> +
> + /*
> + * Copy data first into the mirror host pointer; we can't directly copy
> + * data into rb->host because otherwise our thread will get trapped too.
> + */
> + memcpy(mirror_addr, from, psize);
> +
> + /* Kick off the faluted threads to fetch data from the page cache
^^^^^^^
> */
Faulted
> + req.range.start = (__u64)host;
> + req.range.len = psize;
> + req.mode = 0;
> + if (ioctl(mis->userfault_fd, UFFDIO_CONTINUE, &req)) {
> + error_report("%s: UFFDIO_CONTINUE failed for start=%p"
> + " len=0x%"PRIx64": %s\n", __func__, host,
> + psize, strerror(-req.mapped));
> + return req.mapped;
> + }
> +
> + postcopy_mark_received(mis, rb, host, psize / qemu_target_page_size());
> +
> + return 0;
> +}
> +
> /*
> * Place a host page (from) at (host) atomically
> * returns 0 on success
> @@ -1359,6 +1396,18 @@ int postcopy_place_page(MigrationIncomingState *mis,
> void *host, void *from,
> {
> size_t pagesize = migration_ram_pagesize(rb);
>
> + trace_postcopy_place_page(rb->idstr, (uint8_t *)host - rb->host, host);
> +
> + if (postcopy_use_minor_fault(rb)) {
> + /*
> + * If minor fault used, we use UFFDIO_CONTINUE instead.
> + *
> + * TODO: support shared uffds (e.g. vhost-user). Currently we're
> + * skipping them.
> + */
> + return qemu_uffd_continue(mis, rb, host, from);
> + }
> +
> /* copy also acks to the kernel waking the stalled thread up
> * TODO: We can inhibit that ack and only do it if it was requested
> * which would be slightly cheaper, but we'd have to be careful
> @@ -1372,7 +1421,6 @@ int postcopy_place_page(MigrationIncomingState *mis,
> void *host, void *from,
> return -e;
> }
>
> - trace_postcopy_place_page(host);
> return postcopy_notify_shared_wake(rb,
> qemu_ram_block_host_offset(rb, host));
> }
> @@ -1385,10 +1433,13 @@ int postcopy_place_page_zero(MigrationIncomingState
> *mis, void *host,
> RAMBlock *rb)
> {
> size_t pagesize = migration_ram_pagesize(rb);
> - trace_postcopy_place_page_zero(host);
> + trace_postcopy_place_page_zero(rb->idstr, (uint8_t *)host - rb->host,
> host);
It is me, or to be standard compliant, you need to cast also rb->host?
> /* Normal RAMBlocks can zero a page using UFFDIO_ZEROPAGE
> * but it's not available for everything (e.g. hugetlbpages)
> + *
> + * NOTE: when hugetlb double-map enabled, then this ramblock will never
> + * have RAM_UF_ZEROPAGE, so it'll always go to postcopy_place_page().
> */
> if (qemu_ram_is_uf_zeroable(rb)) {
> if (qemu_ufd_copy_ioctl(mis, host, NULL, pagesize, rb)) {
> diff --git a/migration/trace-events b/migration/trace-events
> index 6b418a0e9e..7baf235d22 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -265,8 +265,8 @@ postcopy_discard_send_range(const char *ramblock,
> unsigned long start, unsigned
> postcopy_cleanup_range(const char *ramblock, void *host_addr, size_t offset,
> size_t length) "%s: %p offset=0x%zx length=0x%zx"
> postcopy_init_range(const char *ramblock, void *host_addr, size_t offset,
> size_t length) "%s: %p offset=0x%zx length=0x%zx"
> postcopy_nhp_range(const char *ramblock, void *host_addr, size_t offset,
> size_t length) "%s: %p offset=0x%zx length=0x%zx"
> -postcopy_place_page(void *host_addr) "host=%p"
> -postcopy_place_page_zero(void *host_addr) "host=%p"
> +postcopy_place_page(const char *id, size_t offset, void *host_addr) "id=%s
> offset=0x%zx host=%p"
> +postcopy_place_page_zero(const char *id, size_t offset, void *host_addr)
> "id=%s offset=0x%zx host=%p"
> postcopy_ram_enable_notify(void) ""
> mark_postcopy_blocktime_begin(uint64_t addr, void *dd, uint32_t time, int
> cpu, int received) "addr: 0x%" PRIx64 ", dd: %p, time: %u, cpu: %d,
> already_received: %d"
> mark_postcopy_blocktime_end(uint64_t addr, void *dd, uint32_t time, int
> affected_cpu) "addr: 0x%" PRIx64 ", dd: %p, time: %u, affected_cpu: %d"
I think that you can split the part of the patch that changes the
traces. But again, it is up to you.
- Re: [PATCH RFC 20/21] migration: Handle page faults using UFFDIO_CONTINUE,
Juan Quintela <=