[Mesa-dev] [PATCH 3/4] util/disk_cache: have disk_cache_put() optionally free memory

Timothy Arceri tarceri at itsqueeze.com
Mon Mar 13 01:01:34 UTC 2017


The following patch will move disk_cache_put() into a thread queue
so we need to be sure memory is not freed before the thread is
completed. Here we move responsibility for releasing the memory
onto the disk cache.

We also make a copy of the data in radeonsi as we don't want to
free the original.
---
 src/compiler/glsl/shader_cache.cpp              |  5 ++---
 src/compiler/glsl/tests/cache_test.c            | 16 ++++++----------
 src/gallium/drivers/radeonsi/si_state_shaders.c | 12 ++++++++++--
 src/mesa/state_tracker/st_shader_cache.c        |  4 +---
 src/util/disk_cache.c                           |  8 ++++----
 src/util/disk_cache.h                           |  4 ++--
 6 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp
index 6e2c527..f50dd60 100644
--- a/src/compiler/glsl/shader_cache.cpp
+++ b/src/compiler/glsl/shader_cache.cpp
@@ -1269,23 +1269,22 @@ shader_cache_write_program_metadata(struct gl_context *ctx,
 
    char sha1_buf[41];
    for (unsigned i = 0; i < prog->NumShaders; i++) {
       disk_cache_put_key(cache, prog->Shaders[i]->sha1);
       if (ctx->_Shader->Flags & GLSL_CACHE_INFO) {
          fprintf(stderr, "marking shader: %s\n",
                  _mesa_sha1_format(sha1_buf, prog->Shaders[i]->sha1));
       }
    }
 
-   disk_cache_put(cache, prog->data->sha1, metadata->data, metadata->size);
-
-   free(metadata);
+   disk_cache_put(cache, prog->data->sha1, metadata->data, metadata->size,
+                  metadata);
 
    if (ctx->_Shader->Flags & GLSL_CACHE_INFO) {
       fprintf(stderr, "putting program metadata in cache: %s\n",
               _mesa_sha1_format(sha1_buf, prog->data->sha1));
    }
 }
 
 bool
 shader_cache_read_program_metadata(struct gl_context *ctx,
                                    struct gl_shader_program *prog)
diff --git a/src/compiler/glsl/tests/cache_test.c b/src/compiler/glsl/tests/cache_test.c
index 7a1ff0a..6451e65 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -251,31 +251,31 @@ test_put_and_get(void)
    cache = disk_cache_create("test", "make_check");
 
    _mesa_sha1_compute(blob, sizeof(blob), blob_key);
 
    /* Ensure that disk_cache_get returns nothing before anything is added. */
    result = disk_cache_get(cache, blob_key, &size);
    expect_null(result, "disk_cache_get with non-existent item (pointer)");
    expect_equal(size, 0, "disk_cache_get with non-existent item (size)");
 
    /* Simple test of put and get. */
-   disk_cache_put(cache, blob_key, blob, sizeof(blob));
+   disk_cache_put(cache, blob_key, blob, sizeof(blob), NULL);
 
    result = disk_cache_get(cache, blob_key, &size);
    expect_equal_str(blob, result, "disk_cache_get of existing item (pointer)");
    expect_equal(size, sizeof(blob), "disk_cache_get of existing item (size)");
 
    free(result);
 
    /* Test put and get of a second item. */
    _mesa_sha1_compute(string, sizeof(string), string_key);
-   disk_cache_put(cache, string_key, string, sizeof(string));
+   disk_cache_put(cache, string_key, string, sizeof(string), NULL);
 
    result = disk_cache_get(cache, string_key, &size);
    expect_equal_str(result, string, "2nd disk_cache_get of existing item (pointer)");
    expect_equal(size, sizeof(string), "2nd disk_cache_get of existing item (size)");
 
    free(result);
 
    /* Set the cache size to 1KB and add a 1KB item to force an eviction. */
    disk_cache_destroy(cache);
 
@@ -297,23 +297,21 @@ test_put_and_get(void)
     * That's actually expected given how the eviction code is
     * implemented, (which expects to only evict once things are more
     * interestingly full than that).
     *
     * For this test, we force this signature to land in the same
     * directory as the original blob first written to the cache.
     */
    _mesa_sha1_compute(one_KB, 1024, one_KB_key);
    one_KB_key[0] = blob_key_byte_zero;
 
-   disk_cache_put(cache, one_KB_key, one_KB, 1024);
-
-   free(one_KB);
+   disk_cache_put(cache, one_KB_key, one_KB, 1024, one_KB);
 
    result = disk_cache_get(cache, one_KB_key, &size);
    expect_non_null(result, "3rd disk_cache_get of existing item (pointer)");
    expect_equal(size, 1024, "3rd disk_cache_get of existing item (size)");
 
    free(result);
 
    /* Ensure eviction happened by checking that only one of the two
     * previously-added items can still be fetched.
     */
@@ -327,44 +325,42 @@ test_put_and_get(void)
    expect_equal(count, 1, "disk_cache_put eviction with MAX_SIZE=1K");
 
    /* Now increase the size to 1M, add back both items, and ensure all
     * three that have been added are available via disk_cache_get.
     */
    disk_cache_destroy(cache);
 
    setenv("MESA_GLSL_CACHE_MAX_SIZE", "1M", 1);
    cache = disk_cache_create("test", "make_check");
 
-   disk_cache_put(cache, blob_key, blob, sizeof(blob));
-   disk_cache_put(cache, string_key, string, sizeof(string));
+   disk_cache_put(cache, blob_key, blob, sizeof(blob), NULL);
+   disk_cache_put(cache, string_key, string, sizeof(string), NULL);
 
    count = 0;
    if (does_cache_contain(cache, blob_key))
        count++;
 
    if (does_cache_contain(cache, string_key))
        count++;
 
    if (does_cache_contain(cache, one_KB_key))
        count++;
 
    expect_equal(count, 3, "no eviction before overflow with MAX_SIZE=1M");
 
    /* Finally, check eviction again after adding an object of size 1M. */
    one_MB = calloc(1024, 1024);
 
    _mesa_sha1_compute(one_MB, 1024 * 1024, one_MB_key);
    one_MB_key[0] = blob_key_byte_zero;;
 
-   disk_cache_put(cache, one_MB_key, one_MB, 1024 * 1024);
-
-   free(one_MB);
+   disk_cache_put(cache, one_MB_key, one_MB, 1024 * 1024, one_MB);
 
    count = 0;
    if (does_cache_contain(cache, blob_key))
        count++;
 
    if (does_cache_contain(cache, string_key))
        count++;
 
    if (does_cache_contain(cache, one_KB_key))
        count++;
diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
index 9cde0aa..b11f15f 100644
--- a/src/gallium/drivers/radeonsi/si_state_shaders.c
+++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
@@ -201,22 +201,30 @@ static bool si_shader_cache_insert_shader(struct si_screen *sscreen,
 		return false;
 
 	if (_mesa_hash_table_insert(sscreen->shader_cache, tgsi_binary,
 				    hw_binary) == NULL) {
 		FREE(hw_binary);
 		return false;
 	}
 
 	if (sscreen->b.disk_shader_cache && insert_into_disk_cache) {
 		_mesa_sha1_compute(tgsi_binary, *((uint32_t *)tgsi_binary), key);
-		disk_cache_put(sscreen->b.disk_shader_cache, key, hw_binary,
-			       *((uint32_t *) hw_binary));
+
+		size_t hw_binary_size = *((uint32_t *) hw_binary);
+
+		/* The binary could be freed before we write it to disk so
+		 * make a copy.
+		 */
+		void *hw_binary_cp = malloc(hw_binary_size);
+		memcpy(hw_binary_cp, hw_binary, hw_binary_size);
+		disk_cache_put(sscreen->b.disk_shader_cache, key,
+			       hw_binary_cp, hw_binary_size, hw_binary_cp);
 	}
 
 	return true;
 }
 
 static bool si_shader_cache_load_shader(struct si_screen *sscreen,
 					void *tgsi_binary,
 				        struct si_shader *shader)
 {
 	struct hash_entry *entry =
diff --git a/src/mesa/state_tracker/st_shader_cache.c b/src/mesa/state_tracker/st_shader_cache.c
index 4383194..4f615aa 100644
--- a/src/mesa/state_tracker/st_shader_cache.c
+++ b/src/mesa/state_tracker/st_shader_cache.c
@@ -40,21 +40,21 @@ write_stream_out_to_cache(struct blob *blob,
 
 static void
 write_tgsi_to_cache(struct blob *blob, struct pipe_shader_state *tgsi,
                     struct st_context *st, unsigned char *sha1,
                     unsigned num_tokens)
 {
    blob_write_uint32(blob, num_tokens);
    blob_write_bytes(blob, tgsi->tokens,
                     num_tokens * sizeof(struct tgsi_token));
 
-   disk_cache_put(st->ctx->Cache, sha1, blob->data, blob->size);
+   disk_cache_put(st->ctx->Cache, sha1, blob->data, blob->size, blob);
 }
 
 /**
  * Store tgsi and any other required state in on-disk shader cache.
  */
 void
 st_store_tgsi_in_disk_cache(struct st_context *st, struct gl_program *prog,
                             struct pipe_shader_state *out_state,
                             unsigned num_tokens)
 {
@@ -126,22 +126,20 @@ st_store_tgsi_in_disk_cache(struct st_context *st, struct gl_program *prog,
    default:
       unreachable("Unsupported stage");
    }
 
    if (st->ctx->_Shader->Flags & GLSL_CACHE_INFO) {
       char sha1_buf[41];
       _mesa_sha1_format(sha1_buf, sha1);
       fprintf(stderr, "putting %s tgsi_tokens in cache: %s\n",
               _mesa_shader_stage_to_string(prog->info.stage), sha1_buf);
    }
-
-   free(blob);
 }
 
 static void
 read_stream_out_from_cache(struct blob_reader *blob_reader,
                            struct pipe_shader_state *tgsi)
 {
    blob_copy_bytes(blob_reader, (uint8_t *) &tgsi->stream_output,
                     sizeof(tgsi->stream_output));
 }
 
diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index 160774a..4f902df 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -782,24 +782,22 @@ destroy_put_job(void *job, int thread_index)
       free(dc_job);
    }
 }
 
 struct cache_entry_file_data {
    uint32_t crc32;
    uint32_t uncompressed_size;
 };
 
 void
-disk_cache_put(struct disk_cache *cache,
-          const cache_key key,
-          const void *data,
-          size_t size)
+disk_cache_put(struct disk_cache *cache, const cache_key key,
+               const void *data, size_t size, void *mem_to_free)
 {
    int fd = -1, fd_final = -1, err, ret;
    size_t len;
    char *filename = NULL, *filename_tmp = NULL;
 
    filename = get_cache_file(cache, key);
    if (filename == NULL)
       goto done;
 
    /* Write to a temporary file to allow for an atomic rename to the
@@ -887,20 +885,22 @@ disk_cache_put(struct disk_cache *cache,
       close(fd_final);
    /* This close finally releases the flock, (now that the final dile
     * has been renamed into place and the size has been added).
     */
    if (fd != -1)
       close(fd);
    if (filename_tmp)
       free(filename_tmp);
    if (filename)
       free(filename);
+   if (mem_to_free)
+      free(mem_to_free);
 }
 
 /**
  * Decompresses cache entry, returns true if successful.
  */
 static bool
 inflate_cache_data(uint8_t *in_data, size_t in_data_size,
                    uint8_t *out_data, size_t out_data_size)
 {
    z_stream strm;
diff --git a/src/util/disk_cache.h b/src/util/disk_cache.h
index 3659b6d..52847bb 100644
--- a/src/util/disk_cache.h
+++ b/src/util/disk_cache.h
@@ -128,21 +128,21 @@ disk_cache_remove(struct disk_cache *cache, const cache_key key);
  * Store an item in the cache under the name \key.
  *
  * The item can be retrieved later with disk_cache_get(), (unless the item has
  * been evicted in the interim).
  *
  * Any call to disk_cache_put() may cause an existing, random item to be
  * evicted from the cache.
  */
 void
 disk_cache_put(struct disk_cache *cache, const cache_key key,
-               const void *data, size_t size);
+               const void *data, size_t size, void *mem_to_free);
 
 /**
  * Retrieve an item previously stored in the cache with the name <key>.
  *
  * The item must have been previously stored with a call to disk_cache_put().
  *
  * If \size is non-NULL, then, on successful return, it will be set to the
  * size of the object.
  *
  * \return A pointer to the stored object if found. NULL if the object
@@ -186,21 +186,21 @@ disk_cache_create(const char *gpu_name, const char *timestamp)
    return NULL;
 }
 
 static inline void
 disk_cache_destroy(struct disk_cache *cache) {
    return;
 }
 
 static inline void
 disk_cache_put(struct disk_cache *cache, const cache_key key,
-          const void *data, size_t size)
+               const void *data, size_t size, void *mem_to_free)
 {
    return;
 }
 
 static inline void
 disk_cache_remove(struct disk_cache *cache, const cache_key key)
 {
    return;
 }
 
-- 
2.9.3



More information about the mesa-dev mailing list