From patchwork Tue Aug 22 22:45:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 9916273 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BA729603F9 for ; Tue, 22 Aug 2017 22:45:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A648C287BD for ; Tue, 22 Aug 2017 22:45:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9B12B28877; Tue, 22 Aug 2017 22:45:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id AD49D287BD for ; Tue, 22 Aug 2017 22:45:54 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F2EA76E421; Tue, 22 Aug 2017 22:45:53 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4F3A06E421 for ; Tue, 22 Aug 2017 22:45:52 +0000 (UTC) Received: from akpm3.svl.corp.google.com (unknown [104.133.9.92]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id C9F3D92F; Tue, 22 Aug 2017 22:45:51 +0000 (UTC) Date: Tue, 22 Aug 2017 15:45:50 -0700 From: Andrew Morton To: Chris Wilson Message-Id: <20170822154550.33c8cc61c21e5ccf72959dd1@linux-foundation.org> In-Reply-To: <20170822135325.9191-2-chris@chris-wilson.co.uk> References: <20170815153010.e3cfc177af0b2c0dc421b84c@linux-foundation.org> <20170822135325.9191-1-chris@chris-wilson.co.uk> <20170822135325.9191-2-chris@chris-wilson.co.uk> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Cc: Hillf Danton , Michal Hocko , Minchan Kim , intel-gfx@lists.freedesktop.org, linux-mm@kvack.org, Shaohua Li , Johannes Weiner , Mel Gorman , Vlastimil Babka Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Wire up shrinkctl->nr_scanned X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 22 Aug 2017 14:53:25 +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. It's unclear which tree this is against but I think I got it all fixed up. Please check the changes to i915_gem_shrink(). From: Chris Wilson Subject: drm/i915: wire up shrinkctl->nr_scanned 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. Link: http://lkml.kernel.org/r/20170822135325.9191-2-chris@chris-wilson.co.uk Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Michal Hocko Cc: Johannes Weiner Cc: Hillf Danton Cc: Minchan Kim Cc: Vlastimil Babka Cc: Mel Gorman Cc: Shaohua Li Cc: Christoph Lameter Cc: David Rientjes Cc: Joonsoo Kim Cc: Pekka Enberg Signed-off-by: Andrew Morton --- drivers/gpu/drm/i915/i915_debugfs.c | 4 +-- drivers/gpu/drm/i915/i915_drv.h | 1 drivers/gpu/drm/i915/i915_gem.c | 4 +-- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 - drivers/gpu/drm/i915/i915_gem_shrinker.c | 24 +++++++++++++++------ 5 files changed, 24 insertions(+), 11 deletions(-) diff -puN drivers/gpu/drm/i915/i915_debugfs.c~drm-i915-wire-up-shrinkctl-nr_scanned drivers/gpu/drm/i915/i915_debugfs.c --- a/drivers/gpu/drm/i915/i915_debugfs.c~drm-i915-wire-up-shrinkctl-nr_scanned +++ a/drivers/gpu/drm/i915/i915_debugfs.c @@ -4333,10 +4333,10 @@ i915_drop_caches_set(void *data, u64 val lockdep_set_current_reclaim_state(GFP_KERNEL); if (val & DROP_BOUND) - i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_BOUND); + i915_gem_shrink(dev_priv, LONG_MAX, NULL, I915_SHRINK_BOUND); if (val & DROP_UNBOUND) - i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_UNBOUND); + i915_gem_shrink(dev_priv, LONG_MAX, NULL, I915_SHRINK_UNBOUND); if (val & DROP_SHRINK_ALL) i915_gem_shrink_all(dev_priv); diff -puN drivers/gpu/drm/i915/i915_drv.h~drm-i915-wire-up-shrinkctl-nr_scanned drivers/gpu/drm/i915/i915_drv.h --- a/drivers/gpu/drm/i915/i915_drv.h~drm-i915-wire-up-shrinkctl-nr_scanned +++ a/drivers/gpu/drm/i915/i915_drv.h @@ -3628,6 +3628,7 @@ i915_gem_object_create_internal(struct d /* i915_gem_shrinker.c */ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, unsigned long target, + unsigned long *nr_scanned, unsigned flags); #define I915_SHRINK_PURGEABLE 0x1 #define I915_SHRINK_UNBOUND 0x2 diff -puN drivers/gpu/drm/i915/i915_gem.c~drm-i915-wire-up-shrinkctl-nr_scanned drivers/gpu/drm/i915/i915_gem.c --- a/drivers/gpu/drm/i915/i915_gem.c~drm-i915-wire-up-shrinkctl-nr_scanned +++ a/drivers/gpu/drm/i915/i915_gem.c @@ -2408,7 +2408,7 @@ rebuild_st: goto err_sg; } - i915_gem_shrink(dev_priv, 2 * page_count, *s++); + i915_gem_shrink(dev_priv, 2 * page_count, NULL, *s++); cond_resched(); /* We've tried hard to allocate the memory by reaping @@ -5012,7 +5012,7 @@ int i915_gem_freeze_late(struct drm_i915 * the objects as well, see i915_gem_freeze() */ - i915_gem_shrink(dev_priv, -1UL, I915_SHRINK_UNBOUND); + i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND); i915_gem_drain_freed_objects(dev_priv); mutex_lock(&dev_priv->drm.struct_mutex); diff -puN drivers/gpu/drm/i915/i915_gem_gtt.c~drm-i915-wire-up-shrinkctl-nr_scanned drivers/gpu/drm/i915/i915_gem_gtt.c --- a/drivers/gpu/drm/i915/i915_gem_gtt.c~drm-i915-wire-up-shrinkctl-nr_scanned +++ a/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2061,7 +2061,7 @@ int i915_gem_gtt_prepare_pages(struct dr */ GEM_BUG_ON(obj->mm.pages == pages); } while (i915_gem_shrink(to_i915(obj->base.dev), - obj->base.size >> PAGE_SHIFT, + obj->base.size >> PAGE_SHIFT, NULL, I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE)); diff -puN drivers/gpu/drm/i915/i915_gem_shrinker.c~drm-i915-wire-up-shrinkctl-nr_scanned drivers/gpu/drm/i915/i915_gem_shrinker.c --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c~drm-i915-wire-up-shrinkctl-nr_scanned +++ a/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -136,6 +136,7 @@ static bool unsafe_drop_pages(struct drm * i915_gem_shrink - Shrink buffer object caches * @dev_priv: i915 device * @target: amount of memory to make available, in pages + * @nr_scanned: optional output for number of pages scanned (incremental) * @flags: control flags for selecting cache types * * This function is the main interface to the shrinker. It will try to release @@ -158,7 +159,9 @@ static bool unsafe_drop_pages(struct drm */ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, - unsigned long target, unsigned flags) + unsigned long target, + unsigned long *nr_scanned, + unsigned flags) { const struct { struct list_head *list; @@ -169,6 +172,7 @@ i915_gem_shrink(struct drm_i915_private { NULL, 0 }, }, *phase; unsigned long count = 0; + unsigned long scanned = 0; bool unlock; if (!shrinker_lock(dev_priv, &unlock)) @@ -249,6 +253,7 @@ i915_gem_shrink(struct drm_i915_private count += obj->base.size >> PAGE_SHIFT; } mutex_unlock(&obj->mm.lock); + scanned += obj->base.size >> PAGE_SHIFT; } } list_splice_tail(&still_in_list, phase->list); @@ -261,6 +266,8 @@ i915_gem_shrink(struct drm_i915_private shrinker_unlock(dev_priv, unlock); + if (nr_scanned) + *nr_scanned += scanned; return count; } @@ -283,7 +290,7 @@ unsigned long i915_gem_shrink_all(struct unsigned long freed; intel_runtime_pm_get(dev_priv); - freed = i915_gem_shrink(dev_priv, -1UL, + freed = i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE); @@ -329,23 +336,28 @@ i915_gem_shrinker_scan(struct shrinker * unsigned long freed; bool unlock; + sc->nr_scanned = 0; + if (!shrinker_lock(dev_priv, &unlock)) return SHRINK_STOP; freed = i915_gem_shrink(dev_priv, sc->nr_to_scan, + &sc->nr_scanned, I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE); if (freed < sc->nr_to_scan) freed += i915_gem_shrink(dev_priv, - sc->nr_to_scan - freed, + 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()) { intel_runtime_pm_get(dev_priv); freed += i915_gem_shrink(dev_priv, - sc->nr_to_scan - freed, + sc->nr_to_scan - sc->nr_scanned, + &sc->nr_scanned, I915_SHRINK_ACTIVE | I915_SHRINK_BOUND | I915_SHRINK_UNBOUND); @@ -354,7 +366,7 @@ i915_gem_shrinker_scan(struct shrinker * shrinker_unlock(dev_priv, unlock); - return freed; + return sc->nr_scanned ? freed : SHRINK_STOP; } static bool @@ -453,7 +465,7 @@ i915_gem_shrinker_vmap(struct notifier_b goto out; intel_runtime_pm_get(dev_priv); - freed_pages += i915_gem_shrink(dev_priv, -1UL, + freed_pages += i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_ACTIVE |