[RFT] too lax, or too false?

[Tvrtko Ursulin]

From: Chris Wilson <chris at chris-wilson.co.uk>

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 60 ++++++------------------
 1 file changed, 14 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index ea90d3a0d511..408f6d54f9fe 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -36,32 +36,14 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 
-static bool shrinker_lock(struct drm_i915_private *i915, bool *unlock)
+static bool shrinker_lock(struct drm_i915_private *i915)
 {
-	switch (mutex_trylock_recursive(&i915->drm.struct_mutex)) {
-	case MUTEX_TRYLOCK_RECURSIVE:
-		*unlock = false;
-		return true;
-
-	case MUTEX_TRYLOCK_FAILED:
-		*unlock = false;
-		preempt_disable();
-		do {
-			cpu_relax();
-			if (mutex_trylock(&i915->drm.struct_mutex)) {
-				*unlock = true;
-				break;
-			}
-		} while (!need_resched());
-		preempt_enable();
-		return *unlock;
-
-	case MUTEX_TRYLOCK_SUCCESS:
-		*unlock = true;
+	if (__mutex_owner(&i915->drm.struct_mutex) == current) {
+		return false;
+	} else {
+		mutex_lock_nested(&i915->drm.struct_mutex, I915_MM_SHRINKER);
 		return true;
 	}
-
-	BUG();
 }
 
 static void shrinker_unlock(struct drm_i915_private *i915, bool unlock)
@@ -160,8 +142,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
 	unsigned long scanned = 0;
 	bool unlock;
 
-	if (!shrinker_lock(i915, &unlock))
-		return 0;
+	unlock = shrinker_lock(i915);
 
 	/*
 	 * When shrinking the active list, also consider active contexts.
@@ -357,8 +338,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
 
 	sc->nr_scanned = 0;
 
-	if (!shrinker_lock(i915, &unlock))
-		return SHRINK_STOP;
+	unlock = shrinker_lock(i915);
 
 	freed = i915_gem_shrink(i915,
 				sc->nr_to_scan,
@@ -389,26 +369,14 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
 }
 
 static bool
-shrinker_lock_uninterruptible(struct drm_i915_private *i915, bool *unlock,
-			      int timeout_ms)
+shrinker_lock_idle(struct drm_i915_private *i915, bool *unlock, int timeout_ms)
 {
-	unsigned long timeout = jiffies + msecs_to_jiffies_timeout(timeout_ms);
-
-	do {
-		if (i915_gem_wait_for_idle(i915,
-					   0, MAX_SCHEDULE_TIMEOUT) == 0 &&
-		    shrinker_lock(i915, unlock))
-			break;
-
-		schedule_timeout_killable(1);
-		if (fatal_signal_pending(current))
-			return false;
+	if (i915_gem_wait_for_idle(i915, 0, msecs_to_jiffies(timeout_ms))) {
+		pr_err("Unable to idle GPU to purge memory.\n");
+		return false;
+	}
 
-		if (time_after(jiffies, timeout)) {
-			pr_err("Unable to lock GPU to purge memory.\n");
-			return false;
-		}
-	} while (1);
+	*unlock = shrinker_lock(i915);
 
 	return true;
 }
@@ -466,7 +434,7 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
 	bool unlock;
 	int ret;
 
-	if (!shrinker_lock_uninterruptible(i915, &unlock, 5000))
+	if (!shrinker_lock_idle(i915, &unlock, 5000))
 		return NOTIFY_DONE;
 
 	/* Force everything onto the inactive lists */
-- 
2.19.1