From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Only discard backing storage on releasing the last ref Date: Thu, 22 May 2014 10:26:20 +0200 Message-ID: <20140522082619.GO14357@phenom.ffwll.local> References: <1400746612-26135-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 572ED6E3D9 for ; Thu, 22 May 2014 01:26:24 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id d17so2323347eek.2 for ; Thu, 22 May 2014 01:26:23 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1400746612-26135-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, May 22, 2014 at 09:16:52AM +0100, Chris Wilson wrote: > Before purging our pages (as opposed to copying back the contents from > the GPU), make sure that there is not an exposed CPU mmapping through > which the user can inspect the results. > > Regression from > > commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd > Author: Chris Wilson > Date: Tue Mar 25 13:23:06 2014 +0000 > > drm/i915: Invalidate our pages under memory pressure > > Testcase: igt/gem_mmap/new-object > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79005 > Signed-off-by: Chris Wilson > Tested-by: Guo Jinxian Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_gem.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 9cb7c1908279..31b9a04bd543 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4668,6 +4668,30 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev, > return obj; > } > > +static bool discard_backing_storage(struct drm_i915_gem_object *obj) > +{ > + /* If we are the last user of the backing storage (be it shmemfs > + * pages or stolen etc), we know that the pages are going to be > + * immediately released. In this case, we can then skip copying > + * back the contents from the GPU. > + */ > + > + if (obj->madv != I915_MADV_WILLNEED) > + return false; > + > + if (obj->base.filp == NULL) > + return true; > + > + /* At first glance, this looks racy, but then again so would be > + * userspace racing mmap against close. However, the first external > + * reference to the filp can only be obtained through the > + * i915_gem_mmap_ioctl() which safeguards us against the user > + * acquiring such a reference whilst we are in the middle of > + * freeing the object. > + */ > + return atomic_long_read(&obj->base.filp->f_count) == 1; > +} > + > void i915_gem_free_object(struct drm_gem_object *gem_obj) > { > struct drm_i915_gem_object *obj = to_intel_bo(gem_obj); > @@ -4705,7 +4729,7 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) > > if (WARN_ON(obj->pages_pin_count)) > obj->pages_pin_count = 0; > - if (obj->madv != __I915_MADV_PURGED) > + if (discard_backing_storage(obj)) > obj->madv = I915_MADV_DONTNEED; > i915_gem_object_put_pages(obj); > i915_gem_object_free_mmap_offset(obj); > -- > 2.0.0.rc2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch