[PATCH 1/2] drm/i915: Guard unpark/park with the gt.active_mutex

Chris Wilson chris at chris-wilson.co.uk
Tue Mar 12 14:28:51 UTC 2019


Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c           |  9 +++----
 drivers/gpu/drm/i915/i915_drv.h               |  3 ++-
 drivers/gpu/drm/i915/i915_gem.c               | 24 +++++++++++--------
 drivers/gpu/drm/i915/i915_gem_evict.c         |  2 +-
 drivers/gpu/drm/i915/i915_request.c           | 21 ++++++++++++----
 .../gpu/drm/i915/selftests/i915_gem_context.c |  4 ++--
 .../gpu/drm/i915/selftests/i915_gem_object.c  | 10 ++++----
 .../gpu/drm/i915/selftests/mock_gem_device.c  |  3 ++-
 8 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6a90558de213..c760e7574f9d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2034,7 +2034,8 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 
 	seq_printf(m, "RPS enabled? %d\n", rps->enabled);
 	seq_printf(m, "GPU busy? %s [%d requests]\n",
-		   yesno(dev_priv->gt.awake), dev_priv->gt.active_requests);
+		   yesno(dev_priv->gt.awake),
+		   atomic_read(&dev_priv->gt.active_requests));
 	seq_printf(m, "Boosts outstanding? %d\n",
 		   atomic_read(&rps->num_waiters));
 	seq_printf(m, "Interactive? %d\n", READ_ONCE(rps->power.interactive));
@@ -2055,7 +2056,7 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 
 	if (INTEL_GEN(dev_priv) >= 6 &&
 	    rps->enabled &&
-	    dev_priv->gt.active_requests) {
+	    atomic_read(&dev_priv->gt.active_requests)) {
 		u32 rpup, rpupei;
 		u32 rpdown, rpdownei;
 
@@ -3087,7 +3088,7 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 
 	seq_printf(m, "GT awake? %s\n", yesno(dev_priv->gt.awake));
 	seq_printf(m, "Global active requests: %d\n",
-		   dev_priv->gt.active_requests);
+		   atomic_read(&dev_priv->gt.active_requests));
 	seq_printf(m, "CS timestamp frequency: %u kHz\n",
 		   RUNTIME_INFO(dev_priv)->cs_timestamp_frequency_khz);
 
@@ -3934,7 +3935,7 @@ i915_drop_caches_set(void *data, u64 val)
 
 	if (val & DROP_IDLE) {
 		do {
-			if (READ_ONCE(i915->gt.active_requests))
+			if (atomic_read(&i915->gt.active_requests))
 				flush_delayed_work(&i915->gt.retire_work);
 			drain_delayed_work(&i915->gt.idle_work);
 		} while (READ_ONCE(i915->gt.awake));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dc63303225fc..05bb2ceda6bd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1998,7 +1998,8 @@ struct drm_i915_private {
 		intel_engine_mask_t active_engines;
 		struct list_head active_rings;
 		struct list_head closed_vma;
-		u32 active_requests;
+		atomic_t active_requests;
+		struct mutex active_mutex;
 
 		/**
 		 * Is the GPU currently considered idle, or busy executing
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b38c9531b5e8..dcbe5bc6993b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -108,7 +108,7 @@ static void __i915_gem_park(struct drm_i915_private *i915)
 	GEM_TRACE("\n");
 
 	lockdep_assert_held(&i915->drm.struct_mutex);
-	GEM_BUG_ON(i915->gt.active_requests);
+	GEM_BUG_ON(atomic_read(&i915->gt.active_requests));
 	GEM_BUG_ON(!list_empty(&i915->gt.active_rings));
 
 	if (!i915->gt.awake)
@@ -148,8 +148,8 @@ void i915_gem_park(struct drm_i915_private *i915)
 {
 	GEM_TRACE("\n");
 
-	lockdep_assert_held(&i915->drm.struct_mutex);
-	GEM_BUG_ON(i915->gt.active_requests);
+	lockdep_assert_held(&i915->gt.active_mutex);
+	GEM_BUG_ON(atomic_read(&i915->gt.active_requests));
 
 	if (!i915->gt.awake)
 		return;
@@ -162,8 +162,8 @@ void i915_gem_unpark(struct drm_i915_private *i915)
 {
 	GEM_TRACE("\n");
 
-	lockdep_assert_held(&i915->drm.struct_mutex);
-	GEM_BUG_ON(!i915->gt.active_requests);
+	lockdep_assert_held(&i915->gt.active_mutex);
+	GEM_BUG_ON(!atomic_read(&i915->gt.active_requests));
 	assert_rpm_wakelock_held(i915);
 
 	if (i915->gt.awake)
@@ -2889,12 +2889,13 @@ i915_gem_idle_work_handler(struct work_struct *work)
 	if (!READ_ONCE(i915->gt.awake))
 		return;
 
-	if (READ_ONCE(i915->gt.active_requests))
+	if (atomic_read(&i915->gt.active_requests))
 		return;
 
 	rearm_hangcheck =
 		cancel_delayed_work_sync(&i915->gpu_error.hangcheck_work);
 
+	/* XXX need to support lockless kernel_context before removing! */
 	if (!mutex_trylock(&i915->drm.struct_mutex)) {
 		/* Currently busy, come back later */
 		mod_delayed_work(i915->wq,
@@ -2909,17 +2910,19 @@ i915_gem_idle_work_handler(struct work_struct *work)
 	 * while we are idle (such as the GPU being power cycled), no users
 	 * will be harmed.
 	 */
+	mutex_lock(&i915->gt.active_mutex);
 	if (!work_pending(&i915->gt.idle_work.work) &&
-	    !i915->gt.active_requests) {
-		++i915->gt.active_requests; /* don't requeue idle */
+	    !atomic_read(&i915->gt.active_requests)) {
+		atomic_inc(&i915->gt.active_requests); /* don't requeue idle */
 
 		switch_to_kernel_context_sync(i915, i915->gt.active_engines);
 
-		if (!--i915->gt.active_requests) {
+		if (atomic_dec_and_test(&i915->gt.active_requests)) {
 			__i915_gem_park(i915);
 			rearm_hangcheck = false;
 		}
 	}
+	mutex_unlock(&i915->gt.active_mutex);
 
 	mutex_unlock(&i915->drm.struct_mutex);
 
@@ -3069,7 +3072,7 @@ wait_for_timelines(struct drm_i915_private *i915,
 	struct i915_gt_timelines *gt = &i915->gt.timelines;
 	struct i915_timeline *tl;
 
-	if (!READ_ONCE(i915->gt.active_requests))
+	if (!atomic_read(&i915->gt.active_requests))
 		return timeout;
 
 	mutex_lock(&gt->mutex);
@@ -5043,6 +5046,7 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv)
 {
 	int err;
 
+	mutex_init(&dev_priv->gt.active_mutex);
 	INIT_LIST_HEAD(&dev_priv->gt.active_rings);
 	INIT_LIST_HEAD(&dev_priv->gt.closed_vma);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 060f5903544a..20e835a7f116 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -38,7 +38,7 @@ I915_SELFTEST_DECLARE(static struct igt_evict_ctl {
 
 static bool ggtt_is_idle(struct drm_i915_private *i915)
 {
-	return !i915->gt.active_requests;
+	return !atomic_read(&i915->gt.active_requests);
 }
 
 static int ggtt_flush(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 9533a85cb0b3..cc2e83fc9c85 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -132,15 +132,26 @@ i915_request_remove_from_client(struct i915_request *request)
 
 static void reserve_gt(struct drm_i915_private *i915)
 {
-	if (!i915->gt.active_requests++)
+	if (atomic_add_unless(&i915->gt.active_requests, 1, 0))
+		return;
+
+	mutex_lock(&i915->gt.active_mutex);
+	if (!atomic_read(&i915->gt.active_requests)) {
 		i915_gem_unpark(i915);
+		smp_mb__before_atomic();
+	}
+	atomic_inc(&i915->gt.active_requests);
+	mutex_unlock(&i915->gt.active_mutex);
 }
 
 static void unreserve_gt(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(!i915->gt.active_requests);
-	if (!--i915->gt.active_requests)
-		i915_gem_park(i915);
+	if (!atomic_dec_and_mutex_lock(&i915->gt.active_requests,
+				       &i915->gt.active_mutex))
+		return;
+
+	i915_gem_park(i915);
+	mutex_unlock(&i915->gt.active_mutex);
 }
 
 static void advance_ring(struct i915_request *request)
@@ -1329,7 +1340,7 @@ void i915_retire_requests(struct drm_i915_private *i915)
 
 	lockdep_assert_held(&i915->drm.struct_mutex);
 
-	if (!i915->gt.active_requests)
+	if (!atomic_read(&i915->gt.active_requests))
 		return;
 
 	list_for_each_entry_safe(ring, tmp, &i915->gt.active_rings, active_link)
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 4399ef9ebf15..615becbe26b3 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -1525,9 +1525,9 @@ static int __igt_switch_to_kernel_context(struct drm_i915_private *i915,
 				return err;
 		}
 
-		if (i915->gt.active_requests) {
+		if (atomic_read(&i915->gt.active_requests)) {
 			pr_err("%d active requests remain after switching to kernel context, pass %d (%s) on %s engine%s\n",
-			       i915->gt.active_requests,
+			       atomic_read(&i915->gt.active_requests),
 			       pass, from_idle ? "idle" : "busy",
 			       __engine_name(i915, engines),
 			       is_power_of_2(engines) ? "" : "s");
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
index 971148fbe6f5..d0920666fd12 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
@@ -506,12 +506,14 @@ static void disable_retire_worker(struct drm_i915_private *i915)
 	i915_gem_shrinker_unregister(i915);
 
 	mutex_lock(&i915->drm.struct_mutex);
-	if (!i915->gt.active_requests++) {
+	mutex_lock(&i915->gt.active_mutex);
+	if (!atomic_fetch_inc(&i915->gt.active_requests)) {
 		intel_wakeref_t wakeref;
 
 		with_intel_runtime_pm(i915, wakeref)
 			i915_gem_unpark(i915);
 	}
+	mutex_unlock(&i915->gt.active_mutex);
 	mutex_unlock(&i915->drm.struct_mutex);
 
 	cancel_delayed_work_sync(&i915->gt.retire_work);
@@ -616,10 +618,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
 	drm_mm_remove_node(&resv);
 out_park:
 	mutex_lock(&i915->drm.struct_mutex);
-	if (--i915->gt.active_requests)
-		queue_delayed_work(i915->wq, &i915->gt.retire_work, 0);
-	else
+	if (atomic_dec_and_test(&i915->gt.active_requests))
 		queue_delayed_work(i915->wq, &i915->gt.idle_work, 0);
+	else
+		queue_delayed_work(i915->wq, &i915->gt.retire_work, 0);
 	mutex_unlock(&i915->drm.struct_mutex);
 	i915_gem_shrinker_register(i915);
 	return err;
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 54cfb611c0aa..fe8252d0e73c 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -44,7 +44,7 @@ void mock_device_flush(struct drm_i915_private *i915)
 		mock_engine_flush(engine);
 
 	i915_retire_requests(i915);
-	GEM_BUG_ON(i915->gt.active_requests);
+	GEM_BUG_ON(atomic_read(&i915->gt.active_requests));
 }
 
 static void mock_device_release(struct drm_device *dev)
@@ -203,6 +203,7 @@ struct drm_i915_private *mock_gem_device(void)
 
 	i915_timelines_init(i915);
 
+	mutex_init(&i915->gt.active_mutex);
 	INIT_LIST_HEAD(&i915->gt.active_rings);
 	INIT_LIST_HEAD(&i915->gt.closed_vma);
 
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list