Re: [RFC 5/5] libvhost-user: handle removal of identical regions

[Stefan Hajnoczi]

On Wed, Dec 15, 2021 at 10:29:55PM +0000, Raphael Norwitz wrote:
> diff --git a/subprojects/libvhost-user/libvhost-user.c 
> b/subprojects/libvhost-user/libvhost-user.c
> index 74a9980194..2f465a4f0e 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -809,6 +809,7 @@ static bool
>  vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
>      VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m;
>      int i;
> +    bool found = false;
>  
>      if (vmsg->fd_num != 1 ||
>          vmsg->size != sizeof(vmsg->payload.memreg)) {
> @@ -831,25 +832,25 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
>              VuDevRegion *r = &dev->regions[i];
>              void *m = (void *) (uintptr_t) r->mmap_addr;
>  
> -            if (m) {
> +            if (m && !found) {
>                  munmap(m, r->size + r->mmap_offset);
>              }

Why is only the first region unmapped? My interpretation of
vu_add_mem_reg() is that it mmaps duplicate regions to unique mmap_addr
addresses, so we need to munmap each of them.

>  
> -            break;
> +            /*
> +             * Shift all affected entries by 1 to close the hole at index i 
> and
> +             * zero out the last entry.
> +             */
> +            memmove(dev->regions + i, dev->regions + i + 1,
> +                    sizeof(VuDevRegion) * (dev->nregions - i - 1));
> +            memset(dev->regions + dev->nregions - 1, 0, sizeof(VuDevRegion));
> +            DPRINT("Successfully removed a region\n");
> +            dev->nregions--;
> +
> +            found = true;
>          }

i-- is missing. dev->regions[] has been shortened so we need to check
the same element again.

signature.asc
Description: PGP signature