From patchwork Thu Feb 15 13:45:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 10221269 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 3745460467 for ; Thu, 15 Feb 2018 13:45:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 28D9329271 for ; Thu, 15 Feb 2018 13:45:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1D71B29330; Thu, 15 Feb 2018 13:45: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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID 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 174942932A for ; Thu, 15 Feb 2018 13:45:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5B14D89C51; Thu, 15 Feb 2018 13:45:20 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4411189C51 for ; Thu, 15 Feb 2018 13:45:19 +0000 (UTC) Received: by mail-wm0-x243.google.com with SMTP id v71so939107wmv.2 for ; Thu, 15 Feb 2018 05:45:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ursulin-net.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=oRIAoexW6tjF4ABQhJExbDNGEJ9mbQZw3QccDHPQyzs=; b=Z/cVvkAYmKWX37K54q+JpRpYX54yRdXKio7SylWIn05eARjeazlEkoQRG57oqrZIGY BKsV84PQOU4AdrC704DSM0fAHjqCUk/1wPfnpI2XUC+optrme+LlUCl0xeyGl9Ot40c1 640vKY47nSU3LgPjvDGNnOHqGKpT1HIW16CorWM0V3VIBeNJDp/BZ0TdPHyjfoQylLAV vzgsTBQyKmVCuiirOiTcKUgHlzAND59ERQIFgrZtflwxC2Fkq9LyOgIVmACLZSWdvCFM qgw6IgR8OdPh0Rj9V+dM2BsBl5oVYyqb+0eEMA9A00vpn7tM1yVzmHwOSbrD3WRqUwbV djlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=oRIAoexW6tjF4ABQhJExbDNGEJ9mbQZw3QccDHPQyzs=; b=MYmK17qftVsZknLPIqzjT1F2gEQKyNG+h2rPAGUKJwXxKIp1cRk3kO8j+9QESDTV+x VybEKqb5iHPJTJtpOTdtU2k+QG5nN7WQVb7Ki2JFW5240CY9jlmmOXFLvJ2p3ZqletUB QYkYUx+MZ7B9xD66ziZCirjCdYLzGFfcoXEGMEnj1C2OPMVUfvuxawF5sYrEm20s6z23 s5nBXxlAo4R7cUndemJqnGwcqtglLX2i36Jv90KM/bNNWutQnOQ2fmVkESFGRXluaYX+ rT4oA+VP3tA3rvb+NPw61GenwixlNV+XKvCf8rZIhrUoexe5mYoPKPzhQMP/wDcVObYI O8RA== X-Gm-Message-State: APf1xPAaM7hwl41YbYTJ2FHuW7cEq5DKJALdULmUdnpMSadq6phT00UK g2ZStsP8kX3GT1D83TRfN+4pI8hp X-Google-Smtp-Source: AH8x224fD2eTcrfa288sUkBCXhhx9s8jRDrOAPM7mG63qUaNNxZwrLYRrT6v1RpjZawVL92J3e1+7w== X-Received: by 10.28.166.195 with SMTP id p186mr1990720wme.81.1518702317414; Thu, 15 Feb 2018 05:45:17 -0800 (PST) Received: from localhost.localdomain ([95.146.144.186]) by smtp.gmail.com with ESMTPSA id v20sm14727934wrd.32.2018.02.15.05.45.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Feb 2018 05:45:16 -0800 (PST) From: Tvrtko Ursulin X-Google-Original-From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Thu, 15 Feb 2018 13:45:08 +0000 Message-Id: <20180215134508.32398-1-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <151869627300.15373.12921987748435843882@mail.alporthouse.com> References: <151869627300.15373.12921987748435843882@mail.alporthouse.com> Subject: [Intel-gfx] [PATCH v2] drm/i915: Use seqlock in engine stats 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: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Tvrtko Ursulin We can convert engine stats from a spinlock to seqlock to ensure interrupt processing is never even a tiny bit delayed by parallel readers. There is a smidgen bit more cost on the write lock side, and an extremely unlikely chance that readers will have to retry a few times in face of heavy interrupt load.Bbut it should be extremely unlikely given how lightweight read side section is compared to the interrupt processing side, and also compared to the rest of the code paths which can lead into it. v2: Relax locking to reflect API usage is now from process context and tasklet. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Suggested-by: Chris Wilson Cc: Chris Wilson --- Note the call path to intel_engine_context_out from execlists_cancel_requests which is not from the tasklet, but happens to run with interrupts disabled. So should work buty perhaps not the most obvious. --- drivers/gpu/drm/i915/intel_engine_cs.c | 22 ++++++++++------------ drivers/gpu/drm/i915/intel_ringbuffer.h | 16 +++++++--------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index f3c5100d629e..92a6e82304f9 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -246,7 +246,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv, /* Nothing to do here, execute in order of dependencies */ engine->schedule = NULL; - spin_lock_init(&engine->stats.lock); + seqlock_init(&engine->stats.lock); ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); @@ -1968,14 +1968,13 @@ intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance) int intel_enable_engine_stats(struct intel_engine_cs *engine) { struct intel_engine_execlists *execlists = &engine->execlists; - unsigned long flags; int err = 0; if (!intel_engine_supports_stats(engine)) return -ENODEV; tasklet_disable(&execlists->tasklet); - spin_lock_irqsave(&engine->stats.lock, flags); + write_seqlock_bh(&engine->stats.lock); if (unlikely(engine->stats.enabled == ~0)) { err = -EBUSY; @@ -1999,7 +1998,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine) } unlock: - spin_unlock_irqrestore(&engine->stats.lock, flags); + write_sequnlock_bh(&engine->stats.lock); tasklet_enable(&execlists->tasklet); return err; @@ -2028,12 +2027,13 @@ static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine) */ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine) { + unsigned int seq; ktime_t total; - unsigned long flags; - spin_lock_irqsave(&engine->stats.lock, flags); - total = __intel_engine_get_busy_time(engine); - spin_unlock_irqrestore(&engine->stats.lock, flags); + do { + seq = read_seqbegin(&engine->stats.lock); + total = __intel_engine_get_busy_time(engine); + } while (read_seqretry(&engine->stats.lock, seq)); return total; } @@ -2046,18 +2046,16 @@ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine) */ void intel_disable_engine_stats(struct intel_engine_cs *engine) { - unsigned long flags; - if (!intel_engine_supports_stats(engine)) return; - spin_lock_irqsave(&engine->stats.lock, flags); + write_seqlock_bh(&engine->stats.lock); WARN_ON_ONCE(engine->stats.enabled == 0); if (--engine->stats.enabled == 0) { engine->stats.total = __intel_engine_get_busy_time(engine); engine->stats.active = 0; } - spin_unlock_irqrestore(&engine->stats.lock, flags); + write_sequnlock_bh(&engine->stats.lock); } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 51523ad049de..68f273cde012 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -3,6 +3,8 @@ #define _INTEL_RINGBUFFER_H_ #include +#include + #include "i915_gem_batch_pool.h" #include "i915_gem_request.h" #include "i915_gem_timeline.h" @@ -567,7 +569,7 @@ struct intel_engine_cs { /** * @lock: Lock protecting the below fields. */ - spinlock_t lock; + seqlock_t lock; /** * @enabled: Reference count indicating number of listeners. */ @@ -1014,12 +1016,10 @@ intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance); static inline void intel_engine_context_in(struct intel_engine_cs *engine) { - unsigned long flags; - if (READ_ONCE(engine->stats.enabled) == 0) return; - spin_lock_irqsave(&engine->stats.lock, flags); + write_seqlock(&engine->stats.lock); if (engine->stats.enabled > 0) { if (engine->stats.active++ == 0) @@ -1027,17 +1027,15 @@ static inline void intel_engine_context_in(struct intel_engine_cs *engine) GEM_BUG_ON(engine->stats.active == 0); } - spin_unlock_irqrestore(&engine->stats.lock, flags); + write_sequnlock(&engine->stats.lock); } static inline void intel_engine_context_out(struct intel_engine_cs *engine) { - unsigned long flags; - if (READ_ONCE(engine->stats.enabled) == 0) return; - spin_lock_irqsave(&engine->stats.lock, flags); + write_seqlock(&engine->stats.lock); if (engine->stats.enabled > 0) { ktime_t last; @@ -1064,7 +1062,7 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine) } } - spin_unlock_irqrestore(&engine->stats.lock, flags); + write_sequnlock(&engine->stats.lock); } int intel_enable_engine_stats(struct intel_engine_cs *engine);