[PATCH v9 0/5] Support add/remove memory region and get-max-slots

[Bathija, Pravin]

Dear Stephen,

The changes you suggested have been incorporated in the latest patch-set v10. Responses to comments inline.


Internal Use - Confidential
> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Tuesday, April 7, 2026 7:21 AM
> To: Bathija, Pravin <Pravin.Bathija at dell.com>
> Cc: dev at dpdk.org; thomas at monjalon.net; maxime.coquelin at redhat.com;
> fengchengwen at huawei.com
> Subject: Re: [PATCH v9 0/5] Support add/remove memory region and get-max-
> slots
>
>
> [EXTERNAL EMAIL]
>
> On Tue, 7 Apr 2026 08:06:31 +0000
> <pravin.bathija at dell.com> wrote:
>
> > From: Pravin M Bathija <pravin.bathija at dell.com>
> >
> > This is version v9 of the patchset and it incorporates the
> > recommendations made by 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_ADD_MEM_REG as message ID.
> >
> > 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 v9 (Current version): Incorporate code review suggestions from
> > Stephen Hemminger as described above.
> > 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 | 403
> > ++++++++++++++++++++++++++++++++++++-----
> >  lib/vhost/vhost_user.h |  10 +
> >  4 files changed, 375 insertions(+), 43 deletions(-)
> >
>
> AI review is mostly clean now.
>
> Patch series looks good overall. One issue in 4/5:
> In rem_mem_reg, dev_invalidate_vrings (which calls
> translate_ring_addresses) runs while the region being removed is still mapped.
> Any vring address that resolves into that region will get a pointer that becomes
> invalid after the subsequent free_mem_region/munmap. Consider freeing the
> region first so translate only sees surviving regions.
>
Thanks for catching that — moved dev_invalidate_vrings after free_mem_region and the array compaction so that
translate_ring_addresses only sees surviving regions.

> Also minor: the v9 cover letter changelog says rem_mem_reg passes
> VHOST_USER_ADD_MEM_REG but the code correctly uses
> VHOST_USER_REM_MEM_REG — just a typo in the changelog.

Again good catch, made the edit to the cover letter.