diff mbox

drm/i915: optionally disable shrinker lock stealing

Message ID 1355918915-12938-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Dec. 19, 2012, 12:08 p.m. UTC
commit 5774506f157a91400c587b85d1ce4de56f0d32f6
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Nov 21 13:04:04 2012 +0000

    drm/i915: Borrow our struct_mutex for the direct reclaim

added a nice trick to steal the struct_mutex lock in the shrinker if
it's the current task holding it. But this also caused the requirement
that every place which allocates memory needs to be careful about the
gem state of objects, since the shrinker could have pulled the rug out
from under it. We've usually solved this by carefully preallocating
things or ensure that buffers are pinned already.

But the shrinker also reaps mmap offset, so allocating those needs to
be careful, too. Now that code has been factored out into some common
helpers, so either we have fragile code depending upon the common
helper not doing something we don't want it to do. Or we need to
reimplement the mmap offset creation and so also leak implementation
details into our code.

Since this all results in leaky abstraction, cop out by disabling the
lock borrowing trick while calling down into the helpers. That way our
craziness is nicely confined to files in drm/i915.

This should fix igt/gem_tiled_swapping.

Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 16 +++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

Comments

Chris Wilson Dec. 19, 2012, 12:22 p.m. UTC | #1
On Wed, 19 Dec 2012 13:08:35 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> commit 5774506f157a91400c587b85d1ce4de56f0d32f6
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Wed Nov 21 13:04:04 2012 +0000
> 
>     drm/i915: Borrow our struct_mutex for the direct reclaim

If you choose to take this path, do it as two patches so that we can
simply remove the mmap_offset wrapping. I still think this is a much
greater kludge than fixing the search/allocation ordering.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2ab476d..87747da 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -807,6 +807,7 @@  typedef struct drm_i915_private {
 		struct i915_hw_ppgtt *aliasing_ppgtt;
 
 		struct shrinker inactive_shrinker;
+		bool shrinker_no_lock_stealing;
 
 		/**
 		 * List of objects currently involved in rendering.
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9530592..4d3605c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1514,9 +1514,11 @@  static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
 	if (obj->base.map_list.map)
 		return 0;
 
+	dev_priv->mm.shrinker_no_lock_stealing = true;
+
 	ret = drm_gem_create_mmap_offset(&obj->base);
 	if (ret != -ENOSPC)
-		return ret;
+		goto out;
 
 	/* Badly fragmented mmap space? The only way we can recover
 	 * space is by destroying unwanted objects. We can't randomly release
@@ -1528,10 +1530,15 @@  static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
 	i915_gem_purge(dev_priv, obj->base.size >> PAGE_SHIFT);
 	ret = drm_gem_create_mmap_offset(&obj->base);
 	if (ret != -ENOSPC)
-		return ret;
+		goto out;
 
 	i915_gem_shrink_all(dev_priv);
-	return drm_gem_create_mmap_offset(&obj->base);
+	ret = drm_gem_create_mmap_offset(&obj->base);
+
+	dev_priv->mm.shrinker_no_lock_stealing = false;
+
+out:
+	return ret;
 }
 
 static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
@@ -4400,6 +4407,9 @@  i915_gem_inactive_shrink(struct shrinker *shrinker, struct shrink_control *sc)
 		if (!mutex_is_locked_by(&dev->struct_mutex, current))
 			return 0;
 
+		if (dev_priv->mm.shrinker_no_lock_stealing)
+			return 0;
+
 		unlock = false;
 	}