From patchwork Fri Jan 4 14:02:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 10748379 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4E9AF13BF for ; Fri, 4 Jan 2019 14:02:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3F43B2834A for ; Fri, 4 Jan 2019 14:02:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 331D328464; Fri, 4 Jan 2019 14:02:26 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 8CAF8283C3 for ; Fri, 4 Jan 2019 14:02:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1564B6EC72; Fri, 4 Jan 2019 14:02:22 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 30A996EC70 for ; Fri, 4 Jan 2019 14:02:21 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jan 2019 06:02:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,439,1539673200"; d="scan'208";a="114145814" Received: from unknown (HELO localhost.localdomain) ([10.252.3.166]) by fmsmga008.fm.intel.com with ESMTP; 04 Jan 2019 06:02:19 -0800 From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Fri, 4 Jan 2019 14:02:16 +0000 Message-Id: <20190104140217.17822-1-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.19.1 MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Reduce recursive mutex locking from the shrinker X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 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 From: Tvrtko Ursulin In two codepaths internal to the shrinker we know we will end up taking the resursive mutex path. It instead feels more elegant to avoid this altogether and not call mutex_trylock_recursive in those cases. We achieve this by extracting the shrinking part to __i915_gem_shrink, wrapped by struct mutex taking i915_gem_shrink. At the same time move the runtime pm reference taking to always be in the usual, before struct mutex, order. v2: * Don't use flags but split i915_gem_shrink into locked and unlocked. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_shrinker.c | 152 +++++++++++++---------- 1 file changed, 83 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index ea90d3a0d511..1ee5b08dab1b 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -117,36 +117,11 @@ static bool unsafe_drop_pages(struct drm_i915_gem_object *obj) return !i915_gem_object_has_pages(obj); } -/** - * i915_gem_shrink - Shrink buffer object caches - * @i915: 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 - * up to @target pages of main memory backing storage from buffer objects. - * Selection of the specific caches can be done with @flags. This is e.g. useful - * when purgeable objects should be removed from caches preferentially. - * - * Note that it's not guaranteed that released amount is actually available as - * free system memory - the pages might still be in-used to due to other reasons - * (like cpu mmaps) or the mm core has reused them before we could grab them. - * Therefore code that needs to explicitly shrink buffer objects caches (e.g. to - * avoid deadlocks in memory reclaim) must fall back to i915_gem_shrink_all(). - * - * Also note that any kind of pinning (both per-vma address space pins and - * backing storage pins at the buffer object level) result in the shrinker code - * having to skip the object. - * - * Returns: - * The number of pages of backing storage actually released. - */ unsigned long -i915_gem_shrink(struct drm_i915_private *i915, - unsigned long target, - unsigned long *nr_scanned, - unsigned flags) +__i915_gem_shrink(struct drm_i915_private *i915, + unsigned long target, + unsigned long *nr_scanned, + unsigned flags) { const struct { struct list_head *list; @@ -158,10 +133,8 @@ i915_gem_shrink(struct drm_i915_private *i915, }, *phase; unsigned long count = 0; unsigned long scanned = 0; - bool unlock; - if (!shrinker_lock(i915, &unlock)) - return 0; + lockdep_assert_held(&i915->drm.struct_mutex); /* * When shrinking the active list, also consider active contexts. @@ -177,18 +150,8 @@ i915_gem_shrink(struct drm_i915_private *i915, I915_WAIT_LOCKED, MAX_SCHEDULE_TIMEOUT); - trace_i915_gem_shrink(i915, target, flags); i915_retire_requests(i915); - /* - * Unbinding of objects will require HW access; Let us not wake the - * device just to recover a little memory. If absolutely necessary, - * we will force the wake during oom-notifier. - */ - if ((flags & I915_SHRINK_BOUND) && - !intel_runtime_pm_get_if_in_use(i915)) - flags &= ~I915_SHRINK_BOUND; - /* * As we may completely rewrite the (un)bound list whilst unbinding * (due to retiring requests) we have to strictly process only @@ -267,15 +230,70 @@ i915_gem_shrink(struct drm_i915_private *i915, spin_unlock(&i915->mm.obj_lock); } - if (flags & I915_SHRINK_BOUND) - intel_runtime_pm_put(i915); - i915_retire_requests(i915); - shrinker_unlock(i915, unlock); - if (nr_scanned) *nr_scanned += scanned; + + return count; +} + +/** + * i915_gem_shrink - Shrink buffer object caches + * @i915: 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 + * up to @target pages of main memory backing storage from buffer objects. + * Selection of the specific caches can be done with @flags. This is e.g. useful + * when purgeable objects should be removed from caches preferentially. + * + * Note that it's not guaranteed that released amount is actually available as + * free system memory - the pages might still be in-used to due to other reasons + * (like cpu mmaps) or the mm core has reused them before we could grab them. + * Therefore code that needs to explicitly shrink buffer objects caches (e.g. to + * avoid deadlocks in memory reclaim) must fall back to i915_gem_shrink_all(). + * + * Also note that any kind of pinning (both per-vma address space pins and + * backing storage pins at the buffer object level) result in the shrinker code + * having to skip the object. + * + * Returns: + * The number of pages of backing storage actually released. + */ +unsigned long +i915_gem_shrink(struct drm_i915_private *i915, + unsigned long target, + unsigned long *nr_scanned, + unsigned flags) +{ + unsigned long count = 0; + bool unlock; + + trace_i915_gem_shrink(i915, target, flags); + + /* + * Unbinding of objects will require HW access; Let us not wake the + * device just to recover a little memory. If absolutely necessary, + * we will force the wake during oom-notifier. + */ + if ((flags & I915_SHRINK_BOUND) && + !intel_runtime_pm_get_if_in_use(i915)) + flags &= ~I915_SHRINK_BOUND; + + if (!shrinker_lock(i915, &unlock)) + goto out; + + count = __i915_gem_shrink(i915, target, nr_scanned, flags); + + shrinker_unlock(i915, unlock); + +out: + if (flags & I915_SHRINK_BOUND) + intel_runtime_pm_put(i915); + return count; } @@ -352,6 +370,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) { struct drm_i915_private *i915 = container_of(shrinker, struct drm_i915_private, mm.shrinker); + const unsigned int flags = I915_SHRINK_BOUND | I915_SHRINK_UNBOUND; unsigned long freed; bool unlock; @@ -360,26 +379,21 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) if (!shrinker_lock(i915, &unlock)) return SHRINK_STOP; - freed = i915_gem_shrink(i915, - sc->nr_to_scan, - &sc->nr_scanned, - I915_SHRINK_BOUND | - I915_SHRINK_UNBOUND | - I915_SHRINK_PURGEABLE); + freed = __i915_gem_shrink(i915, + sc->nr_to_scan, + &sc->nr_scanned, + flags | I915_SHRINK_PURGEABLE); 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); + freed += __i915_gem_shrink(i915, + sc->nr_to_scan - sc->nr_scanned, + &sc->nr_scanned, + flags); 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, - &sc->nr_scanned, - I915_SHRINK_ACTIVE | - I915_SHRINK_BOUND | - I915_SHRINK_UNBOUND); + freed += __i915_gem_shrink(i915, + sc->nr_to_scan - sc->nr_scanned, + &sc->nr_scanned, + flags | I915_SHRINK_ACTIVE); intel_runtime_pm_put(i915); } @@ -477,11 +491,11 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr goto out; intel_runtime_pm_get(i915); - freed_pages += i915_gem_shrink(i915, -1UL, NULL, - I915_SHRINK_BOUND | - I915_SHRINK_UNBOUND | - I915_SHRINK_ACTIVE | - I915_SHRINK_VMAPS); + freed_pages += __i915_gem_shrink(i915, -1UL, NULL, + I915_SHRINK_BOUND | + I915_SHRINK_UNBOUND | + I915_SHRINK_ACTIVE | + I915_SHRINK_VMAPS); intel_runtime_pm_put(i915); /* We also want to clear any cached iomaps as they wrap vmap */ From patchwork Fri Jan 4 14:02:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 10748377 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6C72A13BF for ; Fri, 4 Jan 2019 14:02:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5BDD1283C3 for ; Fri, 4 Jan 2019 14:02:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5006E28464; Fri, 4 Jan 2019 14:02:25 +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=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 45FA3283C3 for ; Fri, 4 Jan 2019 14:02:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 106166EC70; Fri, 4 Jan 2019 14:02:22 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 68DF36EC70 for ; Fri, 4 Jan 2019 14:02:21 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jan 2019 06:02:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,439,1539673200"; d="scan'208";a="114145821" Received: from unknown (HELO localhost.localdomain) ([10.252.3.166]) by fmsmga008.fm.intel.com with ESMTP; 04 Jan 2019 06:02:20 -0800 From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Fri, 4 Jan 2019 14:02:17 +0000 Message-Id: <20190104140217.17822-2-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20190104140217.17822-1-tvrtko.ursulin@linux.intel.com> References: <20190104140217.17822-1-tvrtko.ursulin@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 2/2] drm/i915: Fix timeout handling in i915_gem_shrinker_vmap X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 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 From: Tvrtko Ursulin The code tries to grab struct mutex for 5ms every time the unlocked GPU idle wait succeeds. But the GPU idle wait itself is practically unbound which means the 5ms timeout might not be honoured. Cap the GPU idle wait to 5ms as well to fix this. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_shrinker.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 1ee5b08dab1b..e848f38223d6 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -404,13 +404,12 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) static bool shrinker_lock_uninterruptible(struct drm_i915_private *i915, bool *unlock, - int timeout_ms) + unsigned long timeout) { - unsigned long timeout = jiffies + msecs_to_jiffies_timeout(timeout_ms); + const unsigned long timeout_end = jiffies + timeout; do { - if (i915_gem_wait_for_idle(i915, - 0, MAX_SCHEDULE_TIMEOUT) == 0 && + if (i915_gem_wait_for_idle(i915, 0, timeout) == 0 && shrinker_lock(i915, unlock)) break; @@ -418,7 +417,7 @@ shrinker_lock_uninterruptible(struct drm_i915_private *i915, bool *unlock, if (fatal_signal_pending(current)) return false; - if (time_after(jiffies, timeout)) { + if (time_after(jiffies, timeout_end)) { pr_err("Unable to lock GPU to purge memory.\n"); return false; } @@ -476,11 +475,12 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr struct drm_i915_private *i915 = container_of(nb, struct drm_i915_private, mm.vmap_notifier); struct i915_vma *vma, *next; + const unsigned long timeout = msecs_to_jiffies_timeout(5000); unsigned long freed_pages = 0; bool unlock; int ret; - if (!shrinker_lock_uninterruptible(i915, &unlock, 5000)) + if (!shrinker_lock_uninterruptible(i915, &unlock, timeout)) return NOTIFY_DONE; /* Force everything onto the inactive lists */