From 507f701d9e89b891fdfa03ad23dc73acf3c5c6b4 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Sat, 5 Jun 2021 12:13:24 -0700 Subject: [PATCH] freedreno: Fix batch flush race condition We need to remove the batch cache entry before marking as flushed. Note that we are already holding the batch lock, to prevent flushing something that another context is emitting cmdstream to, but there is a window between batch cache lookup (under screen lock) and acquiring the batch lock that could previously result in batch cache lookup returning a flushed batch. Signed-off-by: Rob Clark Part-of: --- src/gallium/drivers/freedreno/freedreno_batch.c | 37 ++++++++++------------ src/gallium/drivers/freedreno/freedreno_batch.h | 2 +- .../drivers/freedreno/freedreno_batch_cache.c | 11 ++----- 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/gallium/drivers/freedreno/freedreno_batch.c b/src/gallium/drivers/freedreno/freedreno_batch.c index 0f4e91a..e1704d0 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.c +++ b/src/gallium/drivers/freedreno/freedreno_batch.c @@ -260,7 +260,7 @@ batch_reset_dependencies(struct fd_batch *batch) } static void -batch_reset_resources_locked(struct fd_batch *batch) +batch_reset_resources(struct fd_batch *batch) { fd_screen_assert_locked(batch->ctx->screen); @@ -275,20 +275,15 @@ batch_reset_resources_locked(struct fd_batch *batch) } static void -batch_reset_resources(struct fd_batch *batch) assert_dt -{ - fd_screen_lock(batch->ctx->screen); - batch_reset_resources_locked(batch); - fd_screen_unlock(batch->ctx->screen); -} - -static void batch_reset(struct fd_batch *batch) assert_dt { DBG("%p", batch); batch_reset_dependencies(batch); + + fd_screen_lock(batch->ctx->screen); batch_reset_resources(batch); + fd_screen_unlock(batch->ctx->screen); batch_fini(batch); batch_init(batch); @@ -316,7 +311,7 @@ __fd_batch_destroy(struct fd_batch *batch) fd_bc_invalidate_batch(batch, true); - batch_reset_resources_locked(batch); + batch_reset_resources(batch); debug_assert(batch->resources->entries == 0); _mesa_set_destroy(batch->resources, NULL); @@ -366,26 +361,28 @@ batch_flush(struct fd_batch *batch) assert_dt batch_flush_dependencies(batch); + fd_screen_lock(batch->ctx->screen); + batch_reset_resources(batch); + /* NOTE: remove=false removes the batch from the hashtable, so future + * lookups won't cache-hit a flushed batch, but leaves the weak reference + * to the batch to avoid having multiple batches with same batch->idx, as + * that causes all sorts of hilarity. + */ + fd_bc_invalidate_batch(batch, false); batch->flushed = true; + if (batch == batch->ctx->batch) - fd_batch_reference(&batch->ctx->batch, NULL); + fd_batch_reference_locked(&batch->ctx->batch, NULL); + + fd_screen_unlock(batch->ctx->screen); if (batch->fence) fd_fence_ref(&batch->ctx->last_fence, batch->fence); fd_gmem_render_tiles(batch); - batch_reset_resources(batch); debug_assert(batch->reference.count > 0); - fd_screen_lock(batch->ctx->screen); - /* NOTE: remove=false removes the batch from the hashtable, so future - * lookups won't cache-hit a flushed batch, but leaves the weak reference - * to the batch to avoid having multiple batches with same batch->idx, as - * that causes all sorts of hilarity. - */ - fd_bc_invalidate_batch(batch, false); - fd_screen_unlock(batch->ctx->screen); cleanup_submit(batch); fd_batch_unlock_submit(batch); } diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h b/src/gallium/drivers/freedreno/freedreno_batch.h index dc3a3ff..535d0de 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.h +++ b/src/gallium/drivers/freedreno/freedreno_batch.h @@ -351,7 +351,7 @@ fd_batch_unlock_submit(struct fd_batch *batch) } /** - * Returns true if emit-lock was aquired, false if failed to aquire lock, + * Returns true if emit-lock was acquired, false if failed to acquire lock, * ie. batch already flushed. */ static inline bool MUST_CHECK diff --git a/src/gallium/drivers/freedreno/freedreno_batch_cache.c b/src/gallium/drivers/freedreno/freedreno_batch_cache.c index 78fc816..751c34a 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch_cache.c +++ b/src/gallium/drivers/freedreno/freedreno_batch_cache.c @@ -436,15 +436,10 @@ batch_from_key(struct fd_batch_cache *cache, struct fd_batch_key *key, _mesa_hash_table_search_pre_hashed(cache->ht, hash, key); if (entry) { + free(key); fd_batch_reference_locked(&batch, (struct fd_batch *)entry->data); - - if (batch->flushed) { - fd_bc_invalidate_batch(batch, false); - fd_batch_reference_locked(&batch, NULL); - } else { - free(key); - return batch; - } + assert(!batch->flushed); + return batch; } batch = alloc_batch_locked(cache, ctx, false); -- 2.7.4