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

Marc-André Lureau marcandre.lureau at gmail.com
Thu Jan 29 05:16:56 PST 2015


I forgot that line:

--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -854,7 +854,6 @@ qxl_enter_vt (VT_FUNC_ARGS_DECL)
     if (qxl->mem)
     {
        qxl_mem_free_all (qxl->mem);
-       qxl_drop_image_cache (qxl);

On Thu, Jan 29, 2015 at 2:14 PM, Marc-André Lureau
<marcandre.lureau at redhat.com> wrote:
> 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
>



-- 
Marc-André Lureau


More information about the Spice-devel mailing list