| Message ID | 1355924026-13159-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 19 Dec 2012 14:33:46 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > The mmap offset structure is not part of the drm/i915 code, but > provided by gem helpers. To avoid leaky abstractions (by either > depending upon implementation details of said helper wrt to > preallocations, or reimplementing it in our code and so fuzzing > around in internal details of that helpr) simply disable > the shrinker lock stealing accross calls into the helper functions. > > This should fix igt/gem_tiled_swapping. Proving that if we just fixed the code stolen from us, we'd run less chance of introducing bugs.... :-p -Chris
On Wed, Dec 19, 2012 at 4:02 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Wed, 19 Dec 2012 15:40:16 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> The mmap offset structure is not part of the drm/i915 code, but >> provided by gem helpers. To avoid leaky abstractions (by either >> depending upon implementation details of said helper wrt to >> preallocations, or reimplementing it in our code and so fuzzing >> around in internal details of that helpr) simply disable >> the shrinker lock stealing accross calls into the helper functions. >> >> This should fix igt/gem_tiled_swapping. >> >> v2: Fix cleanup path confusion bemoaned by Chris Wilson. >> >> 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> > > You can have a Reviewed-by for the flag, and a > This-works-but-I-do-not-think-it-addresses-the-bug for this kludge. Merged both - I agree that this here is not a beauty, but it removes a tricky dependency on the helper code while that one seems to be a bit in flux. Beating the helper code a bit into shape cane be done on top, and if we embed the drm_mm_node (as we should imo) all allocations disapper. So this patch here would then have no effect any longer and we could drop it again. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index db4b470..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)
The mmap offset structure is not part of the drm/i915 code, but provided by gem helpers. To avoid leaky abstractions (by either depending upon implementation details of said helper wrt to preallocations, or reimplementing it in our code and so fuzzing around in internal details of that helpr) simply disable the shrinker lock stealing accross calls into the helper functions. 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_gem.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)