wsim: Lock the seqno to prevent reordering of submissions

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

1 files changed, 43 insertions, 43 deletions

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index c15dc365..48d84fd1 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -181,7 +181,9 @@ struct workload
 
 	struct workload *global_wrk;
 	const struct workload_balancer *global_balancer;
-	pthread_mutex_t mutex;
+
+	pthread_mutex_t mutex[NUM_ENGINES];
+	pthread_mutex_t global_mutex;
 
 	union {
 		struct rtavg {
@@ -759,13 +761,17 @@ eb_update_flags(struct w_step *w, enum intel_engine_id engine,
 		w->eb.flags |= LOCAL_I915_EXEC_FENCE_OUT;
 }
 
+static inline struct workload *get_workload(struct workload *wrk)
+{
+	if (wrk->flags & GLOBAL_BALANCE)
+		wrk = wrk->global_wrk;
+	return wrk;
+}
+
 static struct drm_i915_gem_exec_object2 *
 get_status_objects(struct workload *wrk)
 {
-	if (wrk->flags & GLOBAL_BALANCE)
-		return wrk->global_wrk->status_object;
-	else
-		return wrk->status_object;
+	return get_workload(wrk)->status_object;
 }
 
 static void
@@ -842,19 +848,22 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 	unsigned int ctx_vcs = 0;
 	int max_ctx = -1;
 	struct w_step *w;
+	int ret;
 	int i;
 
 	wrk->id = id;
 	wrk->prng = rand();
 	wrk->run = true;
 
-	if (flags & INITVCSRR)
-		wrk->vcs_rr = id & 1;
-
-	if (flags & GLOBAL_BALANCE) {
-		int ret = pthread_mutex_init(&wrk->mutex, NULL);
+	for (i = 0; i < NUM_ENGINES; i++) {
+		ret = pthread_mutex_init(&wrk->mutex[i], NULL);
 		igt_assert(ret == 0);
 	}
+	ret = pthread_mutex_init(&wrk->global_mutex, NULL);
+	igt_assert(ret == 0);
+
+	if (flags & INITVCSRR)
+		wrk->vcs_rr = id & 1;
 
 	if (flags & SEQNO) {
 		if (!(flags & GLOBAL_BALANCE) || id == 0) {
@@ -948,36 +957,23 @@ static enum intel_engine_id get_vcs_engine(unsigned int n)
 	return vcs_engines[n];
 }
 
-static uint32_t new_seqno(struct workload *wrk, enum intel_engine_id engine)
+static uint32_t lock_seqno(struct workload *wrk, enum intel_engine_id engine)
 {
-	uint32_t seqno;
-	int ret;
-
-	if (wrk->flags & GLOBAL_BALANCE) {
-		igt_assert(wrk->global_wrk);
-		wrk = wrk->global_wrk;
-
-		ret = pthread_mutex_lock(&wrk->mutex);
-		igt_assert(ret == 0);
-	}
-
-	seqno = ++wrk->seqno[engine];
-
-	if (wrk->flags & GLOBAL_BALANCE) {
-		ret = pthread_mutex_unlock(&wrk->mutex);
-		igt_assert(ret == 0);
-	}
+	wrk = get_workload(wrk);
+	igt_assert(pthread_mutex_lock(&wrk->mutex[engine]) == 0);
+	return ++wrk->seqno[engine];
+}
 
-	return seqno;
+static void unlock_seqno(struct workload *wrk, enum intel_engine_id engine)
+{
+	wrk = get_workload(wrk);
+	igt_assert(pthread_mutex_unlock(&wrk->mutex[engine]) == 0);
 }
 
 static uint32_t
 current_seqno(struct workload *wrk, enum intel_engine_id engine)
 {
-	if (wrk->flags & GLOBAL_BALANCE)
-		return wrk->global_wrk->seqno[engine];
-	else
-		return wrk->seqno[engine];
+	return get_workload(wrk)->seqno[engine];
 }
 
 #define READ_ONCE(x) (*(volatile typeof(x) *)(&(x)))
@@ -985,10 +981,7 @@ current_seqno(struct workload *wrk, enum intel_engine_id engine)
 static uint32_t
 read_status_page(struct workload *wrk, unsigned int idx)
 {
-	if (wrk->flags & GLOBAL_BALANCE)
-		return READ_ONCE(wrk->global_wrk->status_page[idx]);
-	else
-		return READ_ONCE(wrk->status_page[idx]);
+	return READ_ONCE(get_workload(wrk)->status_page[idx]);
 }
 
 static uint32_t
@@ -1471,12 +1464,15 @@ global_balance(const struct workload_balancer *balancer,
 
 	wrk = wrk->global_wrk;
 
-	ret = pthread_mutex_lock(&wrk->mutex);
+	/* This is overkill, we just need to rewrite each balancer to be
+	 * lockless! (They mostly are.)
+	 * */
+	ret = pthread_mutex_lock(&wrk->global_mutex);
 	igt_assert(ret == 0);
 
 	engine = wrk->global_balancer->balance(wrk->global_balancer, wrk, w);
 
-	ret = pthread_mutex_unlock(&wrk->mutex);
+	ret = pthread_mutex_unlock(&wrk->global_mutex);
 	igt_assert(ret == 0);
 
 	return engine;
@@ -1588,6 +1584,7 @@ static void init_status_page(struct workload *wrk, unsigned int flags)
 		uint64_t presumed_offset = status_object[0].offset;
 		uint32_t offset = engine * 128;
 		uint32_t *cs = base + offset / sizeof(*cs);
+		uint32_t seqno = lock_seqno(wrk, engine);
 		uint64_t addr;
 
 		r->offset = offset + sizeof(uint32_t);
@@ -1598,7 +1595,7 @@ static void init_status_page(struct workload *wrk, unsigned int flags)
 		*cs++ = MI_STORE_DWORD_IMM;
 		*cs++ = addr;
 		*cs++ = addr >> 32;
-		*cs++ = new_seqno(wrk, engine);
+		*cs++ = seqno;
 		offset += 4 * sizeof(uint32_t);
 
 		/* When we are busy, we can just reuse the last set of timings.
@@ -1637,7 +1634,7 @@ static void init_status_page(struct workload *wrk, unsigned int flags)
 		*cs++ = MI_STORE_DWORD_IMM;
 		*cs++ = addr;
 		*cs++ = addr >> 32;
-		*cs++ = current_seqno(wrk, engine);
+		*cs++ = seqno;
 		offset += 4 * sizeof(uint32_t);
 
 		*cs++ = MI_BATCH_BUFFER_END;
@@ -1649,6 +1646,7 @@ static void init_status_page(struct workload *wrk, unsigned int flags)
 		eb.batch_start_offset = 128 * engine;
 
 		gem_execbuf(fd, &eb);
+		unlock_seqno(wrk, engine);
 	}
 }
 
@@ -1656,7 +1654,7 @@ static void
 do_eb(struct workload *wrk, struct w_step *w, enum intel_engine_id engine,
       unsigned int flags)
 {
-	uint32_t seqno = new_seqno(wrk, engine);
+	uint32_t seqno = lock_seqno(wrk, engine);
 	unsigned int i;
 
 	eb_update_flags(w, engine, flags);
@@ -1668,7 +1666,7 @@ do_eb(struct workload *wrk, struct w_step *w, enum intel_engine_id engine,
 
 	w->eb.batch_start_offset =
 		ALIGN(w->bb_sz - get_bb_sz(get_duration(w)),
-			2 * sizeof(uint32_t));
+		      2 * sizeof(uint32_t));
 
 	for (i = 0; i < w->fence_deps.nr; i++) {
 		int tgt = w->idx + w->fence_deps.list[i];
@@ -1691,6 +1689,8 @@ do_eb(struct workload *wrk, struct w_step *w, enum intel_engine_id engine,
 		w->emit_fence = w->eb.rsvd2 >> 32;
 		igt_assert(w->emit_fence > 0);
 	}
+
+	unlock_seqno(wrk, engine);
 }
 
 static bool sync_deps(struct workload *wrk, struct w_step *w)