From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5EB283E92A7 for ; Thu, 7 May 2026 11:48:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778154521; cv=none; b=pAyXsZsjJ9xOKmAXfdMJswGzFwLe+x5cGeKlzxaN3WlnjCq0Kp2lKG5KyHuj7Yp1hXgo+GhiX2fw/Su1tF4UefpRqgogjHc+YCLucfjVz5SmeW9PpQe6rn2mHPVim3g/3DaK9GFkmsNYWcfQUEc/8dLlxg+ANz2V+UC6nC5GmYQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778154521; c=relaxed/simple; bh=r/o3ZU7q6RhwCLtSEz/WFUAh8XRqFGkTd7OgeDGMtwY=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=o/5/lwkQBUYekNXALwUd0UJL7Xlrg7IehJJwHGd4Emfgnub+zNf2OpTqEIGSap6i7ojDjnigo55nC33WD7DYKHA5cCgj1ujmLZ+djunv3EA/JIqAOMnRls1LWXe8IMeXC4ZcrwGrRsQgqYA7K83JEMFrupA5MdYWJsl1DMVyWhE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jIV3ZoUX; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jIV3ZoUX" Received: by smtp.kernel.org (Postfix) with ESMTPSA id F3942C2BCB2; Thu, 7 May 2026 11:48:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778154521; bh=r/o3ZU7q6RhwCLtSEz/WFUAh8XRqFGkTd7OgeDGMtwY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=jIV3ZoUX9CcNwILvAmqBlwu6YNQdo6rBdW8zWbY80+KnrU5/QuKh1YBUYZe8mTJrN rsLkHEJL2OlAqrrz9nffKrHU2hm5JzB6vEK6BV4J/YgWtmEhVNc3j8OsmDgxRXNQFd wEmCSlRtOFfjYxDuAfQfMzg/oaSXq72qmZIrgAxXAMRxh1IBOYf37e9LsCHbBFdEjo z/pKhRVNMVrfu+7Pc9gup1WT5wVw/VGQIm0y3EG4fkXuIIPT+6KKC6aFLB0UPIlDYJ HsfItvX/TYV/K0O2pOLxQr2ZcDuXHzKrW8Ib3NY+omIUr3D1ARIi96vp6cMLYJjdNk rqVEcLK34CNYg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1wKxDa-00000000bz2-35Fh; Thu, 07 May 2026 11:48:38 +0000 Date: Thu, 07 May 2026 12:48:38 +0100 Message-ID: <86tssjxwx5.wl-maz@kernel.org> From: Marc Zyngier To: Mark Rutland Cc: linux-kernel@vger.kernel.org, Thomas Gleixner , ada.coupriediaz@arm.com, justin.he@arm.com, mhklinux@outlook.com, vladimir.murzin@arm.com Subject: Re: [PATCH] genirq/chip: Don't call add_interrupt_randomness() for NMIs In-Reply-To: <20260507110518.3128248-1-mark.rutland@arm.com> References: <20260507110518.3128248-1-mark.rutland@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: mark.rutland@arm.com, linux-kernel@vger.kernel.org, tglx@kernel.org, ada.coupriediaz@arm.com, justin.he@arm.com, mhklinux@outlook.com, vladimir.murzin@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Thu, 07 May 2026 12:05:18 +0100, Mark Rutland wrote: >=20 > Recently handle_percpu_devid_irq() was changed to call > add_interrupt_randomness(). This introduced a potential deadlock when > handle_percpu_devid_irq() is used to handle an NMI, which can be > detected with lockdep, e.g. >=20 > =C2=A0 =C2=A0 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =C2=A0 =C2=A0 WARNING: inconsistent lock state > =C2=A0 =C2=A0 7.1.0-rc2-pnmi #465 Not tainted > =C2=A0 =C2=A0 -------------------------------- > =C2=A0 =C2=A0 inconsistent {INITIAL USE} -> {IN-NMI} usage. > =C2=A0 =C2=A0 perf/695 [HC1[1]:SC0[0]:HE0:SE1] takes: > =C2=A0 =C2=A0 ffff00837dfd3a18 (&base->lock){-.-.}-{2:2}, at: lock_timer_= base+0x6c/0xac > =C2=A0 =C2=A0 {INITIAL USE} state was registered at: > =C2=A0 =C2=A0 =C2=A0 lock_acquire+0x260/0x40c > =C2=A0 =C2=A0 =C2=A0 _raw_spin_lock_irqsave+0x68/0xb0 > =C2=A0 =C2=A0 =C2=A0 lock_timer_base+0x6c/0xac > =C2=A0 =C2=A0 =C2=A0 __mod_timer+0x100/0x32c > =C2=A0 =C2=A0 =C2=A0 add_timer_global+0x2c/0x40 > =C2=A0 =C2=A0 =C2=A0 __queue_delayed_work+0xf0/0x140 > =C2=A0 =C2=A0 =C2=A0 queue_delayed_work_on+0x134/0x138 > =C2=A0 =C2=A0 =C2=A0 mem_cgroup_css_online+0x30c/0x310 > =C2=A0 =C2=A0 =C2=A0 online_css+0x34/0x10c > =C2=A0 =C2=A0 =C2=A0 cgroup_init_subsys+0x158/0x1c8 > =C2=A0 =C2=A0 =C2=A0 cgroup_init+0x440/0x524 > =C2=A0 =C2=A0 =C2=A0 start_kernel+0x888/0x998 > =C2=A0 =C2=A0 =C2=A0 __primary_switched+0x88/0x90 > =C2=A0 =C2=A0 irq event stamp: 62068 > =C2=A0 =C2=A0 hardirqs last=C2=A0 enabled at (62067): [= ] seqcount_lockdep_reader_access.constprop.0+0xf0/0xfc > =C2=A0 =C2=A0 hardirqs last disabled at (62068): [] _ra= w_spin_lock_irqsave+0x94/0xb0 > =C2=A0 =C2=A0 softirqs last=C2=A0 enabled at (62050): [= ] put_cpu_fpsimd_context+0x1c/0x4c > =C2=A0 =C2=A0 softirqs last disabled at (62048): [] get= _cpu_fpsimd_context+0x1c/0x4c > =C2=A0 =C2=A0 =C2=A0 =C2=A0 other info that might help us debug this: > =C2=A0 =C2=A0 Possible unsafe locking scenario: > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CPU0 > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0---- > =C2=A0 =C2=A0 =C2=A0 lock(&base->lock); > =C2=A0 =C2=A0 =C2=A0 > =C2=A0 =C2=A0 =C2=A0 =C2=A0 lock(&base->lock); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 *** DEADLOCK *** > =C2=A0 =C2=A0 3 locks held by perf/695: > =C2=A0 =C2=A0 =C2=A0#0: ffff0080146cd2c8 (&type->i_mutex_dir_key#6){++++}= -{4:4}, at: lookup_slow+0x30/0x68 > =C2=A0 =C2=A0 =C2=A0#1: ffff80008332b858 (rcu_read_lock){....}-{1:3}, at:= blk_mq_run_hw_queue+0xf4/0x1fc > =C2=A0 =C2=A0 =C2=A0#2: ffff008000b6aa18 (&host->lock){-.-.}-{3:3}, at: a= ta_scsi_queuecmd+0x28/0x88 > =C2=A0 =C2=A0 stack backtrace: > =C2=A0 =C2=A0 CPU: 3 UID: 0 PID: 695 Comm: perf Not tainted 7.1.0-rc2-pnm= i #465 PREEMPT > =C2=A0 =C2=A0 Hardware name: ARM LTD Morello System Development Platform,= BIOS EDK II Mar=C2=A0 7 2024 > =C2=A0 =C2=A0 Call trace: > =C2=A0 =C2=A0 =C2=A0show_stack+0x18/0x24 (C) > =C2=A0 =C2=A0 =C2=A0dump_stack_lvl+0xc4/0x148 > =C2=A0 =C2=A0 =C2=A0dump_stack+0x18/0x24 > =C2=A0 =C2=A0 =C2=A0print_usage_bug.part.0+0x29c/0x364 > =C2=A0 =C2=A0 =C2=A0lock_acquire+0x364/0x40c > =C2=A0 =C2=A0 =C2=A0_raw_spin_lock_irqsave+0x68/0xb0 > =C2=A0 =C2=A0 =C2=A0lock_timer_base+0x6c/0xac > =C2=A0 =C2=A0 =C2=A0add_timer_on+0x78/0x16c > =C2=A0 =C2=A0 =C2=A0add_interrupt_randomness+0x124/0x134 > =C2=A0 =C2=A0 =C2=A0handle_percpu_devid_irq+0xd4/0x16c > =C2=A0 =C2=A0 =C2=A0handle_irq_desc+0x40/0x58 > =C2=A0 =C2=A0 =C2=A0generic_handle_domain_nmi+0x28/0x50 > =C2=A0 =C2=A0 =C2=A0__gic_handle_nmi.isra.0+0x4c/0xa0 > =C2=A0 =C2=A0 =C2=A0gic_handle_irq+0x38/0x2bc > =C2=A0 =C2=A0 =C2=A0call_on_irq_stack+0x30/0x48 > =C2=A0 =C2=A0 =C2=A0do_interrupt_handler+0x80/0x98 > =C2=A0 =C2=A0 =C2=A0el1_interrupt+0x90/0xac > =C2=A0 =C2=A0 =C2=A0el1h_64_irq_handler+0x18/0x24 > =C2=A0 =C2=A0 =C2=A0el1h_64_irq+0x80/0x84 > [...] >=20 > During review, Thomas pointed out it wouldn't be safe for > handle_percpu_devid_irq() to call add_interrupt_randomness() if it was > used to handle NMIs: >=20 > https://lore.kernel.org/lkml/87bjgik042.ffs@tglx/ >=20 > ... but evidently people missed that handle_percpu_devid_irq() *is* used > for NMIs. >=20 > While it might seem that we should handle NMIs with a separate > handle_percpu_devid_nmi() function, for various structural reasons this > was impractical, and handle_percpu_devid_irq() has been expected to be > used for NMIs since commits: >=20 > 21bbbc50f398f ("irqchip/gic-v3: Switch high priority PPIs over to handl= e_percpu_devid_irq()") > 5ff78c8de9d83 ("genirq: Kill handle_percpu_devid_fasteoi_nmi()") >=20 > Taking the above into account, avoid the deadlock by not calling > add_interrupt_randomness() when handle_percpu_devid_irq() is called in > an NMI context. This is consistent with our other NNI handling flows, > which do not call add_interrupt_randomness(). >=20 > At the same time, update the kerneldoc comment to make it clear that > handle_percpu_devid_irq() can be called in NMI context. The rest of > handle_percpu_devid_irq() is currently NMI safe and doesn't need to > change. >=20 > Fixes: fd7400cfcbaa ("genirq/chip: Invoke add_interrupt_randomness() in h= andle_percpu_devid_irq()") > Reported-by: Ada Couprie Diaz > Signed-off-by: Mark Rutland > Cc: Justin He > Cc: Marc Zyngier > Cc: Michael Kelley > Cc: Thomas Gleixner > Cc: Vladimir Murzin Thanks for catching that one. FWIW: Reviewed-by: Marc Zyngier M. --=20 Without deviation from the norm, progress is not possible.