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

[Chris Wilson]

Quoting Matthew Auld (2020-03-23 13:53:13)
> On Mon, 23 Mar 2020 at 13:17, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > 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.
> 
> Ok. My rough thinking was that the shrinker page accounting(i.e what
> we return) is currently based on obj->base.size, which might be pure
> lies for huge_gem_object.
> 
> >
> > 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.
> 
> Do you mean WILLNEED and IS_SHRINKABLE, if object doesn't also support
> swapping? DONTNEED and IS_SHRINKABLE is correct for volatile objects.
> 
> GEM_BUG_ON(is_shrinkable(obj) && !is_swappable(obj) && obj->mm.madv ==
> WILLNEED);

For the moment, we can just drop IS_SHRINKABLE here. But there is room
for a huge object that can take part in shrinker ops by being volatile.

But for the moment there's no pressing need, so we can defer the API
problem. (I'm thinking we'll need to expose i915_gem_object_madvise.)
-Chris