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 <robdclark@chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11200>

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);
 
@@ -274,21 +274,16 @@ 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);
 
-   batch->flushed = true;
-   if (batch == batch->ctx->batch)
-      fd_batch_reference(&batch->ctx->batch, NULL);
-
-   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);
+   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_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);
+
+   debug_assert(batch->reference.count > 0);
+
    cleanup_submit(batch);
    fd_batch_unlock_submit(batch);
 }

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

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);