Re: [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned

Chris Wilson
2017-10-16T10:52:00Z

Quoting Joonas Lahtinen (2017-10-16 11:46:09)
> On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> > shrink_slab() allows us to report back the number of objects we
> > successfully scanned (out of the target shrinkctl->nr_to_scan). As
> > report the number of pages owned by each GEM object as a separate item
> > to the shrinker, we cannot precisely control the number of shrinker
> > objects we scan on each pass; and indeed may free more than requested.
> > If we fail to tell the shrinker about the number of objects we process,
> > it will continue to hold a grudge against us as any objects left
> > unscanned are added to the next reclaim -- and so we will keep on
> > "unfairly" shrinking our own slab in comparison to other slabs.
> >
> > v2: fixup the misplaced addition, we want to count everything we scan
> > (to match the number we reported earlier) not just the objects we
> > successfully validated and freed.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Umm, full explanation and "v2" is bit misleading. Should not this be
> one Fixes: if significant enough, or just a clear reference to
> 912d572d63b8 ("drm/i915: wire up shrinkctl->nr_scanned")?

It's the patch I had sent, and thought I acked for mm; I didn't check
carefully enough. The current code is more or less a no-op, the counter
is identical to free, so we don't change the reported value to
shrinkctl. This patch is required to make the code do as the commitlog
describes.

So the code isn't broken and we don't need a Fixes tag, maybe just a
References?
-Chris