tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

drm/amdgpu: replace PASID IDR with XArray

Replace the PASID IDR + spinlock with XArray as noted in the TODO
left by commit ea56aa262570 ("drm/amdgpu: fix the idr allocation
flags").

The IDR conversion still has an IRQ safety issue:
amdgpu_pasid_free() can be called from hardirq context via the fence
signal path, but amdgpu_pasid_idr_lock is taken with plain spin_lock()
in process context, creating a potential deadlock:

CPU0
----
spin_lock(&amdgpu_pasid_idr_lock) // process context, IRQs on
<Interrupt>
spin_lock(&amdgpu_pasid_idr_lock) // deadlock

The hardirq call chain is:

sdma_v6_0_process_trap_irq
-> amdgpu_fence_process
-> dma_fence_signal
-> drm_sched_job_done
-> dma_fence_signal
-> amdgpu_pasid_free_cb
-> amdgpu_pasid_free

Use XArray with XA_FLAGS_LOCK_IRQ (all xa operations use IRQ-safe
locking internally) and XA_FLAGS_ALLOC1 (zero is not a valid PASID).
Both xa_alloc_cyclic() and xa_erase() then handle locking
consistently, fixing the IRQ safety issue and removing the need for
an explicit spinlock.

v8: squash in irq safe fix

Reviewed-by: Christian König <christian.koenig@amd.com>
Suggested-by: Lijo Lazar <lijo.lazar@amd.com>
Fixes: ea56aa262570 ("drm/amdgpu: fix the idr allocation flags")
Fixes: 8f1de51f49be ("drm/amdgpu: prevent immediate PASID reuse case")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

authored by

Mikhail Gavrilov and committed by
Alex Deucher 3c863ff9 1d4ade36

+19 -20
+19 -20
drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
··· 22 22 */ 23 23 #include "amdgpu_ids.h" 24 24 25 - #include <linux/idr.h> 25 + #include <linux/xarray.h> 26 26 #include <linux/dma-fence-array.h> 27 27 28 28 ··· 40 40 * VMs are looked up from the PASID per amdgpu_device. 41 41 */ 42 42 43 - static DEFINE_IDR(amdgpu_pasid_idr); 44 - static DEFINE_SPINLOCK(amdgpu_pasid_idr_lock); 43 + static DEFINE_XARRAY_FLAGS(amdgpu_pasid_xa, XA_FLAGS_LOCK_IRQ | XA_FLAGS_ALLOC1); 44 + static u32 amdgpu_pasid_xa_next; 45 45 46 46 /* Helper to free pasid from a fence callback */ 47 47 struct amdgpu_pasid_cb { ··· 62 62 */ 63 63 int amdgpu_pasid_alloc(unsigned int bits) 64 64 { 65 - int pasid; 65 + u32 pasid; 66 + int r; 66 67 67 68 if (bits == 0) 68 69 return -EINVAL; 69 70 70 - spin_lock(&amdgpu_pasid_idr_lock); 71 - /* TODO: Need to replace the idr with an xarry, and then 72 - * handle the internal locking with ATOMIC safe paths. 73 - */ 74 - pasid = idr_alloc_cyclic(&amdgpu_pasid_idr, NULL, 1, 75 - 1U << bits, GFP_ATOMIC); 76 - spin_unlock(&amdgpu_pasid_idr_lock); 71 + r = xa_alloc_cyclic_irq(&amdgpu_pasid_xa, &pasid, xa_mk_value(0), 72 + XA_LIMIT(1, (1U << bits) - 1), 73 + &amdgpu_pasid_xa_next, GFP_KERNEL); 74 + if (r < 0) 75 + return r; 77 76 78 - if (pasid >= 0) 79 - trace_amdgpu_pasid_allocated(pasid); 80 - 77 + trace_amdgpu_pasid_allocated(pasid); 81 78 return pasid; 82 79 } 83 80 84 81 /** 85 82 * amdgpu_pasid_free - Free a PASID 86 83 * @pasid: PASID to free 84 + * 85 + * Called in IRQ context. 87 86 */ 88 87 void amdgpu_pasid_free(u32 pasid) 89 88 { 89 + unsigned long flags; 90 + 90 91 trace_amdgpu_pasid_freed(pasid); 91 92 92 - spin_lock(&amdgpu_pasid_idr_lock); 93 - idr_remove(&amdgpu_pasid_idr, pasid); 94 - spin_unlock(&amdgpu_pasid_idr_lock); 93 + xa_lock_irqsave(&amdgpu_pasid_xa, flags); 94 + __xa_erase(&amdgpu_pasid_xa, pasid); 95 + xa_unlock_irqrestore(&amdgpu_pasid_xa, flags); 95 96 } 96 97 97 98 static void amdgpu_pasid_free_cb(struct dma_fence *fence, ··· 635 634 */ 636 635 void amdgpu_pasid_mgr_cleanup(void) 637 636 { 638 - spin_lock(&amdgpu_pasid_idr_lock); 639 - idr_destroy(&amdgpu_pasid_idr); 640 - spin_unlock(&amdgpu_pasid_idr_lock); 637 + xa_destroy(&amdgpu_pasid_xa); 641 638 }