Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
powerpc/mm_iommu: Fix potential deadlock
Currently mm_iommu_do_alloc() is called in 2 cases: - VFIO_IOMMU_SPAPR_REGISTER_MEMORY ioctl() for normal memory; - vfio_pci_nvgpu_regops::mmap() for GPU memory. One of the differences here is that the mmap() is called with mm::mmap_sem help and mm_iommu_do_alloc() locks mm::mmap_sem itself (when adjusting locked_vm and when pinning pages) which can potentially cause a deadlock. We did not hit this yet because the mmap() path does not adjust locked_vm and does not pin pages. However with CONFIG_DEBUG_LOCKDEP=y there is an annoying warning (below, it is slightly confusing). This makes a few changes to reduce the amount of time spent under a lock. This holds mem_list_mutex only when looking or changing the mem list. This means the list is checked twice now for the normal memory case - before starting pinning and before adding the item to the list. This changes mm_iommu_do_alloc() to only allocate and add an iommu memory descriptor (used to deal with both normal and GPU memory in a rather messy way). This cleans the code in a way that mm_iommu_new() and mm_iommu_do_alloc() do not need to test for (dev_hpa != MM_IOMMU_TABLE_INVALID_HPA) which makes the code simpler. This moves locked_vm decrementing from under mem_list_mutex for the same reasons. This is one of the lockdep warnings: ====================================================== WARNING: possible circular locking dependency detected 5.1.0-rc2-le_nv2_aikATfstn1-p1 torvalds#363 Not tainted ------------------------------------------------------ qemu-system-ppc/8038 is trying to acquire lock: 000000002ec6c453 (mem_list_mutex){+.+.}, at: mm_iommu_do_alloc+0x70/0x490 but task is already holding lock: 00000000fd7da97f (&mm->mmap_sem){++++}, at: vm_mmap_pgoff+0xf0/0x160 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&mm->mmap_sem){++++}: lock_acquire+0xf8/0x260 down_write+0x44/0xa0 mm_iommu_adjust_locked_vm.part.1+0x4c/0x190 mm_iommu_do_alloc+0x310/0x490 tce_iommu_ioctl.part.9+0xb84/0x1150 [vfio_iommu_spapr_tce] vfio_fops_unl_ioctl+0x94/0x430 [vfio] do_vfs_ioctl+0xe4/0x930 ksys_ioctl+0xc4/0x110 sys_ioctl+0x28/0x80 system_call+0x5c/0x70 -> #0 (mem_list_mutex){+.+.}: __lock_acquire+0x1484/0x1900 lock_acquire+0xf8/0x260 __mutex_lock+0x88/0xa70 mm_iommu_do_alloc+0x70/0x490 vfio_pci_nvgpu_mmap+0xc0/0x130 [vfio_pci] vfio_pci_mmap+0x198/0x2a0 [vfio_pci] vfio_device_fops_mmap+0x44/0x70 [vfio] mmap_region+0x5d4/0x770 do_mmap+0x42c/0x650 vm_mmap_pgoff+0x124/0x160 ksys_mmap_pgoff+0xdc/0x2f0 sys_mmap+0x40/0x80 system_call+0x5c/0x70 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&mm->mmap_sem); lock(mem_list_mutex); lock(&mm->mmap_sem); lock(mem_list_mutex); *** DEADLOCK *** 1 lock held by qemu-system-ppc/8038: #0: 00000000fd7da97f (&mm->mmap_sem){++++}, at: vm_mmap_pgoff+0xf0/0x160 Signed-off-by: Alexey Kardashevskiy <[email protected]>
- Loading branch information