[Spice-devel] [PATCH xf86-qxl] Remove image cache

Marc-André Lureau marcandre.lureau at redhat.com
Thu Jan 29 05:14:01 PST 2015


While looking for leaks, I realized that the image cache looks
quite suspicious.

Not only it leaks when qxl_drop_image_cache() is called,
since all the allocated image_info_t references are lost.
But it is also useless...

(note: this won't help all the bug reports about vram memory "leak", or
rather "failed allocation", since it's a different memory area)
---
 src/qxl.h        |   2 --
 src/qxl_driver.c |   2 --
 src/qxl_image.c  | 106 ++-----------------------------------------------------
 3 files changed, 3 insertions(+), 107 deletions(-)

diff --git a/src/qxl.h b/src/qxl.h
index 54995cf..95d6682 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -571,8 +571,6 @@ struct qxl_bo *qxl_image_create     (qxl_screen_t           *qxl,
 				       Bool		       fallback);
 void              qxl_image_destroy    (qxl_screen_t           *qxl,
 				        struct qxl_bo *bo);
-void		  qxl_drop_image_cache (qxl_screen_t	       *qxl);
-
 
 /*
  * Malloc
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index 9ad8921..11b805c 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -289,7 +289,6 @@ qxl_unmap_memory (qxl_screen_t *qxl)
     if (qxl->mem)
     {
 	qxl_mem_free_all (qxl->mem);
-	qxl_drop_image_cache (qxl);
 	free(qxl->mem);
 	qxl->mem = NULL;
     }
@@ -360,7 +359,6 @@ qxl_resize_surface0 (qxl_screen_t *qxl, long surface0_size)
 	surfaces = qxl_surface_cache_evacuate_all (qxl->surface_cache);
 	qxl_io_destroy_all_surfaces (qxl); // redundant?
 	qxl_io_flush_release (qxl);
-	qxl_drop_image_cache (qxl);
 	qxl_dump_ring_stat (qxl);
 	qxl_surface_cache_replace_all (qxl->surface_cache, surfaces);
 #else
diff --git a/src/qxl_image.c b/src/qxl_image.c
index 1975df6..fc3d11a 100644
--- a/src/qxl_image.c
+++ b/src/qxl_image.c
@@ -37,18 +37,6 @@
 #include "qxl.h"
 #include "murmurhash3.h"
 
-typedef struct image_info_t image_info_t;
-
-struct image_info_t
-{
-    struct QXLImage *image;
-    int ref_count;
-    image_info_t *next;
-};
-
-#define HASH_SIZE 4096
-static image_info_t *image_table[HASH_SIZE];
-
 static unsigned int
 hash_and_copy (const uint8_t *src, int src_stride,
 	       uint8_t *dest, int dest_stride,
@@ -74,69 +62,12 @@ hash_and_copy (const uint8_t *src, int src_stride,
     return hash;
 }
 
-static image_info_t *
-lookup_image_info (unsigned int hash,
-		   int width,
-		   int height)
-{
-    struct image_info_t *info = image_table[hash % HASH_SIZE];
-
-    while (info)
-    {
-	struct QXLImage *image = info->image;
-
-	if (image->descriptor.id == hash		&&
-	    image->descriptor.width == width		&&
-	    image->descriptor.height == height)
-	{
-	    return info;
-	}
-
-	info = info->next;
-    }
-
-#if 0
-    ErrorF ("lookup of %u failed\n", hash);
-#endif
-    
-    return NULL;
-}
-
-static image_info_t *
-insert_image_info (unsigned int hash)
-{
-    struct image_info_t *info = malloc (sizeof (image_info_t));
-
-    if (!info)
-	return NULL;
-
-    info->next = image_table[hash % HASH_SIZE];
-    image_table[hash % HASH_SIZE] = info;
-    
-    return info;
-}
-
-static void
-remove_image_info (image_info_t *info)
-{
-    struct image_info_t **location = &image_table[info->image->descriptor.id % HASH_SIZE];
-
-    while (*location && (*location) != info)
-	location = &((*location)->next);
-
-    if (*location)
-	*location = info->next;
-
-    free (info);
-}
-
 struct qxl_bo *
 qxl_image_create (qxl_screen_t *qxl, const uint8_t *data,
 		  int x, int y, int width, int height,
 		  int stride, int Bpp, Bool fallback)
 {
 	uint32_t hash;
-	image_info_t *info;
 	struct QXLImage *image;
 	struct qxl_bo *head_bo, *tail_bo;
 	struct qxl_bo *image_bo;
@@ -252,18 +183,11 @@ qxl_image_create (qxl_screen_t *qxl, const uint8_t *data,
 	if ((fallback && qxl->enable_fallback_cache)	||
 	    (!fallback && qxl->enable_image_cache))
 	{
-	    if ((info = insert_image_info (hash)))
-	    {
-		info->image = image;
-		info->ref_count = 1;
-
-		image->descriptor.id = hash;
-		image->descriptor.flags = QXL_IMAGE_CACHE;
-
+            image->descriptor.id = hash;
+            image->descriptor.flags = QXL_IMAGE_CACHE;
 #if 0
-		ErrorF ("added with hash %u\n", hash);
+            ErrorF ("added with hash %u\n", hash);
 #endif
-	    }
 	}
 
 	qxl->bo_funcs->bo_unmap(image_bo);
@@ -275,28 +199,10 @@ qxl_image_destroy (qxl_screen_t *qxl,
 		   struct qxl_bo *image_bo)
 {
     struct QXLImage *image;
-
-    image_info_t *info;
     uint64_t chunk, prev_chunk;
 
     image = qxl->bo_funcs->bo_map(image_bo);
-    info = lookup_image_info (image->descriptor.id,
-			      image->descriptor.width,
-			      image->descriptor.height);
     qxl->bo_funcs->bo_unmap(image_bo);
-    if (info && info->image == image)
-    {
-	--info->ref_count;
-
-	if (info->ref_count != 0)
-	    return;
-
-#if 0
-	ErrorF ("removed %p from hash table\n", info->image);
-#endif
-	
-	remove_image_info (info);
-    }
 
     image = qxl->bo_funcs->bo_map(image_bo);
     chunk = image->bitmap.data;
@@ -322,9 +228,3 @@ qxl_image_destroy (qxl_screen_t *qxl,
     qxl->bo_funcs->bo_unmap(image_bo);
     qxl->bo_funcs->bo_decref (qxl, image_bo);
 }
-
-void
-qxl_drop_image_cache (qxl_screen_t *qxl)
-{
-    memset (image_table, 0, HASH_SIZE * sizeof (image_info_t *));
-}
-- 
2.1.0



More information about the Spice-devel mailing list