[Mesa-dev] [PATCH 3/3] util/disk_cache: write cache entry keys to file header

Nicolai Hähnle nhaehnle at gmail.com
Tue Mar 21 08:29:10 UTC 2017


On 21.03.2017 06:14, Timothy Arceri wrote:
> This can be used to deal with key hash collisions from different
> versions (should we find that to actually happen) and to find
> which mesa version produced the cache entry.
> ---
>
>  I considered creating the cache key blob at cache creation time but since
>  we would want a blob with the null terminator dropped for the hash and one
>  that includes the terminator for reading the strings from the header, I
>  decided just to create it on the fly to keep the code easier to follow.

This changes if the previous patches are modified according to my 
comments, right?

The patch mostly looks good to me, but perhaps the code could be 
simplified after all, by using the same blob in both places?

Cheers,
Nicolai

>
>  src/util/disk_cache.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 85 insertions(+), 6 deletions(-)
>
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index 599e58f..f2d67c9 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -737,33 +737,60 @@ create_put_job(struct disk_cache *cache, const cache_key key,
>  }
>
>  static void
>  destroy_put_job(void *job, int thread_index)
>  {
>     if (job) {
>        free(job);
>     }
>  }
>
> +static size_t
> +cache_keys_size(struct disk_cache *cache)
> +{
> +   /* The 2 is for the terminating null bytes of the strings */
> +   return cache->timestamp_size + cache->gpu_name_size +
> +      sizeof(cache->ptr_size) + 2;
> +}
> +
> +static uint8_t *
> +generate_cache_keys_blob(struct disk_cache *cache, size_t ck_size)
> +{
> +   uint8_t *key_blob = malloc(ck_size);
> +   if (!key_blob)
> +      return NULL;
> +
> +   size_t ts_size = cache->timestamp_size + 1;
> +   memcpy(key_blob, cache->timestamp, ts_size);
> +
> +   size_t gn_size = cache->gpu_name_size + 1;
> +   memcpy(key_blob + ts_size, cache->gpu_name, gn_size);
> +
> +   memcpy(key_blob + ts_size + gn_size, &cache->ptr_size, sizeof(uint8_t));
> +
> +   return key_blob;
> +}
> +
>  struct cache_entry_file_data {
>     uint32_t crc32;
>     uint32_t uncompressed_size;
>  };
>
>  static void
>  cache_put(void *job, int thread_index)
>  {
>     assert(job);
>
>     int fd = -1, fd_final = -1, err, ret;
>     unsigned i = 0;
>     char *filename = NULL, *filename_tmp = NULL;
> +   uint8_t *key_blob = NULL;
>     struct disk_cache_put_job *dc_job = (struct disk_cache_put_job *) job;
>
>     filename = get_cache_file(dc_job->cache, dc_job->key);
>     if (filename == NULL)
>        goto done;
>
>     /* If the cache is too large, evict something else first. */
>     while (*dc_job->cache->size + dc_job->size > dc_job->cache->max_size &&
>            i < 8) {
>        evict_lru_item(dc_job->cache);
> @@ -808,23 +835,41 @@ cache_put(void *job, int thread_index)
>      */
>     fd_final = open(filename, O_RDONLY | O_CLOEXEC);
>     if (fd_final != -1) {
>        unlink(filename_tmp);
>        goto done;
>     }
>
>     /* OK, we're now on the hook to write out a file that we know is
>      * not in the cache, and is also not being written out to the cache
>      * by some other process.
> -    *
> -    * Create CRC of the data and store at the start of the file. We will
> -    * read this when restoring the cache and use it to check for corruption.
> +    */
> +
> +   /* Write the key_blob, this can be used find information about the
> +    * mesa version that produced the entry or deal with hash collisions,
> +    * should that ever become a real problem.
> +    */
> +   size_t ck_size = cache_keys_size(dc_job->cache);
> +   key_blob = generate_cache_keys_blob(dc_job->cache, ck_size);
> +   if (!key_blob) {
> +      unlink(filename_tmp);
> +      goto done;
> +   }
> +
> +   ret = write_all(fd, key_blob, ck_size);
> +   if (ret == -1) {
> +      unlink(filename_tmp);
> +      goto done;
> +   }
> +
> +   /* Create CRC of the data. We will read this when restoring the cache and
> +    * use it to check for corruption.
>      */
>     struct cache_entry_file_data cf_data;
>     cf_data.crc32 = util_hash_crc32(dc_job->data, dc_job->size);
>     cf_data.uncompressed_size = dc_job->size;
>
>     size_t cf_data_size = sizeof(cf_data);
>     ret = write_all(fd, &cf_data, cf_data_size);
>     if (ret == -1) {
>        unlink(filename_tmp);
>        goto done;
> @@ -839,35 +884,37 @@ cache_put(void *job, int thread_index)
>     if (file_size == 0) {
>        unlink(filename_tmp);
>        goto done;
>     }
>     ret = rename(filename_tmp, filename);
>     if (ret == -1) {
>        unlink(filename_tmp);
>        goto done;
>     }
>
> -   file_size += cf_data_size;
> +   file_size += cf_data_size + ck_size;
>     p_atomic_add(dc_job->cache->size, file_size);
>
>   done:
>     if (fd_final != -1)
>        close(fd_final);
>     /* This close finally releases the flock, (now that the final file
>      * 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 (key_blob)
> +      free(key_blob);
>  }
>
>  void
>  disk_cache_put(struct disk_cache *cache, const cache_key key,
>                 const void *data, size_t size)
>  {
>     struct disk_cache_put_job *dc_job =
>        create_put_job(cache, key, data, size);
>
>     if (dc_job) {
> @@ -936,32 +983,64 @@ disk_cache_get(struct disk_cache *cache, const cache_key key, size_t *size)
>     if (fd == -1)
>        goto fail;
>
>     if (fstat(fd, &sb) == -1)
>        goto fail;
>
>     data = malloc(sb.st_size);
>     if (data == NULL)
>        goto fail;
>
> +   size_t ck_size = cache_keys_size(cache);
> +#ifndef NDEBUG
> +   uint8_t *file_header = malloc(ck_size);
> +   if (!file_header)
> +      goto fail;
> +
> +   assert(sb.st_size > ck_size);
> +   for (len = 0; len < ck_size; len += ret) {
> +      ret = read(fd, ((uint8_t *) file_header) + len, ck_size - len);
> +      if (ret == -1) {
> +         free(file_header);
> +         goto fail;
> +      }
> +   }
> +
> +   uint8_t *key_blob = generate_cache_keys_blob(cache, ck_size);
> +   if (!key_blob) {
> +      free(file_header);
> +      goto fail;
> +   }
> +   assert(memcmp(key_blob, file_header, ck_size) == 0);
> +
> +   free(file_header);
> +   free(key_blob);
> +#else
> +   /* The cache keys are currently just used for distributing precompiled
> +    * shaders, they are not used by Mesa so just skip them for now.
> +    */
> +   ret = lseek(fd, ck_size, SEEK_CUR);
> +   if (ret == -1)
> +      goto fail;
> +#endif
> +
>     /* Load the CRC that was created when the file was written. */
>     struct cache_entry_file_data cf_data;
>     size_t cf_data_size = sizeof(cf_data);
> -   assert(sb.st_size > cf_data_size);
>     for (len = 0; len < cf_data_size; len += ret) {
>        ret = read(fd, ((uint8_t *) &cf_data) + len, cf_data_size - len);
>        if (ret == -1)
>           goto fail;
>     }
>
>     /* Load the actual cache data. */
> -   size_t cache_data_size = sb.st_size - cf_data_size;
> +   size_t cache_data_size = sb.st_size - cf_data_size - ck_size;
>     for (len = 0; len < cache_data_size; len += ret) {
>        ret = read(fd, data + len, cache_data_size - len);
>        if (ret == -1)
>           goto fail;
>     }
>
>     /* Uncompress the cache data */
>     uncompressed_data = malloc(cf_data.uncompressed_size);
>     if (!inflate_cache_data(data, cache_data_size, uncompressed_data,
>                             cf_data.uncompressed_size))
>



More information about the mesa-dev mailing list