This reverts commit 6a651b6b77b68db71a027c826abccc843ace88ef.
This change was not thread safe. Applications are allowed to compile
multiple programs attached to the same context in different threads
which means we can end up with multiple threads inside
disk_cache_path_init() or thinking disk_cache_path_init() has
completed when it hasn't causing various problems.
This change was also causing piglit runs to deadlock with radeonsi
on a Ryzen 1800X (8 cores), I don't believe shader runner does
threaded compiles so I'm unsure of the exact root of this additional
problem.
---
src/util/disk_cache.c | 129 +++++++++++++++++++-------------------------------
1 file changed, 49 insertions(+), 80 deletions(-)
diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index dec5a67a79..2884d3c9c1 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -77,7 +77,6 @@
struct disk_cache {
/* The path to the cache directory. */
char *path;
- bool path_init_failed;
/* Thread queue for compressing and writing cache entries to disk */
struct util_queue cache_queue;
@@ -179,20 +178,37 @@ concatenate_and_mkdir(void *ctx, const char *path, const
char *name)
return NULL;
}
-static bool
-disk_cache_path_init(struct disk_cache *cache)
+#define DRV_KEY_CPY(_dst, _src, _src_size) \
+do { \
+ memcpy(_dst, _src, _src_size); \
+ _dst += _src_size; \
+} while (0);
+
+struct disk_cache *
+disk_cache_create(const char *gpu_name, const char *timestamp,
+ uint64_t driver_flags)
{
- void *local = NULL;
- char *path;
+ void *local;
+ struct disk_cache *cache = NULL;
+ char *path, *max_size_str;
+ uint64_t max_size;
int fd = -1;
struct stat sb;
size_t size;
+ /* If running as a users other than the real user disable cache */
+ if (geteuid() != getuid())
+ return NULL;
+
/* A ralloc context for transient data during this invocation. */
local = ralloc_context(NULL);
if (local == NULL)
goto fail;
+ /* At user request, disable shader cache entirely. */
+ if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", false))
+ goto fail;
+
/* Determine path for cache based on the first defined name as follows:
*
* $MESA_GLSL_CACHE_DIR
@@ -257,6 +273,10 @@ disk_cache_path_init(struct disk_cache *cache)
goto fail;
}
+ cache = ralloc(NULL, struct disk_cache);
+ if (cache == NULL)
+ goto fail;
+
cache->path = ralloc_strdup(cache, path);
if (cache->path == NULL)
goto fail;
@@ -305,58 +325,6 @@ disk_cache_path_init(struct disk_cache *cache)
cache->size = (uint64_t *) cache->index_mmap;
cache->stored_keys = cache->index_mmap + sizeof(uint64_t);
- /* 1 thread was chosen because we don't really care about getting things
- * to disk quickly just that it's not blocking other tasks.
- *
- * The queue will resize automatically when it's full, so adding new jobs
- * doesn't stall.
- */
- util_queue_init(&cache->cache_queue, "disk_cache", 32, 1,
- UTIL_QUEUE_INIT_RESIZE_IF_FULL |
- UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY);
-
- ralloc_free(local);
-
- return true;
-
- fail:
- if (fd != -1)
- close(fd);
-
- if (local)
- ralloc_free(local);
-
- cache->path_init_failed = true;
-
- return false;
-}
-
-#define DRV_KEY_CPY(_dst, _src, _src_size) \
-do { \
- memcpy(_dst, _src, _src_size); \
- _dst += _src_size; \
-} while (0);
-
-struct disk_cache *
-disk_cache_create(const char *gpu_name, const char *timestamp,
- uint64_t driver_flags)
-{
- struct disk_cache *cache = NULL;
- char *max_size_str;
- uint64_t max_size;
-
- /* If running as a users other than the real user disable cache */
- if (geteuid() != getuid())
- return NULL;
-
- /* At user request, disable shader cache entirely. */
- if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", false))
- return NULL;
-
- cache = rzalloc(NULL, struct disk_cache);
- if (cache == NULL)
- return NULL;
-
max_size = 0;
max_size_str = getenv("MESA_GLSL_CACHE_MAX_SIZE");
@@ -392,6 +360,16 @@ disk_cache_create(const char *gpu_name, const char
*timestamp,
cache->max_size = max_size;
+ /* 1 thread was chosen because we don't really care about getting things
+ * to disk quickly just that it's not blocking other tasks.
+ *
+ * The queue will resize automatically when it's full, so adding new jobs
+ * doesn't stall.
+ */
+ util_queue_init(&cache->cache_queue, "disk_cache", 32, 1,
+ UTIL_QUEUE_INIT_RESIZE_IF_FULL |
+ UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY);
+
uint8_t cache_version = CACHE_VERSION;
size_t cv_size = sizeof(cache_version);
cache->driver_keys_blob_size = cv_size;
@@ -414,10 +392,8 @@ disk_cache_create(const char *gpu_name, const char
*timestamp,
cache->driver_keys_blob =
ralloc_size(cache, cache->driver_keys_blob_size);
- if (!cache->driver_keys_blob) {
- ralloc_free(cache);
- return NULL;
- }
+ if (!cache->driver_keys_blob)
+ goto fail;
uint8_t *drv_key_blob = cache->driver_keys_blob;
DRV_KEY_CPY(drv_key_blob, &cache_version, cv_size)
@@ -429,13 +405,24 @@ disk_cache_create(const char *gpu_name, const char
*timestamp,
/* Seed our rand function */
s_rand_xorshift128plus(cache->seed_xorshift128plus, true);
+ ralloc_free(local);
+
return cache;
+
+ fail:
+ if (fd != -1)
+ close(fd);
+ if (cache)
+ ralloc_free(cache);
+ ralloc_free(local);
+
+ return NULL;
}
void
disk_cache_destroy(struct disk_cache *cache)
{
- if (cache && cache->index_mmap) {
+ if (cache) {
util_queue_destroy(&cache->cache_queue);
munmap(cache->index_mmap, cache->index_mmap_size);
}
@@ -454,9 +441,6 @@ get_cache_file(struct disk_cache *cache, const cache_key
key)
char buf[41];
char *filename;
- if (!cache->path)
- return NULL;
-
_mesa_sha1_format(buf, key);
if (asprintf(&filename, "%s/%c%c/%s", cache->path, buf[0],
buf[1], buf + 2) == -1)
@@ -1012,11 +996,6 @@ disk_cache_put(struct disk_cache *cache, const cache_key
key,
const void *data, size_t size,
struct cache_item_metadata *cache_item_metadata)
{
- /* Initialize path if not initialized yet. */
- if (cache->path_init_failed ||
- (!cache->path && !disk_cache_path_init(cache)))
- return;
-
struct disk_cache_put_job *dc_job =
create_put_job(cache, key, data, size, cache_item_metadata);
@@ -1194,11 +1173,6 @@ disk_cache_put_key(struct disk_cache *cache, const
cache_key key)
int i = CPU_TO_LE32(*key_chunk) & CACHE_INDEX_KEY_MASK;
unsigned char *entry;
- if (!cache->path) {
- assert(!"disk_cache_put_key called with no path set");
- return;
- }
-
entry = &cache->stored_keys[i * CACHE_KEY_SIZE];
memcpy(entry, key, CACHE_KEY_SIZE);
@@ -1218,11 +1192,6 @@ disk_cache_has_key(struct disk_cache *cache, const
cache_key key)
int i = CPU_TO_LE32(*key_chunk) & CACHE_INDEX_KEY_MASK;
unsigned char *entry;
- /* Initialize path if not initialized yet. */
- if (cache->path_init_failed ||
- (!cache->path && !disk_cache_path_init(cache)))
- return false;
-
entry = &cache->stored_keys[i * CACHE_KEY_SIZE];
return memcmp(entry, key, CACHE_KEY_SIZE) == 0;
--
2.14.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
