All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned
Date: Mon, 16 Oct 2017 15:57:04 +0300	[thread overview]
Message-ID: <1508158624.5300.42.camel@linux.intel.com> (raw)
In-Reply-To: <150815111581.6894.8065248839834661541@mail.alporthouse.com>

On Mon, 2017-10-16 at 11:51 +0100, Chris Wilson wrote:
> 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?

"References:" should be good.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-10-16 12:57 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
2017-10-13 20:26 ` [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global Chris Wilson
2017-10-16 10:40   ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display Chris Wilson
2017-10-16 10:41   ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 4/9] drm/i915: Remove walk over obj->vma_list for the shrinker Chris Wilson
2017-10-13 20:26 ` [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock Chris Wilson
2017-10-13 22:36   ` [PATCH v2] " Chris Wilson
2017-10-13 23:23     ` [PATCH v3] " Chris Wilson
2017-10-16 11:40   ` Chris Wilson
2017-10-13 20:26 ` [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned Chris Wilson
2017-10-16 10:46   ` Joonas Lahtinen
2017-10-16 10:51     ` Chris Wilson
2017-10-16 12:57       ` Joonas Lahtinen [this message]
2017-10-13 20:26 ` [PATCH 7/9] drm/i915: Set our shrinker->batch to 4096 (~16MiB) Chris Wilson
2017-10-13 20:26 ` [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation Chris Wilson
2017-10-16 10:54   ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 9/9] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects Chris Wilson
2017-10-13 21:46 ` ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages Patchwork
2017-10-13 22:32   ` Chris Wilson
2017-10-13 23:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev2) Patchwork
2017-10-13 23:51 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev3) Patchwork
2017-10-16 12:10 ` ✓ Fi.CI.BAT: success for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4) Patchwork
2017-10-16 20:12 ` ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1508158624.5300.42.camel@linux.intel.com \
    --to=joonas.lahtinen@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.