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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 40E3FC61DF4 for ; Fri, 24 Nov 2023 07:19:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0DCA810E5E7; Fri, 24 Nov 2023 07:19:23 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1348710E773 for ; Fri, 24 Nov 2023 07:19:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1700810359; x=1732346359; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=R+XPqEBuTpKrMwsHW2xKpl/JihfJJnLEEXoUmcagHtw=; b=GOF3w70uebxFiHJ+SQcEZb2t64ZpHWhwBsTWPSkUJxGnYm7//PdXGLtq D3q7poIAWhtKmi0tas/DVSfaoDlXakvd7ZlKb+9B02hMgPszw2IIVZWc8 BN0ZTzVz1lV1ljQs5PEHgkngR/twJlkzDxqhwQJJJcOiO7orAxq7S2VdU cz3Ee4KuCtnKrenxQehukPP7w6ksY/2zOHTrFTa478jfr3dTuTJu8xMie dU0eB9YsaoHxHwu9tIN1mxZdh9FPDaetDvUSE6CKAbSKUgJD7xZ37763m 494km1D8DTtQ9YTamQ4F53XxdPKtfM5HSuNV45kCd8+m2g4QIf4UJqPvr Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10902"; a="458868376" X-IronPort-AV: E=Sophos;i="6.04,223,1695711600"; d="scan'208";a="458868376" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2023 23:19:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10902"; a="760883959" X-IronPort-AV: E=Sophos;i="6.04,223,1695711600"; d="scan'208";a="760883959" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.74]) by orsmga007.jf.intel.com with SMTP; 23 Nov 2023 23:19:16 -0800 Received: by stinkbox (sSMTP sendmail emulation); Fri, 24 Nov 2023 09:19:15 +0200 Date: Fri, 24 Nov 2023 09:19:15 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Aravind Iddamsetty Message-ID: References: <20231124064408.1800646-1-aravind.iddamsetty@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20231124064408.1800646-1-aravind.iddamsetty@linux.intel.com> X-Patchwork-Hint: comment Subject: Re: [Intel-xe] [PATCH] drm/xe: Fix lockdep warning in xe_force_wake calls X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-xe@lists.freedesktop.org Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Fri, Nov 24, 2023 at 12:14:08PM +0530, Aravind Iddamsetty wrote: > Introduce atomic version for xe_force_wake calls which uses spin_lock > while the non atomic version uses spin_lock_irq > > Fix for below: > [13994.811263] ======================================================== > [13994.811295] WARNING: possible irq lock inversion dependency detected > [13994.811326] 6.6.0-rc3-xe #2 Tainted: G U > [13994.811358] -------------------------------------------------------- > [13994.811388] swapper/0/0 just changed the state of lock: > [13994.811416] ffff895c7e044db8 (&cpuctx_lock){-...}-{2:2}, at: > __perf_event_read+0xb7/0x3a0 > [13994.811494] but this lock took another, HARDIRQ-unsafe lock in the > past: > [13994.811528] (&fw->lock){+.+.}-{2:2} > [13994.811544] > > and interrupts could create inverse lock ordering between > them. > > [13994.811606] > other info that might help us debug this: > [13994.811636] Possible interrupt unsafe locking scenario: > > [13994.811667] CPU0 CPU1 > [13994.811691] ---- ---- > [13994.811715] lock(&fw->lock); > [13994.811744] local_irq_disable(); > [13994.811773] lock(&cpuctx_lock); > [13994.811810] lock(&fw->lock); > [13994.811846] > [13994.811865] lock(&cpuctx_lock); > [13994.811895] > *** DEADLOCK *** > > v2: Use spin_lock in atomic context and spin_lock_irq in a non atomic > context (Matthew Brost) No idea what this "atomic context" means, but looks like you just want to use spin_lock_irqsave() & co. > > Cc: Matthew Brost > Cc: Anshuman Gupta > Cc: Umesh Nerlige Ramappa > Signed-off-by: Aravind Iddamsetty > --- > drivers/gpu/drm/xe/xe_force_wake.c | 62 +++++++++++++++++++++++++++++- > drivers/gpu/drm/xe/xe_force_wake.h | 4 ++ > drivers/gpu/drm/xe/xe_pmu.c | 4 +- > 3 files changed, 66 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c > index 32d6c4dd2807..1693097f72d3 100644 > --- a/drivers/gpu/drm/xe/xe_force_wake.c > +++ b/drivers/gpu/drm/xe/xe_force_wake.c > @@ -147,7 +147,7 @@ int xe_force_wake_get(struct xe_force_wake *fw, > enum xe_force_wake_domains tmp, woken = 0; > int ret, ret2 = 0; > > - spin_lock(&fw->lock); > + spin_lock_irq(&fw->lock); > for_each_fw_domain_masked(domain, domains, fw, tmp) { > if (!domain->ref++) { > woken |= BIT(domain->id); > @@ -162,7 +162,7 @@ int xe_force_wake_get(struct xe_force_wake *fw, > domain->id, ret); > } > fw->awake_domains |= woken; > - spin_unlock(&fw->lock); > + spin_unlock_irq(&fw->lock); > > return ret2; > } > @@ -176,6 +176,64 @@ int xe_force_wake_put(struct xe_force_wake *fw, > enum xe_force_wake_domains tmp, sleep = 0; > int ret, ret2 = 0; > > + spin_lock_irq(&fw->lock); > + for_each_fw_domain_masked(domain, domains, fw, tmp) { > + if (!--domain->ref) { > + sleep |= BIT(domain->id); > + domain_sleep(gt, domain); > + } > + } > + for_each_fw_domain_masked(domain, sleep, fw, tmp) { > + ret = domain_sleep_wait(gt, domain); Why on earth are we waiting here? Why is this all this stuff called "sleep something"? -- Ville Syrjälä Intel