Mesa (master): freedreno: proper locking for iterating dependent batches

[Rob Clark]

Module: Mesa
Branch: master
Commit: deb57fb237c3be9629a39ef1978dfac4563d6bda
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=deb57fb237c3be9629a39ef1978dfac4563d6bda

Author: Rob Clark <robdclark at gmail.com>
Date:   Mon Nov 20 09:52:04 2017 -0500

freedreno: proper locking for iterating dependent batches

In transfer_map(), when we need to flush batches that read from a
resource, we should be holding screen->lock to guard against race
conditions.  Somehow deferred flush seems to make this existing
race more obvious.

Signed-off-by: Rob Clark <robdclark at gmail.com>

---

 src/gallium/drivers/freedreno/freedreno_batch.h    |  2 +-
 src/gallium/drivers/freedreno/freedreno_resource.c | 26 ++++++++++++++++------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h b/src/gallium/drivers/freedreno/freedreno_batch.h
index 41356e3519..8b05f0657a 100644
--- a/src/gallium/drivers/freedreno/freedreno_batch.h
+++ b/src/gallium/drivers/freedreno/freedreno_batch.h
@@ -66,7 +66,7 @@ struct fd_hw_sample;
 struct fd_batch {
 	struct pipe_reference reference;
 	unsigned seqno;
-	unsigned idx;
+	unsigned idx;       /* index into cache->batches[] */
 
 	int in_fence_fd;
 	bool needs_out_fence_fd;
diff --git a/src/gallium/drivers/freedreno/freedreno_resource.c b/src/gallium/drivers/freedreno/freedreno_resource.c
index 880666d3af..e305b5f9eb 100644
--- a/src/gallium/drivers/freedreno/freedreno_resource.c
+++ b/src/gallium/drivers/freedreno/freedreno_resource.c
@@ -531,14 +531,26 @@ fd_resource_transfer_map(struct pipe_context *pctx,
 
 		if (needs_flush) {
 			if (usage & PIPE_TRANSFER_WRITE) {
-				struct fd_batch *batch, *last_batch = NULL;
-				foreach_batch(batch, &ctx->screen->batch_cache, rsc->batch_mask) {
-					fd_batch_reference(&last_batch, batch);
+				struct fd_batch *batch, *batches[32] = {0};
+				uint32_t batch_mask;
+
+				/* This is a bit awkward, probably a fd_batch_flush_locked()
+				 * would make things simpler.. but we need to hold the lock
+				 * to iterate the batches which reference this resource.  So
+				 * we must first grab references under a lock, then flush.
+				 */
+				mtx_lock(&ctx->screen->lock);
+				batch_mask = rsc->batch_mask;
+				foreach_batch(batch, &ctx->screen->batch_cache, batch_mask)
+					fd_batch_reference(&batches[batch->idx], batch);
+				mtx_unlock(&ctx->screen->lock);
+
+				foreach_batch(batch, &ctx->screen->batch_cache, batch_mask)
 					fd_batch_flush(batch, false);
-				}
-				if (last_batch) {
-					fd_batch_sync(last_batch);
-					fd_batch_reference(&last_batch, NULL);
+
+				foreach_batch(batch, &ctx->screen->batch_cache, batch_mask) {
+					fd_batch_sync(batch);
+					fd_batch_reference(&batches[batch->idx], NULL);
 				}
 				assert(rsc->batch_mask == 0);
 			} else {