Re: [PATCH v4 12/17] xen: mapcache: Unmap first entries in buckets

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxx>]

On Tue, Apr 30, 2024 at 6:50 PM Edgar E. Iglesias
<edgar.iglesias@xxxxxxxxx> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxx>
>
> When invalidating memory ranges, if we happen to hit the first
> entry in a bucket we were never unmapping it. This was harmless
> for foreign mappings but now that we're looking to reuse the
> mapcache for transient grant mappings, we must unmap entries
> when invalidated.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxx>
> ---
>  hw/xen/xen-mapcache.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> index 4f98d284dd..0365311788 100644
> --- a/hw/xen/xen-mapcache.c
> +++ b/hw/xen/xen-mapcache.c
> @@ -486,18 +486,22 @@ static void 
> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
>          return;
>      }
>      entry->lock--;
> -    if (entry->lock > 0 || pentry == NULL) {
> +    if (entry->lock > 0) {
>          return;
>      }
>
> -    pentry->next = entry->next;
>      ram_block_notify_remove(entry->vaddr_base, entry->size, entry->size);
>      if (munmap(entry->vaddr_base, entry->size) != 0) {
>          perror("unmap fails");
>          exit(-1);
>      }
> -    g_free(entry->valid_mapping);
> -    g_free(entry);
> +    if (pentry) {
> +        pentry->next = entry->next;
> +        g_free(entry->valid_mapping);
> +        g_free(entry);
> +    } else {
> +        memset(entry, 0, sizeof *entry);

I noticed that we're leaking entry->valid_mapping here. I'll fix this for v5.

Cheers,
Edgar


> +    }
>  }
>
>  typedef struct XenMapCacheData {
> --
> 2.40.1
>