From patchwork Fri May 8 13:39:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mika Kuoppala X-Patchwork-Id: 6365751 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E59609F1C2 for ; Fri, 8 May 2015 13:40:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 700FE201C8 for ; Fri, 8 May 2015 13:40:03 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 1CC16200E8 for ; Fri, 8 May 2015 13:40:02 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 989106E93B; Fri, 8 May 2015 06:40:01 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 57DF36E937 for ; Fri, 8 May 2015 06:40:00 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 08 May 2015 06:40:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,391,1427785200"; d="scan'208";a="707283816" Received: from rosetta.fi.intel.com (HELO rosetta) ([10.237.72.80]) by fmsmga001.fm.intel.com with ESMTP; 08 May 2015 06:39:59 -0700 Received: by rosetta (Postfix, from userid 1000) id EC2BE80057; Fri, 8 May 2015 16:39:57 +0300 (EEST) From: Mika Kuoppala To: intel-gfx@lists.freedesktop.org Date: Fri, 8 May 2015 16:39:54 +0300 Message-Id: <1431092395-23930-1-git-send-email-mika.kuoppala@intel.com> X-Mailer: git-send-email 1.9.1 Cc: miku@iki.fi Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Detach hangcheck from request lists 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: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hangcheck tries to peek into request list to see if the ring was busy or not. But that leads to race against the list addition in request submission. And hangcheck saw a ring being idle, when in fact work was just being submitted. There is strong desire to keep hangcheck without locks of any kind as it is our last line of defense against failures that surpass our imagination. Rework the hangcheck logic so that we find out the ring busyness by inspecting hw engine state and omit the request->list inspection. v2: start is u32, push for 80 cols (Chris) References: https://bugs.freedesktop.org/show_bug.cgi?id=89931 References: https://bugs.freedesktop.org/show_bug.cgi?id=89644 References: https://bugs.freedesktop.org/show_bug.cgi?id=88984 References: https://bugs.freedesktop.org/show_bug.cgi?id=88981 References: https://bugs.freedesktop.org/show_bug.cgi?id=87730 Cc: Chris Wilson Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_debugfs.c | 10 +- drivers/gpu/drm/i915/i915_gpu_error.c | 4 +- drivers/gpu/drm/i915/i915_irq.c | 214 ++++++++++++++++++++------------ drivers/gpu/drm/i915/intel_ringbuffer.h | 3 + 4 files changed, 149 insertions(+), 82 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index adbbdda..8c647bf 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1295,6 +1295,7 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_engine_cs *ring; u64 acthd[I915_NUM_RINGS]; + u32 start[I915_NUM_RINGS]; u32 seqno[I915_NUM_RINGS]; int i; @@ -1308,6 +1309,7 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) for_each_ring(ring, dev_priv, i) { seqno[i] = ring->get_seqno(ring, false); acthd[i] = intel_ring_get_active_head(ring); + start[i] = I915_READ_START(ring); } intel_runtime_pm_put(dev_priv); @@ -1328,8 +1330,14 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) (long long)acthd[i]); seq_printf(m, "\tmax ACTHD = 0x%08llx\n", (long long)ring->hangcheck.max_acthd); + seq_printf(m, "\tSTART = 0x%08x [current 0x%08x]\n", + ring->hangcheck.start, + start[i]); + seq_printf(m, "\tscore = %d\n", ring->hangcheck.score); - seq_printf(m, "\taction = %d\n", ring->hangcheck.action); + seq_printf(m, "\taction = %s [%d]\n", + i915_hangcheck_action_to_str(ring->hangcheck.action), + ring->hangcheck.action); } return 0; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index a3e330d..9c0db19 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -220,7 +220,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, } } -static const char *hangcheck_action_to_str(enum intel_ring_hangcheck_action a) +const char *i915_hangcheck_action_to_str(enum intel_ring_hangcheck_action a) { switch (a) { case HANGCHECK_IDLE: @@ -301,7 +301,7 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m, err_printf(m, " ring->head: 0x%08x\n", ring->cpu_ring_head); err_printf(m, " ring->tail: 0x%08x\n", ring->cpu_ring_tail); err_printf(m, " hangcheck: %s [%d]\n", - hangcheck_action_to_str(ring->hangcheck_action), + i915_hangcheck_action_to_str(ring->hangcheck_action), ring->hangcheck_score); } diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9da955e..a3244bd 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2685,20 +2685,6 @@ static void gen8_disable_vblank(struct drm_device *dev, int pipe) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } -static struct drm_i915_gem_request * -ring_last_request(struct intel_engine_cs *ring) -{ - return list_entry(ring->request_list.prev, - struct drm_i915_gem_request, list); -} - -static bool -ring_idle(struct intel_engine_cs *ring) -{ - return (list_empty(&ring->request_list) || - i915_gem_request_completed(ring_last_request(ring), false)); -} - static bool ipehr_is_semaphore_wait(struct drm_device *dev, u32 ipehr) { @@ -2882,6 +2868,109 @@ ring_stuck(struct intel_engine_cs *ring, u64 acthd) return HANGCHECK_HUNG; } +static bool engine_busy(struct intel_engine_cs *ring, + const u64 acthd, const u32 start) +{ + struct drm_i915_private *dev_priv = to_i915(ring->dev); + u32 tail, head; + + if (ring->hangcheck.acthd != acthd) + return true; + + if (ring->hangcheck.start != start) + return true; + + head = I915_READ_HEAD(ring) & HEAD_ADDR; + tail = I915_READ_TAIL(ring) & TAIL_ADDR; + + if (head != tail) + return true; + + /* Stop rings mechanism keeps head==tail even if + * there is work to be done. + */ + if (ring->buffer && + ring->buffer->tail != tail && + waitqueue_active(&ring->irq_queue)) + return true; + + return false; +} + +#define engine_idle(ring, acthd, start) (!engine_busy((ring), (acthd), (start))) + +static bool check_for_missed_irq(struct intel_engine_cs *ring) +{ + struct drm_i915_private *dev_priv = to_i915(ring->dev); + bool irq_missing; + + if (!waitqueue_active(&ring->irq_queue)) + return false; + + irq_missing = !test_and_set_bit(ring->id, + &dev_priv->gpu_error.missed_irq_rings); + + if (irq_missing) { + const bool irq_test = + dev_priv->gpu_error.test_irq_rings & + intel_ring_flag(ring); + + if (irq_test) + DRM_INFO("Fake missed irq on %s\n", ring->name); + else + DRM_ERROR("Missed irq on %s\n", ring->name); + } + + return true; +} + +static bool hangcheck_handle_stuck_ring(struct intel_engine_cs *ring, u64 acthd) +{ +#define BUSY 1 +#define KICK 5 +#define HUNG 20 + + struct intel_ring_hangcheck *hc = &ring->hangcheck; + bool there_is_hope = true; + + /* We always increment the hangcheck score + * if the ring is busy and still processing + * the same request, so that no single request + * can run indefinitely (such as a chain of + * batches). If we detect a loop in acthd, + * we increment the busyness twice as fast. + * If this ring is in a legitimate wait for another + * ring, we omit incrementing the score. In that case + * the waiting ring is a victim and we want to be sure we + * catch the right culprit. Then every time we do kick + * the ring, add a small increment to the + * score so that we can catch a batch that is + * being repeatedly kicked and so responsible + * for stalling the machine. + */ + hc->action = ring_stuck(ring, acthd); + + switch (hc->action) { + case HANGCHECK_IDLE: + case HANGCHECK_WAIT: + case HANGCHECK_ACTIVE: + hc->score += BUSY; + break; + case HANGCHECK_ACTIVE_LOOP: + hc->score += 2 * BUSY; + break; + case HANGCHECK_KICK: + hc->score += KICK; + break; + case HANGCHECK_HUNG: + hc->score += HUNG; + there_is_hope = false; + break; + } + + return there_is_hope; +} + /* * This is called when the chip hasn't reported back with completed * batchbuffers in a long time. We keep track per ring seqno progress and @@ -2900,15 +2989,14 @@ static void i915_hangcheck_elapsed(struct work_struct *work) int i; int busy_count = 0, rings_hung = 0; bool stuck[I915_NUM_RINGS] = { 0 }; -#define BUSY 1 -#define KICK 5 -#define HUNG 20 if (!i915.enable_hangcheck) return; for_each_ring(ring, dev_priv, i) { + struct intel_ring_hangcheck *hc = &ring->hangcheck; u64 acthd; + u32 start; u32 seqno; bool busy = true; @@ -2916,76 +3004,44 @@ static void i915_hangcheck_elapsed(struct work_struct *work) seqno = ring->get_seqno(ring, false); acthd = intel_ring_get_active_head(ring); + start = I915_READ_START(ring); - if (ring->hangcheck.seqno == seqno) { - if (ring_idle(ring)) { - ring->hangcheck.action = HANGCHECK_IDLE; - - if (waitqueue_active(&ring->irq_queue)) { - /* Issue a wake-up to catch stuck h/w. */ - if (!test_and_set_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings)) { - if (!(dev_priv->gpu_error.test_irq_rings & intel_ring_flag(ring))) - DRM_ERROR("Hangcheck timer elapsed... %s idle\n", - ring->name); - else - DRM_INFO("Fake missed irq on %s\n", - ring->name); - wake_up_all(&ring->irq_queue); - } - /* Safeguard against driver failure */ - ring->hangcheck.score += BUSY; - } else - busy = false; - } else { - /* We always increment the hangcheck score - * if the ring is busy and still processing - * the same request, so that no single request - * can run indefinitely (such as a chain of - * batches). The only time we do not increment - * the hangcheck score on this ring, if this - * ring is in a legitimate wait for another - * ring. In that case the waiting ring is a - * victim and we want to be sure we catch the - * right culprit. Then every time we do kick - * the ring, add a small increment to the - * score so that we can catch a batch that is - * being repeatedly kicked and so responsible - * for stalling the machine. - */ - ring->hangcheck.action = ring_stuck(ring, - acthd); - - switch (ring->hangcheck.action) { - case HANGCHECK_IDLE: - case HANGCHECK_WAIT: - case HANGCHECK_ACTIVE: - break; - case HANGCHECK_ACTIVE_LOOP: - ring->hangcheck.score += BUSY; - break; - case HANGCHECK_KICK: - ring->hangcheck.score += KICK; - break; - case HANGCHECK_HUNG: - ring->hangcheck.score += HUNG; - stuck[i] = true; - break; - } - } - } else { - ring->hangcheck.action = HANGCHECK_ACTIVE; + if (hc->seqno != seqno) { + hc->action = HANGCHECK_ACTIVE; /* Gradually reduce the count so that we catch DoS * attempts across multiple batches. */ - if (ring->hangcheck.score > 0) - ring->hangcheck.score--; + if (hc->score > 0) + hc->score--; - ring->hangcheck.acthd = ring->hangcheck.max_acthd = 0; + hc->max_acthd = 0; + goto engine_check_done; } - ring->hangcheck.seqno = seqno; - ring->hangcheck.acthd = acthd; + if (engine_idle(ring, acthd, start)) { + busy = check_for_missed_irq(ring); + if (busy) { + /* Start waking up irrespective of + our missed_irq bookkeeping */ + if (hc->score >= KICK) + wake_up_all(&ring->irq_queue); + + hc->score += KICK; + hc->action = HANGCHECK_ACTIVE; + } else { + hc->score = 0; + hc->action = HANGCHECK_IDLE; + } + goto engine_check_done; + } + + hangcheck_handle_stuck_ring(ring, acthd); + +engine_check_done: + hc->seqno = seqno; + hc->acthd = acthd; + hc->start = start; busy_count += busy; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 39f6dfc..83edcef 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -83,11 +83,14 @@ enum intel_ring_hangcheck_action { HANGCHECK_HUNG, }; +const char *i915_hangcheck_action_to_str(enum intel_ring_hangcheck_action a); + #define HANGCHECK_SCORE_RING_HUNG 31 struct intel_ring_hangcheck { u64 acthd; u64 max_acthd; + u32 start; u32 seqno; int score; enum intel_ring_hangcheck_action action;