On 3/30/26 07:30, Mikhail Gavrilov wrote:
> Commit 14b81abe7bdc ("drm/amdgpu: prevent immediate PASID reuse case")
> switched from ida to idr_alloc_cyclic() protected by a spinlock, but
> passes GFP_KERNEL to the allocator.  idr_alloc_cyclic() may need to
> allocate radix-tree nodes, which with GFP_KERNEL can sleep — illegal
> under a spinlock that disables preemption.  With CONFIG_PREEMPT or
> lockdep enabled this triggers:
> 
>   BUG: sleeping function called from invalid context at
>        ./include/linux/sched/mm.h:323
>   in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 570
>   ...
>   #1: ffffffffc2cd24f8 (amdgpu_pasid_idr_lock){+.+.}-{3:3},
>       at: amdgpu_pasid_alloc+0x24/0x210 [amdgpu]
>   ...
>   kmem_cache_alloc_noprof+0x41d/0x780
>   radix_tree_node_alloc.constprop.0+0x56/0x3a0
>   idr_get_free+0x330/0x830
>   idr_alloc_u32+0x14a/0x2e0
>   idr_alloc_cyclic+0xd3/0x1d0
>   amdgpu_pasid_alloc+0x51/0x210 [amdgpu]
> 
> A mutex is not an option because amdgpu_pasid_free() is reachable from
> dma-fence callbacks (amdgpu_pasid_free_cb) which may run in IRQ context.
> 
> Use idr_preload(GFP_KERNEL) before taking the spinlock to pre-allocate
> radix-tree nodes, then pass GFP_NOWAIT inside the critical section so
> the allocator draws from the preloaded pool and never sleeps.  This is
> the standard kernel pattern for IDR allocation under a spinlock.
> 
> Fixes: 14b81abe7bdc ("drm/amdgpu: prevent immediate PASID reuse case")
> Cc: [email protected]

The patch introducing this was never released to any stable kernel as far as I 
know.

> Signed-off-by: Mikhail Gavrilov <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index d88523568b62..515775eab2ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -67,10 +67,12 @@ int amdgpu_pasid_alloc(unsigned int bits)
>       if (bits == 0)
>               return -EINVAL;
>  
> +     idr_preload(GFP_KERNEL);
>       spin_lock(&amdgpu_pasid_idr_lock);
>       pasid = idr_alloc_cyclic(&amdgpu_pasid_idr, NULL, 1,
> -                              1U << bits, GFP_KERNEL);
> +                              1U << bits, GFP_NOWAIT);

That needs to be GFP_ATOMIC and not GFP_NOWAIT.

Regards,
Christian.

>       spin_unlock(&amdgpu_pasid_idr_lock);
> +     idr_preload_end();
>  
>       if (pasid >= 0)
>               trace_amdgpu_pasid_allocated(pasid);

Reply via email to