Re: [PATCH 01/10] drm/i915: Only attempt to scan the requested number of shrinker slabs

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/10] drm/i915: Only attempt to scan the requested number of shrinker slabs
Date: Thu, 18 Jan 2018 09:16:46 +0000	[thread overview]
Message-ID: <151626700678.1487.16405512042248839565@mail.alporthouse.com> (raw)
In-Reply-To: <b80dbeae-dd31-0b98-5010-635b410f8536@linux.intel.com>

Quoting Tvrtko Ursulin (2018-01-17 10:29:51)
> 
> On 15/01/2018 21:24, Chris Wilson wrote:
> > Since commit 4e773c3a8a69 ("drm/i915: Wire up shrinkctl->nr_scanned"),
> 
> Sounds like it deserves this in the Fixes: tag as well?

Not really, the commit maintained the old behaviour just missing an
opportunity to take advantage of the new interface. Not strictly a fix,
I'd rather let it percolate upstream slowly.

> > we track the number of objects we scan and do not wish to exceed that as
> > it will overly penalise our own slabs under mempressure. Given that we
> > now know the target number of objects to scan, use that as our guide for
> > deciding to shrink as opposed to the number of objects we manage to
> > shrink (which doesn't correspond to the numbers we report to shrinkctl).
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > index 9029ed04879c..0e158f9287c4 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -363,13 +363,13 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
> >                               I915_SHRINK_BOUND |
> >                               I915_SHRINK_UNBOUND |
> >                               I915_SHRINK_PURGEABLE);
> > -     if (freed < sc->nr_to_scan)
> > +     if (sc->nr_scanned < sc->nr_to_scan)
> >               freed += i915_gem_shrink(i915,
> >                                        sc->nr_to_scan - sc->nr_scanned,
> >                                        &sc->nr_scanned,
> >                                        I915_SHRINK_BOUND |
> >                                        I915_SHRINK_UNBOUND);
> > -     if (freed < sc->nr_to_scan && current_is_kswapd()) {
> > +     if (sc->nr_scanned < sc->nr_to_scan && current_is_kswapd()) {
> >               intel_runtime_pm_get(i915);
> >               freed += i915_gem_shrink(i915,
> >                                        sc->nr_to_scan - sc->nr_scanned,
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks, pushed this separately.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx