[2/3] drm/i915: Safely acquire the struct_mutex in our mem shrinker

Commit Message

Chris Wilson Feb. 21, 2014, 6:05 p.m. UTC

Under memory pressure it is important that we strive to unpin some of
our objects so that their backing pages can be swapped out. However, we
have to avoid recursion in struct_mutex and also make sure that we
safely wait upon the device and driver. The first part is done by first
using a trylock and checking for recursion, but the latter was ignored.
However, we can use i915_mutex_interruptible() for a safe locking
strategy that should prevent deadlocks.
---
 drivers/gpu/drm/i915/i915_gem.c | 49 +++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d0cd9df4c1f7..3a620779057c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4873,13 +4873,28 @@  int i915_gem_open(struct drm_device *dev, struct drm_file *file)
 	return ret;
 }
 
-static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task)
+static bool
+i915_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
 {
-	if (!mutex_is_locked(mutex))
-		return false;
+	*unlock = true;
+	if (mutex_trylock(&dev->struct_mutex))
+		return true;
 
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_MUTEXES)
-	return mutex->owner == task;
+	if (dev->struct_mutex.owner == current) {
+		if (to_i915(dev)->mm.shrinker_no_lock_stealing)
+			return false;
+
+		*unlock = false;
+	} else {
+		/* lockdep classifies this as an ABBA deadlock,
+		 * but given the lock stealing above this is
+		 * safe. So treat this a separate subclass.
+		 */
+		mutex_lock_nested(&dev->struct_mutex, 1);
+	}
+
+	return true;
 #else
 	/* Since UP may be pre-empted, we cannot assume that we own the lock */
 	return false;
@@ -4908,18 +4923,11 @@  i915_gem_inactive_count(struct shrinker *shrinker, struct shrink_control *sc)
 			     mm.inactive_shrinker);
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_i915_gem_object *obj;
-	bool unlock = true;
 	unsigned long count;
+	bool unlock;
 
-	if (!mutex_trylock(&dev->struct_mutex)) {
-		if (!mutex_is_locked_by(&dev->struct_mutex, current))
-			return 0;
-
-		if (dev_priv->mm.shrinker_no_lock_stealing)
-			return 0;
-
-		unlock = false;
-	}
+	if (!i915_gem_shrinker_lock(dev, &unlock))
+		return 0;
 
 	count = 0;
 	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
@@ -5012,17 +5020,10 @@  i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc)
 			     mm.inactive_shrinker);
 	struct drm_device *dev = dev_priv->dev;
 	unsigned long freed;
-	bool unlock = true;
-
-	if (!mutex_trylock(&dev->struct_mutex)) {
-		if (!mutex_is_locked_by(&dev->struct_mutex, current))
-			return SHRINK_STOP;
+	bool unlock;
 
-		if (dev_priv->mm.shrinker_no_lock_stealing)
-			return SHRINK_STOP;
-
-		unlock = false;
-	}
+	if (!i915_gem_shrinker_lock(dev, &unlock))
+		return SHRINK_STOP;
 
 	freed = i915_gem_purge(dev_priv, sc->nr_to_scan);
 	if (freed < sc->nr_to_scan)