On 2026/8/25 03:16, Peter Xu wrote: > On Mon, Aug 24, 2026 at 08:26:56PM +0800, hongmianquan wrote: >> With postcopy-preempt enabled, a postcopy migration of a guest with a >> vhost-user device can hang at the very end on the destination, after all >> pages are transferred (query-migrate: status=postcopy-active, remaining=0). >> >> Root cause is an add/del key mismatch on mis->page_requested: >> >> - add: a backend fault goes through postcopy_request_shared_page() -> >> postcopy_request_page() -> migrate_send_rp_req_pages(), which inserts >> the request and bumps page_requested_count keyed by client_addr. >> - del: qemu_ufd_copy_ioctl() removes the entry and drops the counter >> keyed by this QEMU process's host address for the page. >> >> client_addr is a VA in the external vhost-user backend's address space and >> never equals QEMU's host address, so the removal misses and the counter >> leaks. postcopy_ram_incoming_cleanup() then waits for it to reach zero >> forever, which also stalls the source (it waits for the return path). >> >> Racing threads: >> dst: postcopy_ram_listen_thread -> postcopy_ram_incoming_cleanup -> >> qemu_cond_wait_impl (waits for page_requested_count==0) >> (postcopy_preempt_thread already placed/woke the pages) >> src: migration_thread -> migration_completion -> >> await_return_path_close_on_source -> qemu_thread_join >> (source_return_path_thread blocked in recvmsg) >> >> The leak is only triggered when the vhost-user backend faults on a page >> that has not been received yet: only then does the request take the >> shared-fault slow path and register an entry in page_requested. If that >> page is subsequently delivered while the request is still outstanding, >> its entry is never removed. (Pages already present when the backend >> faults just take the wake path and never register.) So a run may leak >> only a few entries (9 of ~244k requests in our repro) yet still hang. >> >> Fix: key the request with the same host address the removal uses >> (rb->host + aligned_rbo) instead of client_addr. The backend wake still >> happens at placement time via postcopy_wake_shared(). >> >> Signed-off-by: hongmianquan <[email protected]> >> --- >> migration/postcopy-ram.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c >> index a3314d3180..98268b3c55 100644 >> --- a/migration/postcopy-ram.c >> +++ b/migration/postcopy-ram.c >> @@ -992,7 +992,20 @@ int postcopy_request_shared_page(struct PostCopyFD >> *pcfd, RAMBlock *rb, >> return postcopy_wake_shared(pcfd, client_addr, rb); >> } >> /* TODO: support blocktime tracking */ >> - postcopy_request_page(mis, rb, aligned_rbo, client_addr, 0); >> + >> + /* >> + * The page will be placed by qemu_ufd_copy_ioctl(), which removes the >> + * matching entry from mis->page_requested (and drops >> + * page_requested_count) using this QEMU process's host address for the >> + * page. Register the request with the same key, rb->host + aligned_rbo, >> + * not client_addr: client_addr is a VA in the external vhost-user >> + * backend's address space and can never equal that host address, so the >> + * removal would miss forever, leaking page_requested_count and hanging >> + * postcopy teardown. >> + */ >> + postcopy_request_page(mis, rb, aligned_rbo, >> + (uint64_t)(uintptr_t)qemu_ram_get_host_addr(rb) + >> + aligned_rbo, 0); >> return 0; >> } > > True.. this should have been overlooked, thanks for fixing it. > > Reviewed-by: Peter Xu <[email protected]> > > I haven't been looking at vhost-user path for years, now looking at it > again, I do have this feeling that this ->handler() API is a bit awkward > and too heavy: what we need is a translation of an HVA -> (rb, rb_offset), > then processing it the same as when the fault was generated from QEMU > process. The vhost-user / client code might be more straightforward with a > ->translate_hva(). > > I'm not sure if you agree with that idea, or would like to work on such a > cleanup, but if you're a serious vhost-user postcopy user (we don't have a > lot on the list..), feel free to take this if this sounds beneficial. > > Thanks, > Thanks for the review.
Just to make sure I got the cleanup right: replace the PostCopyFD ->handler() callback with a lighter ->translate_hva() that only maps the client fault address to (RAMBlock, rb_offset), and let the shared fault thread call postcopy_request_shared_page() directly after the translation — i.e. the same path a fault raised by the QEMU process takes. Is that what you have in mind? We do run vhost-user postcopy, so I'm happy to take this and send the cleanup as a follow-up. Thanks,
