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 <ch...@chris-wilson.co.uk>
> 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 <ch...@chris-wilson.co.uk>
> Tested-by: Guo Jinxian <jinxianx....@intel.com>

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to