* [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
@ 2025-02-18 2:24 Lu Baolu
2025-02-18 10:14 ` Ido Schimmel
2025-02-20 7:21 ` Tian, Kevin
0 siblings, 2 replies; 7+ messages in thread
From: Lu Baolu @ 2025-02-18 2:24 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian
Cc: Ido Schimmel, iommu, linux-kernel, Lu Baolu, stable
Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
locally") moved the call to enable_drhd_fault_handling() to a code
path that does not hold any lock while traversing the drhd list. Fix
it by ensuring the dmar_global_lock lock is held when traversing the
drhd list.
Without this fix, the following warning is triggered:
=============================
WARNING: suspicious RCU usage
6.14.0-rc3 #55 Not tainted
-----------------------------
drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 1
2 locks held by cpuhp/1/23:
#0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x87/0x2c0
#1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x87/0x2c0
stack backtrace:
CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
Call Trace:
<TASK>
dump_stack_lvl+0xb7/0xd0
lockdep_rcu_suspicious+0x159/0x1f0
? __pfx_enable_drhd_fault_handling+0x10/0x10
enable_drhd_fault_handling+0x151/0x180
cpuhp_invoke_callback+0x1df/0x990
cpuhp_thread_fun+0x1ea/0x2c0
smpboot_thread_fn+0x1f5/0x2e0
? __pfx_smpboot_thread_fn+0x10/0x10
kthread+0x12a/0x2d0
? __pfx_kthread+0x10/0x10
ret_from_fork+0x4a/0x60
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Simply holding the lock in enable_drhd_fault_handling() will trigger a
lock order splat. Avoid holding the dmar_global_lock when calling
iommu_device_register(), which starts the device probe process.
Fixes: d74169ceb0d2 ("iommu/vt-d: Allocate DMAR fault interrupts locally")
Reported-by: Ido Schimmel <idosch@idosch.org>
Closes: https://lore.kernel.org/linux-iommu/Zx9OwdLIc_VoQ0-a@shredder.mtl.com/
Cc: stable@vger.kernel.org
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/dmar.c | 1 +
drivers/iommu/intel/iommu.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 9f424acf474e..e540092d664d 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -2043,6 +2043,7 @@ int enable_drhd_fault_handling(unsigned int cpu)
/*
* Enable fault control interrupt.
*/
+ guard(rwsem_read)(&dmar_global_lock);
for_each_iommu(iommu, drhd) {
u32 fault_status;
int ret;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index cc46098f875b..9a1e61b429ca 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3146,7 +3146,14 @@ int __init intel_iommu_init(void)
iommu_device_sysfs_add(&iommu->iommu, NULL,
intel_iommu_groups,
"%s", iommu->name);
+ /*
+ * The iommu device probe is protected by the iommu_probe_device_lock.
+ * Release the dmar_global_lock before entering the device probe path
+ * to avoid unnecessary lock order splat.
+ */
+ up_read(&dmar_global_lock);
iommu_device_register(&iommu->iommu, &intel_iommu_ops, NULL);
+ down_read(&dmar_global_lock);
iommu_pmu_register(iommu);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
2025-02-18 2:24 [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage Lu Baolu
@ 2025-02-18 10:14 ` Ido Schimmel
2025-02-18 14:09 ` Breno Leitao
2025-02-20 7:21 ` Tian, Kevin
1 sibling, 1 reply; 7+ messages in thread
From: Ido Schimmel @ 2025-02-18 10:14 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, iommu,
linux-kernel, stable, leitao
+ Breno who also encountered this issue
On Tue, Feb 18, 2025 at 10:24:21AM +0800, Lu Baolu wrote:
> Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
> locally") moved the call to enable_drhd_fault_handling() to a code
> path that does not hold any lock while traversing the drhd list. Fix
> it by ensuring the dmar_global_lock lock is held when traversing the
> drhd list.
>
> Without this fix, the following warning is triggered:
> =============================
> WARNING: suspicious RCU usage
> 6.14.0-rc3 #55 Not tainted
> -----------------------------
> drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
> other info that might help us debug this:
> rcu_scheduler_active = 1, debug_locks = 1
> 2 locks held by cpuhp/1/23:
> #0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x87/0x2c0
> #1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x87/0x2c0
> stack backtrace:
> CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
> Call Trace:
> <TASK>
> dump_stack_lvl+0xb7/0xd0
> lockdep_rcu_suspicious+0x159/0x1f0
> ? __pfx_enable_drhd_fault_handling+0x10/0x10
> enable_drhd_fault_handling+0x151/0x180
> cpuhp_invoke_callback+0x1df/0x990
> cpuhp_thread_fun+0x1ea/0x2c0
> smpboot_thread_fn+0x1f5/0x2e0
> ? __pfx_smpboot_thread_fn+0x10/0x10
> kthread+0x12a/0x2d0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x4a/0x60
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Simply holding the lock in enable_drhd_fault_handling() will trigger a
> lock order splat. Avoid holding the dmar_global_lock when calling
> iommu_device_register(), which starts the device probe process.
>
> Fixes: d74169ceb0d2 ("iommu/vt-d: Allocate DMAR fault interrupts locally")
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Closes: https://lore.kernel.org/linux-iommu/Zx9OwdLIc_VoQ0-a@shredder.mtl.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Thanks for the fix. I tested it and the warning is gone.
Tested-by: Ido Schimmel <idosch@nvidia.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
2025-02-18 10:14 ` Ido Schimmel
@ 2025-02-18 14:09 ` Breno Leitao
0 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2025-02-18 14:09 UTC (permalink / raw)
To: Ido Schimmel
Cc: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian,
iommu, linux-kernel, stable
On Tue, Feb 18, 2025 at 12:14:53PM +0200, Ido Schimmel wrote:
> + Breno who also encountered this issue
Thanks. Please add the following if you feel appropriate.
Reported-by: Breno Leitao <leitao@debian.org>
> On Tue, Feb 18, 2025 at 10:24:21AM +0800, Lu Baolu wrote:
> > Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
> > locally") moved the call to enable_drhd_fault_handling() to a code
> > path that does not hold any lock while traversing the drhd list. Fix
> > it by ensuring the dmar_global_lock lock is held when traversing the
> > drhd list.
> >
> > Without this fix, the following warning is triggered:
> > =============================
> > WARNING: suspicious RCU usage
> > 6.14.0-rc3 #55 Not tainted
> > -----------------------------
> > drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
> > other info that might help us debug this:
> > rcu_scheduler_active = 1, debug_locks = 1
> > 2 locks held by cpuhp/1/23:
> > #0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x87/0x2c0
> > #1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x87/0x2c0
> > stack backtrace:
> > CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
> > Call Trace:
> > <TASK>
> > dump_stack_lvl+0xb7/0xd0
> > lockdep_rcu_suspicious+0x159/0x1f0
> > ? __pfx_enable_drhd_fault_handling+0x10/0x10
> > enable_drhd_fault_handling+0x151/0x180
> > cpuhp_invoke_callback+0x1df/0x990
> > cpuhp_thread_fun+0x1ea/0x2c0
> > smpboot_thread_fn+0x1f5/0x2e0
> > ? __pfx_smpboot_thread_fn+0x10/0x10
> > kthread+0x12a/0x2d0
> > ? __pfx_kthread+0x10/0x10
> > ret_from_fork+0x4a/0x60
> > ? __pfx_kthread+0x10/0x10
> > ret_from_fork_asm+0x1a/0x30
> > </TASK>
> >
> > Simply holding the lock in enable_drhd_fault_handling() will trigger a
> > lock order splat. Avoid holding the dmar_global_lock when calling
> > iommu_device_register(), which starts the device probe process.
> >
> > Fixes: d74169ceb0d2 ("iommu/vt-d: Allocate DMAR fault interrupts locally")
> > Reported-by: Ido Schimmel <idosch@idosch.org>
> > Closes: https://lore.kernel.org/linux-iommu/Zx9OwdLIc_VoQ0-a@shredder.mtl.com/
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>
> Thanks for the fix. I tested it and the warning is gone.
>
> Tested-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Breno Leitao <leitao@debian.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
2025-02-18 2:24 [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage Lu Baolu
2025-02-18 10:14 ` Ido Schimmel
@ 2025-02-20 7:21 ` Tian, Kevin
2025-02-20 11:38 ` Baolu Lu
1 sibling, 1 reply; 7+ messages in thread
From: Tian, Kevin @ 2025-02-20 7:21 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy
Cc: Ido Schimmel, iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, February 18, 2025 10:24 AM
>
> Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
> locally") moved the call to enable_drhd_fault_handling() to a code
> path that does not hold any lock while traversing the drhd list. Fix
> it by ensuring the dmar_global_lock lock is held when traversing the
> drhd list.
>
> Without this fix, the following warning is triggered:
> =============================
> WARNING: suspicious RCU usage
> 6.14.0-rc3 #55 Not tainted
> -----------------------------
> drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
> other info that might help us debug this:
> rcu_scheduler_active = 1, debug_locks = 1
> 2 locks held by cpuhp/1/23:
> #0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at:
> cpuhp_thread_fun+0x87/0x2c0
> #1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at:
> cpuhp_thread_fun+0x87/0x2c0
> stack backtrace:
> CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
> Call Trace:
> <TASK>
> dump_stack_lvl+0xb7/0xd0
> lockdep_rcu_suspicious+0x159/0x1f0
> ? __pfx_enable_drhd_fault_handling+0x10/0x10
> enable_drhd_fault_handling+0x151/0x180
> cpuhp_invoke_callback+0x1df/0x990
> cpuhp_thread_fun+0x1ea/0x2c0
> smpboot_thread_fn+0x1f5/0x2e0
> ? __pfx_smpboot_thread_fn+0x10/0x10
> kthread+0x12a/0x2d0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x4a/0x60
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Simply holding the lock in enable_drhd_fault_handling() will trigger a
> lock order splat. Avoid holding the dmar_global_lock when calling
> iommu_device_register(), which starts the device probe process.
Can you elaborate the splat issue? It's not intuitive to me with a quick
read of the code and iommu_device_register() is not occurred in above
calling stack.
>
> Fixes: d74169ceb0d2 ("iommu/vt-d: Allocate DMAR fault interrupts locally")
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Closes: https://lore.kernel.org/linux-iommu/Zx9OwdLIc_VoQ0-
> a@shredder.mtl.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/dmar.c | 1 +
> drivers/iommu/intel/iommu.c | 7 +++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 9f424acf474e..e540092d664d 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -2043,6 +2043,7 @@ int enable_drhd_fault_handling(unsigned int cpu)
> /*
> * Enable fault control interrupt.
> */
> + guard(rwsem_read)(&dmar_global_lock);
> for_each_iommu(iommu, drhd) {
> u32 fault_status;
> int ret;
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index cc46098f875b..9a1e61b429ca 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -3146,7 +3146,14 @@ int __init intel_iommu_init(void)
> iommu_device_sysfs_add(&iommu->iommu, NULL,
> intel_iommu_groups,
> "%s", iommu->name);
> + /*
> + * The iommu device probe is protected by the
> iommu_probe_device_lock.
> + * Release the dmar_global_lock before entering the device
> probe path
> + * to avoid unnecessary lock order splat.
> + */
> + up_read(&dmar_global_lock);
> iommu_device_register(&iommu->iommu,
> &intel_iommu_ops, NULL);
> + down_read(&dmar_global_lock);
>
> iommu_pmu_register(iommu);
> }
> --
> 2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
2025-02-20 7:21 ` Tian, Kevin
@ 2025-02-20 11:38 ` Baolu Lu
2025-02-21 7:22 ` Tian, Kevin
0 siblings, 1 reply; 7+ messages in thread
From: Baolu Lu @ 2025-02-20 11:38 UTC (permalink / raw)
To: Tian, Kevin, Joerg Roedel, Will Deacon, Robin Murphy
Cc: baolu.lu, Ido Schimmel, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
On 2025/2/20 15:21, Tian, Kevin wrote:
>> From: Lu Baolu<baolu.lu@linux.intel.com>
>> Sent: Tuesday, February 18, 2025 10:24 AM
>>
>> Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
>> locally") moved the call to enable_drhd_fault_handling() to a code
>> path that does not hold any lock while traversing the drhd list. Fix
>> it by ensuring the dmar_global_lock lock is held when traversing the
>> drhd list.
>>
>> Without this fix, the following warning is triggered:
>> =============================
>> WARNING: suspicious RCU usage
>> 6.14.0-rc3 #55 Not tainted
>> -----------------------------
>> drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
>> other info that might help us debug this:
>> rcu_scheduler_active = 1, debug_locks = 1
>> 2 locks held by cpuhp/1/23:
>> #0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at:
>> cpuhp_thread_fun+0x87/0x2c0
>> #1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at:
>> cpuhp_thread_fun+0x87/0x2c0
>> stack backtrace:
>> CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
>> Call Trace:
>> <TASK>
>> dump_stack_lvl+0xb7/0xd0
>> lockdep_rcu_suspicious+0x159/0x1f0
>> ? __pfx_enable_drhd_fault_handling+0x10/0x10
>> enable_drhd_fault_handling+0x151/0x180
>> cpuhp_invoke_callback+0x1df/0x990
>> cpuhp_thread_fun+0x1ea/0x2c0
>> smpboot_thread_fn+0x1f5/0x2e0
>> ? __pfx_smpboot_thread_fn+0x10/0x10
>> kthread+0x12a/0x2d0
>> ? __pfx_kthread+0x10/0x10
>> ret_from_fork+0x4a/0x60
>> ? __pfx_kthread+0x10/0x10
>> ret_from_fork_asm+0x1a/0x30
>> </TASK>
>>
>> Simply holding the lock in enable_drhd_fault_handling() will trigger a
>> lock order splat. Avoid holding the dmar_global_lock when calling
>> iommu_device_register(), which starts the device probe process.
> Can you elaborate the splat issue? It's not intuitive to me with a quick
> read of the code and iommu_device_register() is not occurred in above
> calling stack.
The lockdep splat looks like below:
======================================================
WARNING: possible circular locking dependency detected
6.14.0-rc3-00002-g8e4617b46db1 #57 Not tainted
------------------------------------------------------
swapper/0/1 is trying to acquire lock:
ffffffffa2a67c50 (cpu_hotplug_lock){++++}-{0:0}, at:
iova_domain_init_rcaches.part.0+0x1d3/0x210
but task is already holding lock:
ffff9f4a87b171c8 (&domain->iova_cookie->mutex){+.+.}-{4:4}, at:
iommu_dma_init_domain+0x122/0x2e0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #4 (&domain->iova_cookie->mutex){+.+.}-{4:4}:
__lock_acquire+0x4a0/0xb50
lock_acquire+0xd1/0x2e0
__mutex_lock+0xa5/0xce0
iommu_dma_init_domain+0x122/0x2e0
iommu_setup_dma_ops+0x65/0xe0
bus_iommu_probe+0x100/0x1d0
iommu_device_register+0xd6/0x130
intel_iommu_init+0x527/0x870
pci_iommu_init+0x17/0x60
do_one_initcall+0x7c/0x390
do_initcalls+0xe8/0x1e0
kernel_init_freeable+0x313/0x490
kernel_init+0x24/0x240
ret_from_fork+0x4a/0x60
ret_from_fork_asm+0x1a/0x30
-> #3 (&group->mutex){+.+.}-{4:4}:
__lock_acquire+0x4a0/0xb50
lock_acquire+0xd1/0x2e0
__mutex_lock+0xa5/0xce0
bus_iommu_probe+0x95/0x1d0
iommu_device_register+0xd6/0x130
intel_iommu_init+0x527/0x870
pci_iommu_init+0x17/0x60
do_one_initcall+0x7c/0x390
do_initcalls+0xe8/0x1e0
kernel_init_freeable+0x313/0x490
kernel_init+0x24/0x240
ret_from_fork+0x4a/0x60
ret_from_fork_asm+0x1a/0x30
-> #2 (dmar_global_lock){++++}-{4:4}:
__lock_acquire+0x4a0/0xb50
lock_acquire+0xd1/0x2e0
down_read+0x31/0x170
enable_drhd_fault_handling+0x27/0x1a0
cpuhp_invoke_callback+0x1e2/0x990
cpuhp_issue_call+0xac/0x2c0
__cpuhp_setup_state_cpuslocked+0x229/0x430
__cpuhp_setup_state+0xc3/0x260
irq_remap_enable_fault_handling+0x52/0x80
apic_intr_mode_init+0x59/0xf0
x86_late_time_init+0x29/0x50
start_kernel+0x642/0x7f0
x86_64_start_reservations+0x18/0x30
x86_64_start_kernel+0x91/0xa0
common_startup_64+0x13e/0x148
-> #1 (cpuhp_state_mutex){+.+.}-{4:4}:
__lock_acquire+0x4a0/0xb50
lock_acquire+0xd1/0x2e0
__mutex_lock+0xa5/0xce0
__cpuhp_setup_state_cpuslocked+0x81/0x430
__cpuhp_setup_state+0xc3/0x260
page_alloc_init_cpuhp+0x2d/0x40
mm_core_init+0x1e/0x3a0
start_kernel+0x277/0x7f0
x86_64_start_reservations+0x18/0x30
x86_64_start_kernel+0x91/0xa0
common_startup_64+0x13e/0x148
-> #0 (cpu_hotplug_lock){++++}-{0:0}:
check_prev_add+0xe2/0xc50
validate_chain+0x57c/0x800
__lock_acquire+0x4a0/0xb50
lock_acquire+0xd1/0x2e0
__cpuhp_state_add_instance+0x40/0x250
iova_domain_init_rcaches.part.0+0x1d3/0x210
iova_domain_init_rcaches+0x41/0x60
iommu_dma_init_domain+0x1af/0x2e0
iommu_setup_dma_ops+0x65/0xe0
bus_iommu_probe+0x100/0x1d0
iommu_device_register+0xd6/0x130
intel_iommu_init+0x527/0x870
pci_iommu_init+0x17/0x60
do_one_initcall+0x7c/0x390
do_initcalls+0xe8/0x1e0
kernel_init_freeable+0x313/0x490
kernel_init+0x24/0x240
ret_from_fork+0x4a/0x60
ret_from_fork_asm+0x1a/0x30
other info that might help us debug this:
Chain exists of:
cpu_hotplug_lock --> &group->mutex --> &domain->iova_cookie->mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&domain->iova_cookie->mutex);
lock(&group->mutex);
lock(&domain->iova_cookie->mutex);
rlock(cpu_hotplug_lock);
*** DEADLOCK ***
3 locks held by swapper/0/1:
#0: ffffffffa6442ab0 (dmar_global_lock){++++}-{4:4}, at:
intel_iommu_init+0x42c/0x87
#1: ffff9f4a87b11310 (&group->mutex){+.+.}-{4:4}, at:
bus_iommu_probe+0x95/0x1d0
#2: ffff9f4a87b171c8 (&domain->iova_cookie->mutex){+.+.}-{4:4}, at:
iommu_dma_init_d
stack backtrace:
CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted
6.14.0-rc3-00002-g8e4617b46db1 #57
Call Trace:
<TASK>
dump_stack_lvl+0x93/0xd0
print_circular_bug+0x133/0x1c0
check_noncircular+0x12c/0x150
check_prev_add+0xe2/0xc50
? add_chain_cache+0x108/0x460
validate_chain+0x57c/0x800
__lock_acquire+0x4a0/0xb50
lock_acquire+0xd1/0x2e0
? iova_domain_init_rcaches.part.0+0x1d3/0x210
? rcu_is_watching+0x11/0x50
__cpuhp_state_add_instance+0x40/0x250
? iova_domain_init_rcaches.part.0+0x1d3/0x210
iova_domain_init_rcaches.part.0+0x1d3/0x210
iova_domain_init_rcaches+0x41/0x60
iommu_dma_init_domain+0x1af/0x2e0
iommu_setup_dma_ops+0x65/0xe0
bus_iommu_probe+0x100/0x1d0
iommu_device_register+0xd6/0x130
intel_iommu_init+0x527/0x870
? __pfx_pci_iommu_init+0x10/0x10
pci_iommu_init+0x17/0x60
do_one_initcall+0x7c/0x390
do_initcalls+0xe8/0x1e0
kernel_init_freeable+0x313/0x490
? __pfx_kernel_init+0x10/0x10
kernel_init+0x24/0x240
? _raw_spin_unlock_irq+0x33/0x50
ret_from_fork+0x4a/0x60
? __pfx_kernel_init+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Thanks,
baolu
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
2025-02-20 11:38 ` Baolu Lu
@ 2025-02-21 7:22 ` Tian, Kevin
2025-02-21 8:27 ` Baolu Lu
0 siblings, 1 reply; 7+ messages in thread
From: Tian, Kevin @ 2025-02-21 7:22 UTC (permalink / raw)
To: Baolu Lu, Joerg Roedel, Will Deacon, Robin Murphy
Cc: Ido Schimmel, iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Thursday, February 20, 2025 7:38 PM
>
> On 2025/2/20 15:21, Tian, Kevin wrote:
> >> From: Lu Baolu<baolu.lu@linux.intel.com>
> >> Sent: Tuesday, February 18, 2025 10:24 AM
> >>
> >> Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
> >> locally") moved the call to enable_drhd_fault_handling() to a code
> >> path that does not hold any lock while traversing the drhd list. Fix
> >> it by ensuring the dmar_global_lock lock is held when traversing the
> >> drhd list.
> >>
> >> Without this fix, the following warning is triggered:
> >> =============================
> >> WARNING: suspicious RCU usage
> >> 6.14.0-rc3 #55 Not tainted
> >> -----------------------------
> >> drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
> >> other info that might help us debug this:
> >> rcu_scheduler_active = 1, debug_locks = 1
> >> 2 locks held by cpuhp/1/23:
> >> #0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at:
> >> cpuhp_thread_fun+0x87/0x2c0
> >> #1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at:
> >> cpuhp_thread_fun+0x87/0x2c0
> >> stack backtrace:
> >> CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
> >> Call Trace:
> >> <TASK>
> >> dump_stack_lvl+0xb7/0xd0
> >> lockdep_rcu_suspicious+0x159/0x1f0
> >> ? __pfx_enable_drhd_fault_handling+0x10/0x10
> >> enable_drhd_fault_handling+0x151/0x180
> >> cpuhp_invoke_callback+0x1df/0x990
> >> cpuhp_thread_fun+0x1ea/0x2c0
> >> smpboot_thread_fn+0x1f5/0x2e0
> >> ? __pfx_smpboot_thread_fn+0x10/0x10
> >> kthread+0x12a/0x2d0
> >> ? __pfx_kthread+0x10/0x10
> >> ret_from_fork+0x4a/0x60
> >> ? __pfx_kthread+0x10/0x10
> >> ret_from_fork_asm+0x1a/0x30
> >> </TASK>
> >>
> >> Simply holding the lock in enable_drhd_fault_handling() will trigger a
> >> lock order splat. Avoid holding the dmar_global_lock when calling
> >> iommu_device_register(), which starts the device probe process.
> > Can you elaborate the splat issue? It's not intuitive to me with a quick
> > read of the code and iommu_device_register() is not occurred in above
> > calling stack.
>
> The lockdep splat looks like below:
Thanks and it's clear now. Probably you can expand "to avoid unnecessary
lock order splat " a little bit to mark the dead lock between dmar_global_lock
and cpu_hotplug_lock (acquired in path of iommu_device_register()).
With that:
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.14.0-rc3-00002-g8e4617b46db1 #57 Not tainted
> ------------------------------------------------------
> swapper/0/1 is trying to acquire lock:
> ffffffffa2a67c50 (cpu_hotplug_lock){++++}-{0:0}, at:
> iova_domain_init_rcaches.part.0+0x1d3/0x210
>
> but task is already holding lock:
> ffff9f4a87b171c8 (&domain->iova_cookie->mutex){+.+.}-{4:4}, at:
> iommu_dma_init_domain+0x122/0x2e0
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #4 (&domain->iova_cookie->mutex){+.+.}-{4:4}:
> __lock_acquire+0x4a0/0xb50
> lock_acquire+0xd1/0x2e0
> __mutex_lock+0xa5/0xce0
> iommu_dma_init_domain+0x122/0x2e0
> iommu_setup_dma_ops+0x65/0xe0
> bus_iommu_probe+0x100/0x1d0
> iommu_device_register+0xd6/0x130
> intel_iommu_init+0x527/0x870
> pci_iommu_init+0x17/0x60
> do_one_initcall+0x7c/0x390
> do_initcalls+0xe8/0x1e0
> kernel_init_freeable+0x313/0x490
> kernel_init+0x24/0x240
> ret_from_fork+0x4a/0x60
> ret_from_fork_asm+0x1a/0x30
>
> -> #3 (&group->mutex){+.+.}-{4:4}:
> __lock_acquire+0x4a0/0xb50
> lock_acquire+0xd1/0x2e0
> __mutex_lock+0xa5/0xce0
> bus_iommu_probe+0x95/0x1d0
> iommu_device_register+0xd6/0x130
> intel_iommu_init+0x527/0x870
> pci_iommu_init+0x17/0x60
> do_one_initcall+0x7c/0x390
> do_initcalls+0xe8/0x1e0
> kernel_init_freeable+0x313/0x490
> kernel_init+0x24/0x240
> ret_from_fork+0x4a/0x60
> ret_from_fork_asm+0x1a/0x30
>
> -> #2 (dmar_global_lock){++++}-{4:4}:
> __lock_acquire+0x4a0/0xb50
> lock_acquire+0xd1/0x2e0
> down_read+0x31/0x170
> enable_drhd_fault_handling+0x27/0x1a0
> cpuhp_invoke_callback+0x1e2/0x990
> cpuhp_issue_call+0xac/0x2c0
> __cpuhp_setup_state_cpuslocked+0x229/0x430
> __cpuhp_setup_state+0xc3/0x260
> irq_remap_enable_fault_handling+0x52/0x80
> apic_intr_mode_init+0x59/0xf0
> x86_late_time_init+0x29/0x50
> start_kernel+0x642/0x7f0
> x86_64_start_reservations+0x18/0x30
> x86_64_start_kernel+0x91/0xa0
> common_startup_64+0x13e/0x148
>
> -> #1 (cpuhp_state_mutex){+.+.}-{4:4}:
> __lock_acquire+0x4a0/0xb50
> lock_acquire+0xd1/0x2e0
> __mutex_lock+0xa5/0xce0
> __cpuhp_setup_state_cpuslocked+0x81/0x430
> __cpuhp_setup_state+0xc3/0x260
> page_alloc_init_cpuhp+0x2d/0x40
> mm_core_init+0x1e/0x3a0
> start_kernel+0x277/0x7f0
> x86_64_start_reservations+0x18/0x30
> x86_64_start_kernel+0x91/0xa0
> common_startup_64+0x13e/0x148
>
> -> #0 (cpu_hotplug_lock){++++}-{0:0}:
> check_prev_add+0xe2/0xc50
> validate_chain+0x57c/0x800
> __lock_acquire+0x4a0/0xb50
> lock_acquire+0xd1/0x2e0
> __cpuhp_state_add_instance+0x40/0x250
> iova_domain_init_rcaches.part.0+0x1d3/0x210
> iova_domain_init_rcaches+0x41/0x60
> iommu_dma_init_domain+0x1af/0x2e0
> iommu_setup_dma_ops+0x65/0xe0
> bus_iommu_probe+0x100/0x1d0
> iommu_device_register+0xd6/0x130
> intel_iommu_init+0x527/0x870
> pci_iommu_init+0x17/0x60
> do_one_initcall+0x7c/0x390
> do_initcalls+0xe8/0x1e0
> kernel_init_freeable+0x313/0x490
> kernel_init+0x24/0x240
> ret_from_fork+0x4a/0x60
> ret_from_fork_asm+0x1a/0x30
>
> other info that might help us debug this:
>
> Chain exists of:
> cpu_hotplug_lock --> &group->mutex --> &domain->iova_cookie->mutex
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(&domain->iova_cookie->mutex);
> lock(&group->mutex);
> lock(&domain->iova_cookie->mutex);
> rlock(cpu_hotplug_lock);
>
> *** DEADLOCK ***
>
> 3 locks held by swapper/0/1:
> #0: ffffffffa6442ab0 (dmar_global_lock){++++}-{4:4}, at:
> intel_iommu_init+0x42c/0x87
> #1: ffff9f4a87b11310 (&group->mutex){+.+.}-{4:4}, at:
> bus_iommu_probe+0x95/0x1d0
> #2: ffff9f4a87b171c8 (&domain->iova_cookie->mutex){+.+.}-{4:4}, at:
> iommu_dma_init_d
>
> stack backtrace:
> CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted
> 6.14.0-rc3-00002-g8e4617b46db1 #57
> Call Trace:
> <TASK>
> dump_stack_lvl+0x93/0xd0
> print_circular_bug+0x133/0x1c0
> check_noncircular+0x12c/0x150
> check_prev_add+0xe2/0xc50
> ? add_chain_cache+0x108/0x460
> validate_chain+0x57c/0x800
> __lock_acquire+0x4a0/0xb50
> lock_acquire+0xd1/0x2e0
> ? iova_domain_init_rcaches.part.0+0x1d3/0x210
> ? rcu_is_watching+0x11/0x50
> __cpuhp_state_add_instance+0x40/0x250
> ? iova_domain_init_rcaches.part.0+0x1d3/0x210
> iova_domain_init_rcaches.part.0+0x1d3/0x210
> iova_domain_init_rcaches+0x41/0x60
> iommu_dma_init_domain+0x1af/0x2e0
> iommu_setup_dma_ops+0x65/0xe0
> bus_iommu_probe+0x100/0x1d0
> iommu_device_register+0xd6/0x130
> intel_iommu_init+0x527/0x870
> ? __pfx_pci_iommu_init+0x10/0x10
> pci_iommu_init+0x17/0x60
> do_one_initcall+0x7c/0x390
> do_initcalls+0xe8/0x1e0
> kernel_init_freeable+0x313/0x490
> ? __pfx_kernel_init+0x10/0x10
> kernel_init+0x24/0x240
> ? _raw_spin_unlock_irq+0x33/0x50
> ret_from_fork+0x4a/0x60
> ? __pfx_kernel_init+0x10/0x10
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Thanks,
> baolu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage
2025-02-21 7:22 ` Tian, Kevin
@ 2025-02-21 8:27 ` Baolu Lu
0 siblings, 0 replies; 7+ messages in thread
From: Baolu Lu @ 2025-02-21 8:27 UTC (permalink / raw)
To: Tian, Kevin, Joerg Roedel, Will Deacon, Robin Murphy
Cc: baolu.lu, Ido Schimmel, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
On 2025/2/21 15:22, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@linux.intel.com>
>> Sent: Thursday, February 20, 2025 7:38 PM
>>
>> On 2025/2/20 15:21, Tian, Kevin wrote:
>>>> From: Lu Baolu<baolu.lu@linux.intel.com>
>>>> Sent: Tuesday, February 18, 2025 10:24 AM
>>>>
>>>> Commit <d74169ceb0d2> ("iommu/vt-d: Allocate DMAR fault interrupts
>>>> locally") moved the call to enable_drhd_fault_handling() to a code
>>>> path that does not hold any lock while traversing the drhd list. Fix
>>>> it by ensuring the dmar_global_lock lock is held when traversing the
>>>> drhd list.
>>>>
>>>> Without this fix, the following warning is triggered:
>>>> =============================
>>>> WARNING: suspicious RCU usage
>>>> 6.14.0-rc3 #55 Not tainted
>>>> -----------------------------
>>>> drivers/iommu/intel/dmar.c:2046 RCU-list traversed in non-reader section!!
>>>> other info that might help us debug this:
>>>> rcu_scheduler_active = 1, debug_locks = 1
>>>> 2 locks held by cpuhp/1/23:
>>>> #0: ffffffff84a67c50 (cpu_hotplug_lock){++++}-{0:0}, at:
>>>> cpuhp_thread_fun+0x87/0x2c0
>>>> #1: ffffffff84a6a380 (cpuhp_state-up){+.+.}-{0:0}, at:
>>>> cpuhp_thread_fun+0x87/0x2c0
>>>> stack backtrace:
>>>> CPU: 1 UID: 0 PID: 23 Comm: cpuhp/1 Not tainted 6.14.0-rc3 #55
>>>> Call Trace:
>>>> <TASK>
>>>> dump_stack_lvl+0xb7/0xd0
>>>> lockdep_rcu_suspicious+0x159/0x1f0
>>>> ? __pfx_enable_drhd_fault_handling+0x10/0x10
>>>> enable_drhd_fault_handling+0x151/0x180
>>>> cpuhp_invoke_callback+0x1df/0x990
>>>> cpuhp_thread_fun+0x1ea/0x2c0
>>>> smpboot_thread_fn+0x1f5/0x2e0
>>>> ? __pfx_smpboot_thread_fn+0x10/0x10
>>>> kthread+0x12a/0x2d0
>>>> ? __pfx_kthread+0x10/0x10
>>>> ret_from_fork+0x4a/0x60
>>>> ? __pfx_kthread+0x10/0x10
>>>> ret_from_fork_asm+0x1a/0x30
>>>> </TASK>
>>>>
>>>> Simply holding the lock in enable_drhd_fault_handling() will trigger a
>>>> lock order splat. Avoid holding the dmar_global_lock when calling
>>>> iommu_device_register(), which starts the device probe process.
>>> Can you elaborate the splat issue? It's not intuitive to me with a quick
>>> read of the code and iommu_device_register() is not occurred in above
>>> calling stack.
>> The lockdep splat looks like below:
> Thanks and it's clear now. Probably you can expand "to avoid unnecessary
> lock order splat " a little bit to mark the dead lock between dmar_global_lock
> and cpu_hotplug_lock (acquired in path of iommu_device_register()).
Yes, sure.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-02-21 8:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-18 2:24 [PATCH 1/1] iommu/vt-d: Fix suspicious RCU usage Lu Baolu
2025-02-18 10:14 ` Ido Schimmel
2025-02-18 14:09 ` Breno Leitao
2025-02-20 7:21 ` Tian, Kevin
2025-02-20 11:38 ` Baolu Lu
2025-02-21 7:22 ` Tian, Kevin
2025-02-21 8:27 ` Baolu Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox