On Tue, Nov 01, 2016 at 02:29:39PM +0000, Tvrtko Ursulin wrote:
>
> On 01/11/2016 13:52, Chris Wilson wrote:
> >On Tue, Nov 01, 2016 at 01:28:06PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >>
> >>Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
> >>are backed by swap") stopped considering the userptr objects
> >>in shrinker callbacks.
> >>
> >>Restore that so idle userptr objects can be discarded in order
> >>to free up memory.
> >>
> >>One change further to what was introduced in 1bec9b0bda3d is
> >>to start considering userptr objects in oom but that should
> >>also be a correct thing to do.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >>Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
> >>Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >>Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> >>Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> >>Cc: <stable@xxxxxxxxxxxxxxx>
> >>---
> >> drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>index 0993afc0e725..dfca1f6b3630 100644
> >>--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>@@ -83,8 +83,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
> >> if (!obj->mm.pages)
> >> return false;
> >>
> >>- /* Only shmemfs objects are backed by swap */
> >>- if (!obj->base.filp)
> >>+ /* shmemfs and userptr objects are backed by swap */
> >>+ if (!obj->base.filp && !obj->userptr.mm)
> >
> >Hmm. Another sticking point if we want to use the union again.
> >
> >How about obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ?
> >Or I915_GEM_OBJECT_CAN_SWAP since we want this for i915_gem_internal.c
> >as well, and that techinically doesn't have a backing store but can be
> >reaped. Hmm.
> >
> >if (!(obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ||
> > obj->mm.madv == I915_MADV_DONTNEED))
>
> Hm I915_GEM_OBJECT_HAS_STRUCT_PAGE happens to be right - shm,
> userptr and internal. But that would be bad. Neither do I like
> HAS_BACKING_STORE.
Yeah, I was trying to keep the concept separate from has-struct-page
just in case it made it easier for future extension.
> Maybe I915_GEM_OBJECT_IS_SHRINKABLE, fully dumb and explicit?
Dumb describes me best, yup.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html