| Message ID | 20240430164939.925307-13-edgar.iglesias@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Support grant mappings | expand |
On Tue, 30 Apr 2024, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com> > > 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@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > 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); > + } > } > > typedef struct XenMapCacheData { > -- > 2.40.1 >
On Tue, Apr 30, 2024 at 6:50 PM Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com> > > 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@amd.com> > --- > 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 >
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); + } } typedef struct XenMapCacheData {