[PATCH v13 0/5] Support add/remove memory region and get-max-slots
Stephen Hemminger
stephen at networkplumber.org
Mon May 18 19:13:44 CEST 2026
On Thu, 14 May 2026 02:01:52 +0000
<pravin.bathija at dell.com> wrote:
> From: Pravin M Bathija <pravin.bathija at dell.com>
>
> This is version v13 of the patchset and it incorporates the
> recommendations made by Fengcheng Wen.
>
> Changes made to patch 3/5 and 4/5
> * Relocated function remove_guest_pages from patch 3/5 to 4/5.
> * Renamed VhostUserSingleMemReg to VhostUserMemRegMsg and memory_single
> to memreg.
>
> This implementation has been extensively tested by doing Read/Write I/O
> from multiple instances of fio + libblkio (front-end) talking to
> spdk/dpdk (back-end) based drives. Tested with qemu front-end talking to
> dpdk testpmd (back-end) performing add/removal of memory regions. Also
> tested post-copy live migration after doing add_memory_region.
>
> Version Log:
> Version v13 (Current version): Incorporate code review suggestions from
> Fengcheng Wen as described above.
> Version v12: Incorporate code review suggestions from Maxime Coquelin
> and ai-code-review.
> Changes made to patch 3/5
> Refactored async_dma_map() to delegate to async_dma_map_region(),
> eliminating code duplication between the two functions.
> Restored original comments in async_dma_map_region() explaining why
> ENODEV and EINVAL errors are ignored (these were stripped in v10)
> Reverted unnecessary changes to vhost_user_postcopy_register() --
> removed the host_user_addr == 0 checks and reg_msg_index indirection
> that were added in v10, since this function is only called from
> vhost_user_set_mem_table() where regions are always contiguous.
>
> Version v11: Incorporate code review suggestions from Stephen Hemminger.
> Change made to patch 4/5
> Fix incomplete cleanup in vhost_user_add_mem_reg() when
> vhost_user_mmap_region() fails after the mmap succeeds (e.g.
> add_guest_pages() realloc failure) realloc failure). The error path now
> calls remove_guest_pages() and free_mem_region() to undo the mapping
> and stale guest-page entries, preventing a leaked mmap and slot reuse
> corruption. The plain close(fd) path is kept for pre-mmap failures.
>
> Version v10: Incorporate code review suggestions from Stephen Hemminger.
> Change made to patch 4/5
> Moved dev_invalidate_vrings after free_mem_region, array compaction, and
> nregions decrement. This ensures translate_ring_addresses only sees
> surviving memory regions, preventing vring pointers from resolving into
> a region that is about to be unmapped.
>
> Version v9: Incorporate code review suggestions from Stephen Hemminger.
> Changes made to patch 3/5
> Restored max_guest_pages initial value to hardcoded 8 instead of
> VHOST_MEMORY_MAX_NREGIONS, matching upstream semantics.
> Changes made to patch 4/5
> Added close(reg->fd) and reg->fd = -1 before goto close_msg_fds in the
> mmap failure path to fix fd leak after fd was moved from ctx->fds[0].
> Converted dev_invalidate_vrings from a plain function to a macro +
> implementation function pair, accepting message ID as a parameter so
> the static_assert reports the correct handler at each call site.
> Updated dev_invalidate_vrings call in add_mem_reg to pass
> VHOST_USER_ADD_MEM_REG as message ID.
> Updated dev_invalidate_vrings call in rem_mem_reg to pass
> VHOST_USER_REM_MEM_REG as message ID.
>
> Version v8: Incorporate code review suggestions from Stephen Hemminger.
> rewrite async_dma_map_region function to iterate guest pages by host
> address range matching
> change function dev_invalidate_vrings to accept a double pointer to
> propagate pointer updates
> new function remove_guest_pages was added
> add_mem_reg error path was narrowed to only clean up the single failed
> region instead of destroting all existing regions
>
> Version v7: Incorporate code review suggestions from Maxime Coquelin.
> Add debug messages to vhost_postcopy_register function.
>
> Version v6: Added the enablement of this feature as a final patch in
> this patch-set and other code optimizations as suggested by Maxime
> Coquelin.
>
> Version v5: removed the patch that increased the number of memory regions
> from 8 to 128. This will be submitted as a separate feature at a later
> point after incorporating additional optimizations. Also includes code
> optimizations as suggested by Feng Cheng Wen.
>
> Version v4: code optimizations as suggested by Feng Cheng Wen.
>
> Version v3: code optimizations as suggested by Maxime Coquelin
> and Thomas Monjalon.
>
> Version v2: code optimizations as suggested by Maxime Coquelin.
>
> Version v1: Initial patch set.
>
> Pravin M Bathija (5):
> vhost: add user to mailmap and define to vhost hdr
> vhost_user: header defines for add/rem mem region
> vhost_user: support function defines for back-end
> vhost_user: Function defs for add/rem mem regions
> vhost_user: enable configure memory slots
>
> .mailmap | 1 +
> lib/vhost/rte_vhost.h | 4 +
> lib/vhost/vhost_user.c | 418 +++++++++++++++++++++++++++++++++++------
> lib/vhost/vhost_user.h | 10 +
> 4 files changed, 371 insertions(+), 62 deletions(-)
>
Some useful AI feedback
Review of [PATCH v13 0-5/5] vhost: configure memory slots support
Author: Pravin M Bathija <pravin.bathija at dell.com>
This revision addresses essentially every correctness issue raised in
the v7-v12 reviews:
- ctx->fds[0] = -1 ownership transfer is now done before mmap, and
the mmap-failure path closes reg->fd explicitly when mmap never
set reg->mmap_addr.
- _dev_invalidate_vrings now takes struct virtio_net **pdev and
writes back *pdev = dev at the end, so a numa_realloc inside
translate_ring_addresses propagates correctly. Both call sites
refresh "dev = *pdev;" afterwards.
- The dev_invalidate_vrings() macro now takes the message id and
uses static_assert(id ## _LOCK_ALL_QPS, ...), matching the
existing VHOST_USER_ASSERT_LOCK pattern. Works for both
VHOST_USER_ADD_MEM_REG and VHOST_USER_REM_MEM_REG call sites.
- Overlap check in vhost_user_add_mem_reg uses guest address
space (guest_user_addr, size / userspace_addr, memory_size),
no longer mmap_size.
- free_new_region undoes only the failed region: async DMA unmap,
remove_guest_pages, free_mem_region(reg), nregions--.
- async_dma_map_region iterates dev->nr_guest_pages and filters
by [reg_start, reg_end), eliminating the prior reg_size
underflow loop.
- The regions array is kept contiguous via memmove on REM_MEM_REG,
so existing iterators that walk mem->nregions remain correct.
- max_guest_pages is back to 8 in vhost_user_initialize_memory.
One protocol-level issue remains worth raising.
Patch 4/5 -- vhost_user: Function defs for add/rem mem regions
--------------------------------------------------------------------
Warning: ADD_MEM_REG does not send the host_user_addr reply
Per the vhost-user spec for VHOST_USER_ADD_MEM_REG, the back-end
is expected to reply with the same message format and the
userspace_addr field replaced by the host userspace address that
the region was mapped into. The handler returns
RTE_VHOST_MSG_RESULT_OK with no reply constructed, so the
dispatcher does not call send_vhost_reply().
For postcopy migration this matters in particular: the original
vhost_user_postcopy_register() does two things -- exchange the
host_user_addr with the front-end and wait for an ack, then
register the regions with userfaultfd. The patch only does the
userfaultfd registration via vhost_user_postcopy_region_register().
The in-code comment notes the payload-layout mismatch with
vhost_user_postcopy_register() but stops there.
Without the address reply, QEMU will not know the back-end's
mapping for regions added via ADD_MEM_REG, so the userfaultfd
handling on the QEMU side cannot resolve faults in those
regions. Postcopy migration combined with the
CONFIGURE_MEM_SLOTS feature will not work.
Suggested fix: construct a memreg-payload reply with
region->userspace_addr replaced by reg->host_user_addr and
return RTE_VHOST_MSG_RESULT_REPLY. At minimum, refuse
ADD_MEM_REG when dev->postcopy_listening is set, so that the
combination fails cleanly rather than silently mis-mapping.
Info: vhost_user_rem_mem_reg does not validate ctx->fd_num
The handler is registered with accepts_fd = true and does not
call validate_msg_fds(). The trailing close_msg_fds(ctx) cleans
up whatever fds were passed, so this is not a leak, but a
malformed message with an unexpected fd count is silently
accepted. The other accepts_fd handlers in this file validate
fd_num explicitly.
Info: vhost_user_get_max_mem_slots cast is unnecessary
ctx->msg.payload.u64 = (uint64_t)max_mem_slots;
max_mem_slots is uint32_t and the assignment widens
automatically; the cast can be dropped. Minor.
Reviewed-by would be appropriate once the postcopy reply is
addressed (or the combination is rejected). The rest of the
series looks correct.
More information about the dev
mailing list