util/disk_cache: do crc32 check on compressed data for ZSTD

This will be faster and avoids checking for errors with the
compression implementation which we shouldn't need to do. Instead
we trust the compression library does the correct thing and simply
error check the data loaded from disk.

Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7725>

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index 402991a..bf77fe0 100644 (file)
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -322,14 +322,7 @@ cache_put(void *job, int thread_index)
       i++;
    }
 
-   /* 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;
-
-   disk_cache_write_item_to_disk(dc_job, &cf_data, filename);
+   disk_cache_write_item_to_disk(dc_job, filename);
 
 done:
    free(filename);

diff --git a/src/util/disk_cache_os.c b/src/util/disk_cache_os.c
index c4ee186..52de69e 100644 (file)
--- a/src/util/disk_cache_os.c
+++ b/src/util/disk_cache_os.c
@@ -39,6 +39,13 @@
 #include "zstd.h"
 #endif
 
+#include "util/crc32.h"
+
+struct cache_entry_file_data {
+   uint32_t crc32;
+   uint32_t uncompressed_size;
+};
+
 /* 3 is the recomended level, with 22 as the absolute maximum */
 #define ZSTD_COMPRESSION_LEVEL 3
 
@@ -71,7 +78,22 @@ deflate_and_write_to_disk(const void *in_data, size_t in_data_size, int dest)
       free(out);
       return 0;
    }
-   ssize_t written = write_all(dest, out, ret);
+
+   /* Create CRC of the compressed 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(out, ret);
+   cf_data.uncompressed_size = in_data_size;
+
+   size_t cf_data_size = sizeof(cf_data);
+   ssize_t written = write_all(dest, &cf_data, cf_data_size);
+   if (written == -1) {
+      free(out);
+      return 0;
+   }
+
+   written = write_all(dest, out, ret);
    if (written == -1) {
       free(out);
       return 0;
@@ -79,6 +101,19 @@ deflate_and_write_to_disk(const void *in_data, size_t in_data_size, int dest)
    free(out);
    return ret;
 #else
+   /* Create CRC of the uncompressed 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(in_data, in_data_size);
+   cf_data.uncompressed_size = in_data_size;
+
+   size_t cf_data_size = sizeof(cf_data);
+   ssize_t written = write_all(dest, &cf_data, cf_data_size);
+   if (written == -1) {
+      return 0;
+   }
+
    unsigned char *out;
 
    /* allocate deflate state */
@@ -119,7 +154,7 @@ deflate_and_write_to_disk(const void *in_data, size_t in_data_size, int dest)
          size_t have = BUFSIZE - strm.avail_out;
          compressed_size += have;
 
-         ssize_t written = write_all(dest, out, have);
+         written = write_all(dest, out, have);
          if (written == -1) {
             (void)deflateEnd(&strm);
             free(out);
@@ -595,16 +630,24 @@ disk_cache_load_item(struct disk_cache *cache, char *filename, size_t *size)
    if (ret == -1)
       goto fail;
 
+#ifdef HAVE_ZSTD
+   /* Check the data for corruption */
+   if (cf_data.crc32 != util_hash_crc32(data, cache_data_size))
+      goto fail;
+#endif
+
    /* 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))
       goto fail;
 
+#ifndef HAVE_ZSTD
    /* Check the data for corruption */
    if (cf_data.crc32 != util_hash_crc32(uncompressed_data,
                                         cf_data.uncompressed_size))
       goto fail;
+#endif
 
    free(data);
    free(filename);
@@ -654,7 +697,6 @@ disk_cache_get_cache_filename(struct disk_cache *cache, const cache_key key)
 
 void
 disk_cache_write_item_to_disk(struct disk_cache_put_job *dc_job,
-                              struct cache_entry_file_data *cf_data,
                               char *filename)
 {
    int fd = -1, fd_final = -1;
@@ -756,13 +798,6 @@ disk_cache_write_item_to_disk(struct disk_cache_put_job *dc_job,
       }
    }
 
-   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;
-   }
-
    /* Now, finally, write out the contents to the temporary file, then
     * rename them atomically to the destination filename, and also
     * perform an atomic increment of the total cache size.

diff --git a/src/util/disk_cache_os.h b/src/util/disk_cache_os.h
index 490e807..7ac6a51 100644 (file)
--- a/src/util/disk_cache_os.h
+++ b/src/util/disk_cache_os.h
@@ -89,11 +89,6 @@ struct disk_cache_put_job {
    struct cache_item_metadata cache_item_metadata;
 };
 
-struct cache_entry_file_data {
-   uint32_t crc32;
-   uint32_t uncompressed_size;
-};
-
 char *
 disk_cache_generate_cache_dir(void *mem_ctx);
 
@@ -111,7 +106,6 @@ disk_cache_get_cache_filename(struct disk_cache *cache, const cache_key key);
 
 void
 disk_cache_write_item_to_disk(struct disk_cache_put_job *dc_job,
-                              struct cache_entry_file_data *cf_data,
                               char *filename);
 
 bool