On Thu, May 13, 2021 at 01:07:56PM +0100, Matthew Auld wrote:
> From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
This one seems to fare better than v1.
Tested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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
