From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com ([192.55.52.120]:39335 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966391AbcKAOaA (ORCPT ); Tue, 1 Nov 2016 10:30:00 -0400 Subject: Re: [Intel-gfx] [PATCH] drm/i915: Allow shrinking of userptr objects once again To: Chris Wilson , Tvrtko Ursulin , Intel-gfx@lists.freedesktop.org, Tvrtko Ursulin , Joonas Lahtinen , Mika Kuoppala , stable@vger.kernel.org References: <1478006886-4489-1-git-send-email-tvrtko.ursulin@linux.intel.com> <20161101135250.GA26576@nuc-i3427.alporthouse.com> From: Tvrtko Ursulin Message-ID: Date: Tue, 1 Nov 2016 14:29:39 +0000 MIME-Version: 1.0 In-Reply-To: <20161101135250.GA26576@nuc-i3427.alporthouse.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: 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 >> >> 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 >> Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap") >> Cc: Chris Wilson >> Cc: Joonas Lahtinen >> Cc: Mika Kuoppala >> Cc: >> --- >> 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. Maybe I915_GEM_OBJECT_IS_SHRINKABLE, fully dumb and explicit? Regards, Tvrtko