Hi, With 5.2.0-rc5 I can easily trigger this with lockdep: -------------------------------------------- Jun 20 14:37:37 xz-x1 kernel: ====================================================== Jun 20 14:37:37 xz-x1 kernel: WARNING: possible circular locking dependency detected Jun 20 14:37:37 xz-x1 kernel: 5.2.0-rc5 #78 Not tainted Jun 20 14:37:37 xz-x1 kernel: ------------------------------------------------------ Jun 20 14:37:37 xz-x1 kernel: swapper/0/1 is trying to acquire lock: Jun 20 14:37:37 xz-x1 kernel: 00000000ea2b3beb (&(&iommu->lock)->rlock){+.+.}, at: domain_context_mapping_one+0xa5/0x4e0 Jun 20 14:37:37 xz-x1 kernel: but task is already holding lock: Jun 20 14:37:37 xz-x1 kernel: 00000000a681907b (device_domain_lock){....}, at: domain_context_mapping_one+0x8d/0x4e0 Jun 20 14:37:37 xz-x1 kernel: which lock already depends on the new lock. Jun 20 14:37:37 xz-x1 kernel: the existing dependency chain (in reverse order) is: Jun 20 14:37:37 xz-x1 kernel: -> #1 (device_domain_lock){....}: Jun 20 14:37:37 xz-x1 kernel: _raw_spin_lock_irqsave+0x3c/0x50 Jun 20 14:37:37 xz-x1 kernel: dmar_insert_one_dev_info+0xbb/0x510 Jun 20 14:37:37 xz-x1 kernel: domain_add_dev_info+0x50/0x90 Jun 20 14:37:37 xz-x1 kernel: dev_prepare_static_identity_mapping+0x30/0x68 Jun 20 14:37:37 xz-x1 kernel: intel_iommu_init+0xddd/0x1422 Jun 20 14:37:37 xz-x1 kernel: pci_iommu_init+0x16/0x3f Jun 20 14:37:37 xz-x1 kernel: do_one_initcall+0x5d/0x2b4 Jun 20 14:37:37 xz-x1 kernel: kernel_init_freeable+0x218/0x2c1 Jun 20 14:37:37 xz-x1 kernel: kernel_init+0xa/0x100 Jun 20 14:37:37 xz-x1 kernel: ret_from_fork+0x3a/0x50 Jun 20 14:37:37 xz-x1 kernel: -> #0 (&(&iommu->lock)->rlock){+.+.}: Jun 20 14:37:37 xz-x1 kernel: lock_acquire+0x9e/0x170 Jun 20 14:37:37 xz-x1 kernel: _raw_spin_lock+0x25/0x30 Jun 20 14:37:37 xz-x1 kernel: domain_context_mapping_one+0xa5/0x4e0 Jun 20 14:37:37 xz-x1 kernel: pci_for_each_dma_alias+0x30/0x140 Jun 20 14:37:37 xz-x1 kernel: dmar_insert_one_dev_info+0x3b2/0x510 Jun 20 14:37:37 xz-x1 kernel: domain_add_dev_info+0x50/0x90 Jun 20 14:37:37 xz-x1 kernel: dev_prepare_static_identity_mapping+0x30/0x68 Jun 20 14:37:37 xz-x1 kernel: intel_iommu_init+0xddd/0x1422 Jun 20 14:37:37 xz-x1 kernel: pci_iommu_init+0x16/0x3f Jun 20 14:37:37 xz-x1 kernel: do_one_initcall+0x5d/0x2b4 Jun 20 14:37:37 xz-x1 kernel: kernel_init_freeable+0x218/0x2c1 Jun 20 14:37:37 xz-x1 kernel: kernel_init+0xa/0x100 Jun 20 14:37:37 xz-x1 kernel: ret_from_fork+0x3a/0x50 Jun 20 14:37:37 xz-x1 kernel: other info that might help us debug this: Jun 20 14:37:37 xz-x1 kernel: Possible unsafe locking scenario: Jun 20 14:37:37 xz-x1 kernel: CPU0 CPU1 Jun 20 14:37:37 xz-x1 kernel: ---- ---- Jun 20 14:37:37 xz-x1 kernel: lock(device_domain_lock); Jun 20 14:37:37 xz-x1 kernel: lock(&(&iommu->lock)->rlock); Jun 20 14:37:37 xz-x1 kernel: lock(device_domain_lock); Jun 20 14:37:37 xz-x1 kernel: lock(&(&iommu->lock)->rlock); Jun 20 14:37:37 xz-x1 kernel: *** DEADLOCK *** Jun 20 14:37:37 xz-x1 kernel: 2 locks held by swapper/0/1: Jun 20 14:37:37 xz-x1 kernel: #0: 00000000033eb13d (dmar_global_lock){++++}, at: intel_iommu_init+0x1e0/0x1422 Jun 20 14:37:37 xz-x1 kernel: #1: 00000000a681907b (device_domain_lock){....}, at: domain_context_mapping_one+0x8d/0x4e0 Jun 20 14:37:37 xz-x1 kernel: stack backtrace: Jun 20 14:37:37 xz-x1 kernel: CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5 #78 Jun 20 14:37:37 xz-x1 kernel: Hardware name: LENOVO 20KGS35G01/20KGS35G01, BIOS N23ET50W (1.25 ) 06/25/2018 Jun 20 14:37:37 xz-x1 kernel: Call Trace: Jun 20 14:37:37 xz-x1 kernel: dump_stack+0x85/0xc0 Jun 20 14:37:37 xz-x1 kernel: print_circular_bug.cold.57+0x15c/0x195 Jun 20 14:37:37 xz-x1 kernel: __lock_acquire+0x152a/0x1710 Jun 20 14:37:37 xz-x1 kernel: lock_acquire+0x9e/0x170 Jun 20 14:37:37 xz-x1 kernel: ? domain_context_mapping_one+0xa5/0x4e0 Jun 20 14:37:37 xz-x1 kernel: _raw_spin_lock+0x25/0x30 Jun 20 14:37:37 xz-x1 kernel: ? domain_context_mapping_one+0xa5/0x4e0 Jun 20 14:37:37 xz-x1 kernel: domain_context_mapping_one+0xa5/0x4e0 Jun 20 14:37:37 xz-x1 kernel: ? domain_context_mapping_one+0x4e0/0x4e0 Jun 20 14:37:37 xz-x1 kernel: pci_for_each_dma_alias+0x30/0x140 Jun 20 14:37:37 xz-x1 kernel: dmar_insert_one_dev_info+0x3b2/0x510 Jun 20 14:37:37 xz-x1 kernel: domain_add_dev_info+0x50/0x90 Jun 20 14:37:37 xz-x1 kernel: dev_prepare_static_identity_mapping+0x30/0x68 Jun 20 14:37:37 xz-x1 kernel: intel_iommu_init+0xddd/0x1422 Jun 20 14:37:37 xz-x1 kernel: ? printk+0x58/0x6f Jun 20 14:37:37 xz-x1 kernel: ? lockdep_hardirqs_on+0xf0/0x180 Jun 20 14:37:37 xz-x1 kernel: ? do_early_param+0x8e/0x8e Jun 20 14:37:37 xz-x1 kernel: ? e820__memblock_setup+0x63/0x63 Jun 20 14:37:37 xz-x1 kernel: pci_iommu_init+0x16/0x3f Jun 20 14:37:37 xz-x1 kernel: do_one_initcall+0x5d/0x2b4 Jun 20 14:37:37 xz-x1 kernel: ? do_early_param+0x8e/0x8e Jun 20 14:37:37 xz-x1 kernel: ? rcu_read_lock_sched_held+0x55/0x60 Jun 20 14:37:37 xz-x1 kernel: ? do_early_param+0x8e/0x8e Jun 20 14:37:37 xz-x1 kernel: kernel_init_freeable+0x218/0x2c1 Jun 20 14:37:37 xz-x1 kernel: ? rest_init+0x230/0x230 Jun 20 14:37:37 xz-x1 kernel: kernel_init+0xa/0x100 Jun 20 14:37:37 xz-x1 kernel: ret_from_fork+0x3a/0x50 -------------------------------------------- domain_context_mapping_one() is taking device_domain_lock first then iommu lock, while dmar_insert_one_dev_info() is doing the reverse. I digged a bit and I saw this commit which seems suspicous: 7560cc3ca7d9 ("iommu/vt-d: Fix lock inversion between iommu->lock and device_domain_lock", 2019-05-27) More interestingly, it was trying to fix the inverted deadlock... The thing is that even if above commit is correct on the ordering (I still feel strange that we need to take a per-iommu lock before another global lock), that commit seems to be an incomplete fix because there's still other places that are using the other way round. When I read deeper into that commit message, it seems to be telling me that before reaching iommu_flush_dev_iotlb() we've got iommu lock somewhere but I cannot really understand how it happened because I cannot find a path that iommu lock is taken when reaching iommu_flush_dev_iotlb(). So I cannot understand how that lockdep warning message could trigger. I reverted that commit and now everything is good here (no long runs but at least previous deadlock issue is fixed). And with that, IMHO we'll actually have the correct ordering in the whole repository that we'll take device_domain_lock before per iommu lock always. Is there anything I've missed on why we have had 7560cc3ca7d9? Thanks, -- Peter Xu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Peter, I agree with you that 7560cc3ca7d9 ("iommu/vt-d: Fix lock inversion between iommu->lock and device_domain_lock") isn't a good fix. There is also another thread, https://lkml.org/lkml/2019/6/18/996, which reported this. I think we can revert this patch now. I will try to reproduce the original issue and try to find a new fix. Can you please submit the revert patch? Best regards, Baolu On 6/20/19 6:44 PM, Peter Xu wrote: > Hi, > > With 5.2.0-rc5 I can easily trigger this with lockdep: > > -------------------------------------------- > Jun 20 14:37:37 xz-x1 kernel: ====================================================== > Jun 20 14:37:37 xz-x1 kernel: WARNING: possible circular locking dependency detected > Jun 20 14:37:37 xz-x1 kernel: 5.2.0-rc5 #78 Not tainted > Jun 20 14:37:37 xz-x1 kernel: ------------------------------------------------------ > Jun 20 14:37:37 xz-x1 kernel: swapper/0/1 is trying to acquire lock: > Jun 20 14:37:37 xz-x1 kernel: 00000000ea2b3beb (&(&iommu->lock)->rlock){+.+.}, at: domain_context_mapping_one+0xa5/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: > but task is already holding lock: > Jun 20 14:37:37 xz-x1 kernel: 00000000a681907b (device_domain_lock){....}, at: domain_context_mapping_one+0x8d/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: > which lock already depends on the new lock. > Jun 20 14:37:37 xz-x1 kernel: > the existing dependency chain (in reverse order) is: > Jun 20 14:37:37 xz-x1 kernel: > -> #1 (device_domain_lock){....}: > Jun 20 14:37:37 xz-x1 kernel: _raw_spin_lock_irqsave+0x3c/0x50 > Jun 20 14:37:37 xz-x1 kernel: dmar_insert_one_dev_info+0xbb/0x510 > Jun 20 14:37:37 xz-x1 kernel: domain_add_dev_info+0x50/0x90 > Jun 20 14:37:37 xz-x1 kernel: dev_prepare_static_identity_mapping+0x30/0x68 > Jun 20 14:37:37 xz-x1 kernel: intel_iommu_init+0xddd/0x1422 > Jun 20 14:37:37 xz-x1 kernel: pci_iommu_init+0x16/0x3f > Jun 20 14:37:37 xz-x1 kernel: do_one_initcall+0x5d/0x2b4 > Jun 20 14:37:37 xz-x1 kernel: kernel_init_freeable+0x218/0x2c1 > Jun 20 14:37:37 xz-x1 kernel: kernel_init+0xa/0x100 > Jun 20 14:37:37 xz-x1 kernel: ret_from_fork+0x3a/0x50 > Jun 20 14:37:37 xz-x1 kernel: > -> #0 (&(&iommu->lock)->rlock){+.+.}: > Jun 20 14:37:37 xz-x1 kernel: lock_acquire+0x9e/0x170 > Jun 20 14:37:37 xz-x1 kernel: _raw_spin_lock+0x25/0x30 > Jun 20 14:37:37 xz-x1 kernel: domain_context_mapping_one+0xa5/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: pci_for_each_dma_alias+0x30/0x140 > Jun 20 14:37:37 xz-x1 kernel: dmar_insert_one_dev_info+0x3b2/0x510 > Jun 20 14:37:37 xz-x1 kernel: domain_add_dev_info+0x50/0x90 > Jun 20 14:37:37 xz-x1 kernel: dev_prepare_static_identity_mapping+0x30/0x68 > Jun 20 14:37:37 xz-x1 kernel: intel_iommu_init+0xddd/0x1422 > Jun 20 14:37:37 xz-x1 kernel: pci_iommu_init+0x16/0x3f > Jun 20 14:37:37 xz-x1 kernel: do_one_initcall+0x5d/0x2b4 > Jun 20 14:37:37 xz-x1 kernel: kernel_init_freeable+0x218/0x2c1 > Jun 20 14:37:37 xz-x1 kernel: kernel_init+0xa/0x100 > Jun 20 14:37:37 xz-x1 kernel: ret_from_fork+0x3a/0x50 > Jun 20 14:37:37 xz-x1 kernel: > other info that might help us debug this: > Jun 20 14:37:37 xz-x1 kernel: Possible unsafe locking scenario: > Jun 20 14:37:37 xz-x1 kernel: CPU0 CPU1 > Jun 20 14:37:37 xz-x1 kernel: ---- ---- > Jun 20 14:37:37 xz-x1 kernel: lock(device_domain_lock); > Jun 20 14:37:37 xz-x1 kernel: lock(&(&iommu->lock)->rlock); > Jun 20 14:37:37 xz-x1 kernel: lock(device_domain_lock); > Jun 20 14:37:37 xz-x1 kernel: lock(&(&iommu->lock)->rlock); > Jun 20 14:37:37 xz-x1 kernel: > *** DEADLOCK *** > Jun 20 14:37:37 xz-x1 kernel: 2 locks held by swapper/0/1: > Jun 20 14:37:37 xz-x1 kernel: #0: 00000000033eb13d (dmar_global_lock){++++}, at: intel_iommu_init+0x1e0/0x1422 > Jun 20 14:37:37 xz-x1 kernel: #1: 00000000a681907b (device_domain_lock){....}, at: domain_context_mapping_one+0x8d/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: > stack backtrace: > Jun 20 14:37:37 xz-x1 kernel: CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5 #78 > Jun 20 14:37:37 xz-x1 kernel: Hardware name: LENOVO 20KGS35G01/20KGS35G01, BIOS N23ET50W (1.25 ) 06/25/2018 > Jun 20 14:37:37 xz-x1 kernel: Call Trace: > Jun 20 14:37:37 xz-x1 kernel: dump_stack+0x85/0xc0 > Jun 20 14:37:37 xz-x1 kernel: print_circular_bug.cold.57+0x15c/0x195 > Jun 20 14:37:37 xz-x1 kernel: __lock_acquire+0x152a/0x1710 > Jun 20 14:37:37 xz-x1 kernel: lock_acquire+0x9e/0x170 > Jun 20 14:37:37 xz-x1 kernel: ? domain_context_mapping_one+0xa5/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: _raw_spin_lock+0x25/0x30 > Jun 20 14:37:37 xz-x1 kernel: ? domain_context_mapping_one+0xa5/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: domain_context_mapping_one+0xa5/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: ? domain_context_mapping_one+0x4e0/0x4e0 > Jun 20 14:37:37 xz-x1 kernel: pci_for_each_dma_alias+0x30/0x140 > Jun 20 14:37:37 xz-x1 kernel: dmar_insert_one_dev_info+0x3b2/0x510 > Jun 20 14:37:37 xz-x1 kernel: domain_add_dev_info+0x50/0x90 > Jun 20 14:37:37 xz-x1 kernel: dev_prepare_static_identity_mapping+0x30/0x68 > Jun 20 14:37:37 xz-x1 kernel: intel_iommu_init+0xddd/0x1422 > Jun 20 14:37:37 xz-x1 kernel: ? printk+0x58/0x6f > Jun 20 14:37:37 xz-x1 kernel: ? lockdep_hardirqs_on+0xf0/0x180 > Jun 20 14:37:37 xz-x1 kernel: ? do_early_param+0x8e/0x8e > Jun 20 14:37:37 xz-x1 kernel: ? e820__memblock_setup+0x63/0x63 > Jun 20 14:37:37 xz-x1 kernel: pci_iommu_init+0x16/0x3f > Jun 20 14:37:37 xz-x1 kernel: do_one_initcall+0x5d/0x2b4 > Jun 20 14:37:37 xz-x1 kernel: ? do_early_param+0x8e/0x8e > Jun 20 14:37:37 xz-x1 kernel: ? rcu_read_lock_sched_held+0x55/0x60 > Jun 20 14:37:37 xz-x1 kernel: ? do_early_param+0x8e/0x8e > Jun 20 14:37:37 xz-x1 kernel: kernel_init_freeable+0x218/0x2c1 > Jun 20 14:37:37 xz-x1 kernel: ? rest_init+0x230/0x230 > Jun 20 14:37:37 xz-x1 kernel: kernel_init+0xa/0x100 > Jun 20 14:37:37 xz-x1 kernel: ret_from_fork+0x3a/0x50 > -------------------------------------------- > > domain_context_mapping_one() is taking device_domain_lock first then > iommu lock, while dmar_insert_one_dev_info() is doing the reverse. > > I digged a bit and I saw this commit which seems suspicous: > > 7560cc3ca7d9 ("iommu/vt-d: Fix lock inversion between iommu->lock and > device_domain_lock", 2019-05-27) > > More interestingly, it was trying to fix the inverted deadlock... > > The thing is that even if above commit is correct on the ordering (I > still feel strange that we need to take a per-iommu lock before > another global lock), that commit seems to be an incomplete fix > because there's still other places that are using the other way round. > > When I read deeper into that commit message, it seems to be telling me > that before reaching iommu_flush_dev_iotlb() we've got iommu lock > somewhere but I cannot really understand how it happened because I > cannot find a path that iommu lock is taken when reaching > iommu_flush_dev_iotlb(). So I cannot understand how that lockdep > warning message could trigger. > > I reverted that commit and now everything is good here (no long runs > but at least previous deadlock issue is fixed). And with that, IMHO > we'll actually have the correct ordering in the whole repository that > we'll take device_domain_lock before per iommu lock always. > > Is there anything I've missed on why we have had 7560cc3ca7d9? > > Thanks, > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
On Fri, Jun 21, 2019 at 09:58:28AM +0800, Lu Baolu wrote: > Hi Peter, > > I agree with you that 7560cc3ca7d9 ("iommu/vt-d: Fix lock inversion between > iommu->lock and device_domain_lock") isn't a good fix. There > is also another thread, https://lkml.org/lkml/2019/6/18/996, which > reported this. > > I think we can revert this patch now. I will try to reproduce the > original issue and try to find a new fix. > > Can you please submit the revert patch? Sure, I'll post a formal patch soon. Thanks, -- Peter Xu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu