On Wed, Aug 26, 2026 at 11:18:28PM +0800, hongmianquan wrote: > 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.
I believe we can drop postcopy_request_shared_page() too. With the help of ->translate_hva() for the vhost-user process address space, the fault thread, postcopy_ram_fault_thread(), should be the solo place to request a fault, and it should always be with the QEMU's HVA attached to the request. That's one major reason that could be a good idea: what has been overlooked before and fixed by this patch should be unified together in the fault thread, then it's less error prone. Thanks, -- Peter Xu
