[Intel-gfx] [PATCH] drm/i915/selftests: mark huge_gem_object as not shrinkable

[Chris Wilson]

Quoting Matthew Auld (2020-03-23 13:08:21)
> It looks like the callers expect a non-volatile object, but it looks the
> shrinker will discard the object pages anyway, thinking that the pages
> can be swapped out if the object is marked as WILLNEED. If that's true
> then it might be better to mark it as volatile and fix the callers
> instead, but on the other hand huge_gem_objects are fairly unique in
> that they duplicate pages for the backing store, so maybe shrinking is
> not that applicable.

Duplication of backing store is irrelevant for the shrinker -- it just
deals with trying to make room by releasing objects. If we release the
entire object, all duplicate references are released and the pages
become recoverable.

Now as to whether the callers were expecting the object to be volatile
(for the backing pages to be discarded on swapping) is another question.
The answer would be that originally it was used with perma-pinned pages,
so it was never a problem. But looking at the users, they do *not*
expect to lose data on swapping.

So we need to fix the huge object to not gleefully throw away data,
which also means that we cannot shrink it (as there is no backing
storage to copy the pages to).

So both making the pages as DONTNEED and IS_SHRINKABLE are technically
incorrect.

> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> ---
>  drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c b/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c
> index fa16f2c3f3ac..2b46c6530da9 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/huge_gem_object.c
> @@ -88,8 +88,7 @@ static void huge_put_pages(struct drm_i915_gem_object *obj,
>  }
>  
>  static const struct drm_i915_gem_object_ops huge_ops = {
> -       .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
> -                I915_GEM_OBJECT_IS_SHRINKABLE,
> +       .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE,

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

but I think the changelog can be clarified and we either include the
DONTNEED fixes or follow up.
-Chris