From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9871BC433F5 for ; Tue, 14 Sep 2021 05:15:20 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6367D60F21 for ; Tue, 14 Sep 2021 05:15:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6367D60F21 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 17A9B6E405; Tue, 14 Sep 2021 05:15:05 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 624276E3F7; Tue, 14 Sep 2021 05:15:01 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10106"; a="208976891" X-IronPort-AV: E=Sophos;i="5.85,291,1624345200"; d="scan'208";a="208976891" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Sep 2021 22:15:00 -0700 X-IronPort-AV: E=Sophos;i="5.85,291,1624345200"; d="scan'208";a="697404488" Received: from jons-linux-dev-box.fm.intel.com ([10.1.27.20]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Sep 2021 22:14:59 -0700 From: Matthew Brost To: , Cc: , Subject: [PATCH 4/4] drm/i915/guc: Refcount context during error capture Date: Mon, 13 Sep 2021 22:09:56 -0700 Message-Id: <20210914050956.30685-5-matthew.brost@intel.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210914050956.30685-1-matthew.brost@intel.com> References: <20210914050956.30685-1-matthew.brost@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: John Harrison When i915 receives a context reset notification from GuC, it triggers an error capture before resetting any outstanding requsts of that context. Unfortunately, the error capture is not a time bound operation. In certain situations it can take a long time, particularly when multiple large LMEM buffers must be read back and eoncoded. If this delay is longer than other timeouts (heartbeat, test recovery, etc.) then a full GT reset can be triggered in the middle. That can result in the context being reset by GuC actually being destroyed before the error capture completes and the GuC submission code resumes. Thus, the GuC side can start dereferencing stale pointers and Bad Things ensue. So add a refcount get of the context during the entire reset operation. That way, the context can't be destroyed part way through no matter what other resets or user interactions occur. v2: (Matthew Brost) - Update patch to work with async error capture Signed-off-by: John Harrison Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 1986a57b52cc..02917fc4d4a8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -2888,6 +2888,8 @@ static void capture_worker_func(struct work_struct *w) intel_engine_set_hung_context(engine, ce); with_intel_runtime_pm(&i915->runtime_pm, wakeref) i915_capture_error_state(gt, ce->engine->mask); + + intel_context_put(ce); } static void capture_error_state(struct intel_guc *guc, @@ -2924,7 +2926,7 @@ static void guc_context_replay(struct intel_context *ce) tasklet_hi_schedule(&sched_engine->tasklet); } -static void guc_handle_context_reset(struct intel_guc *guc, +static bool guc_handle_context_reset(struct intel_guc *guc, struct intel_context *ce) { trace_intel_context_reset(ce); @@ -2937,7 +2939,11 @@ static void guc_handle_context_reset(struct intel_guc *guc, !context_blocked(ce))) { capture_error_state(guc, ce); guc_context_replay(ce); + + return false; } + + return true; } int intel_guc_context_reset_process_msg(struct intel_guc *guc, @@ -2945,6 +2951,7 @@ int intel_guc_context_reset_process_msg(struct intel_guc *guc, { struct intel_context *ce; int desc_idx; + unsigned long flags; if (unlikely(len != 1)) { drm_err(&guc_to_gt(guc)->i915->drm, "Invalid length %u", len); @@ -2952,11 +2959,24 @@ int intel_guc_context_reset_process_msg(struct intel_guc *guc, } desc_idx = msg[0]; + + /* + * The context lookup uses the xarray but lookups only require an RCU lock + * not the full spinlock. So take the lock explicitly and keep it until the + * context has been reference count locked to ensure it can't be destroyed + * asynchronously until the reset is done. + */ + xa_lock_irqsave(&guc->context_lookup, flags); ce = g2h_context_lookup(guc, desc_idx); + if (ce) + intel_context_get(ce); + xa_unlock_irqrestore(&guc->context_lookup, flags); + if (unlikely(!ce)) return -EPROTO; - guc_handle_context_reset(guc, ce); + if (guc_handle_context_reset(guc, ce)) + intel_context_put(ce); return 0; } -- 2.32.0 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5738C433EF for ; Tue, 14 Sep 2021 05:15:13 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 92F0060F70 for ; Tue, 14 Sep 2021 05:15:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 92F0060F70 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 529CA6E402; Tue, 14 Sep 2021 05:15:03 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id 624276E3F7; Tue, 14 Sep 2021 05:15:01 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10106"; a="208976891" X-IronPort-AV: E=Sophos;i="5.85,291,1624345200"; d="scan'208";a="208976891" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Sep 2021 22:15:00 -0700 X-IronPort-AV: E=Sophos;i="5.85,291,1624345200"; d="scan'208";a="697404488" Received: from jons-linux-dev-box.fm.intel.com ([10.1.27.20]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Sep 2021 22:14:59 -0700 From: Matthew Brost To: , Cc: , Date: Mon, 13 Sep 2021 22:09:56 -0700 Message-Id: <20210914050956.30685-5-matthew.brost@intel.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210914050956.30685-1-matthew.brost@intel.com> References: <20210914050956.30685-1-matthew.brost@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Intel-gfx] [PATCH 4/4] drm/i915/guc: Refcount context during error capture X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 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" From: John Harrison When i915 receives a context reset notification from GuC, it triggers an error capture before resetting any outstanding requsts of that context. Unfortunately, the error capture is not a time bound operation. In certain situations it can take a long time, particularly when multiple large LMEM buffers must be read back and eoncoded. If this delay is longer than other timeouts (heartbeat, test recovery, etc.) then a full GT reset can be triggered in the middle. That can result in the context being reset by GuC actually being destroyed before the error capture completes and the GuC submission code resumes. Thus, the GuC side can start dereferencing stale pointers and Bad Things ensue. So add a refcount get of the context during the entire reset operation. That way, the context can't be destroyed part way through no matter what other resets or user interactions occur. v2: (Matthew Brost) - Update patch to work with async error capture Signed-off-by: John Harrison Signed-off-by: Matthew Brost --- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 1986a57b52cc..02917fc4d4a8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -2888,6 +2888,8 @@ static void capture_worker_func(struct work_struct *w) intel_engine_set_hung_context(engine, ce); with_intel_runtime_pm(&i915->runtime_pm, wakeref) i915_capture_error_state(gt, ce->engine->mask); + + intel_context_put(ce); } static void capture_error_state(struct intel_guc *guc, @@ -2924,7 +2926,7 @@ static void guc_context_replay(struct intel_context *ce) tasklet_hi_schedule(&sched_engine->tasklet); } -static void guc_handle_context_reset(struct intel_guc *guc, +static bool guc_handle_context_reset(struct intel_guc *guc, struct intel_context *ce) { trace_intel_context_reset(ce); @@ -2937,7 +2939,11 @@ static void guc_handle_context_reset(struct intel_guc *guc, !context_blocked(ce))) { capture_error_state(guc, ce); guc_context_replay(ce); + + return false; } + + return true; } int intel_guc_context_reset_process_msg(struct intel_guc *guc, @@ -2945,6 +2951,7 @@ int intel_guc_context_reset_process_msg(struct intel_guc *guc, { struct intel_context *ce; int desc_idx; + unsigned long flags; if (unlikely(len != 1)) { drm_err(&guc_to_gt(guc)->i915->drm, "Invalid length %u", len); @@ -2952,11 +2959,24 @@ int intel_guc_context_reset_process_msg(struct intel_guc *guc, } desc_idx = msg[0]; + + /* + * The context lookup uses the xarray but lookups only require an RCU lock + * not the full spinlock. So take the lock explicitly and keep it until the + * context has been reference count locked to ensure it can't be destroyed + * asynchronously until the reset is done. + */ + xa_lock_irqsave(&guc->context_lookup, flags); ce = g2h_context_lookup(guc, desc_idx); + if (ce) + intel_context_get(ce); + xa_unlock_irqrestore(&guc->context_lookup, flags); + if (unlikely(!ce)) return -EPROTO; - guc_handle_context_reset(guc, ce); + if (guc_handle_context_reset(guc, ce)) + intel_context_put(ce); return 0; } -- 2.32.0