Re: [PATCH v2] drm/i915/gem: Pin the L-shape quirked object as unshrinkable

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, May 13, 2021 at 01:07:56PM +0100, Matthew Auld wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> When instantiating a tiled object on an L-shaped memory machine, we mark
> the object as unshrinkable to prevent the shrinker from trying to swap
> out the pages. We have to do this as we do not know the swizzling on the
> individual pages, and so the data will be scrambled across swap out/in.
> 
> Not only do we need to move the object off the shrinker list, we need to
> mark the object with shrink_pin so that the counter is consistent across
> calls to madvise.
> 
> v2: in the madvise ioctl we need to check if the object is currently
> shrinkable/purgeable, not if the object type supports shrinking
> 
> Fixes: 0175969e489a ("drm/i915/gem: Use shrinkable status for unknown swizzle quirks")
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/3293
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>

This one seems to fare better than v1.

Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_pages.c |  2 ++
>  drivers/gpu/drm/i915/i915_gem.c           | 11 +++++------
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> index aed8a37ccdc9..7361971c177d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> @@ -63,6 +63,8 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
>  	    i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
>  		GEM_BUG_ON(i915_gem_object_has_tiling_quirk(obj));
>  		i915_gem_object_set_tiling_quirk(obj);
> +		GEM_BUG_ON(!list_empty(&obj->mm.link));
> +		atomic_inc(&obj->mm.shrink_pin);
>  		shrinkable = false;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d0018c5f88bd..cffd7f4f87dc 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1009,12 +1009,11 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>  		obj->mm.madv = args->madv;
>  
>  	if (i915_gem_object_has_pages(obj)) {
> -		struct list_head *list;
> +		unsigned long flags;
>  
> -		if (i915_gem_object_is_shrinkable(obj)) {
> -			unsigned long flags;
> -
> -			spin_lock_irqsave(&i915->mm.obj_lock, flags);
> +		spin_lock_irqsave(&i915->mm.obj_lock, flags);
> +		if (!list_empty(&obj->mm.link)) {
> +			struct list_head *list;
>  
>  			if (obj->mm.madv != I915_MADV_WILLNEED)
>  				list = &i915->mm.purge_list;
> @@ -1022,8 +1021,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>  				list = &i915->mm.shrink_list;
>  			list_move_tail(&obj->mm.link, list);
>  
> -			spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
>  		}
> +		spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
>  	}
>  
>  	/* if the object is no longer attached, discard its backing storage */
> -- 
> 2.26.3

-- 
Ville Syrjälä
Intel