[PATCH] drm/amdkfd: Fix Circular Locking Dependency in 'svm_range_cpu_invalidate_pagetables'
Philip Yang
yangp at amd.com
Tue Feb 18 20:51:00 UTC 2025
On 2025-02-18 11:01, Srinivasan Shanmugam wrote:
> This commit addresses a circular locking dependency in the
> svm_range_cpu_invalidate_pagetables function. The function previously
> held a lock while determining whether to perform an unmap or eviction
> operation, which could lead to deadlocks.
>
> To resolve this issue, a flag named `needs_unmap_or_evict` has been
> introduced to indicate if an unmap or eviction operation is required.
> The function now unlocks the `prange` lock before performing the
> necessary operations, ensuring that the critical section is minimized
> and preventing circular dependencies.
>
> Fixes the below:
>
> [ 223.418794] ======================================================
> [ 223.418820] WARNING: possible circular locking dependency detected
> [ 223.418845] 6.12.0-amdstaging-drm-next-lol-050225 #14 Tainted: G U OE
> [ 223.418869] ------------------------------------------------------
> [ 223.418889] kfdtest/3939 is trying to acquire lock:
> [ 223.418906] ffff8957552eae38 (&dqm->lock_hidden){+.+.}-{3:3}, at: evict_process_queues_cpsch+0x43/0x210 [amdgpu]
> [ 223.419302]
> but task is already holding lock:
> [ 223.419303] ffff8957556b83b0 (&prange->lock){+.+.}-{3:3}, at: svm_range_cpu_invalidate_pagetables+0x9d/0x850 [amdgpu]
> [ 223.419447] Console: switching to colour dummy device 80x25
> [ 223.419477] [IGT] amd_basic: executing
> [ 223.419599]
> which lock already depends on the new lock.
>
> [ 223.419611]
> the existing dependency chain (in reverse order) is:
> [ 223.419621]
> -> #2 (&prange->lock){+.+.}-{3:3}:
> [ 223.419636] __mutex_lock+0x85/0xe20
> [ 223.419647] mutex_lock_nested+0x1b/0x30
> [ 223.419656] svm_range_validate_and_map+0x2f1/0x15b0 [amdgpu]
> [ 223.419954] svm_range_set_attr+0xe8c/0x1710 [amdgpu]
> [ 223.420236] svm_ioctl+0x46/0x50 [amdgpu]
> [ 223.420503] kfd_ioctl_svm+0x50/0x90 [amdgpu]
> [ 223.420763] kfd_ioctl+0x409/0x6d0 [amdgpu]
> [ 223.421024] __x64_sys_ioctl+0x95/0xd0
> [ 223.421036] x64_sys_call+0x1205/0x20d0
> [ 223.421047] do_syscall_64+0x87/0x140
> [ 223.421056] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 223.421068]
> -> #1 (reservation_ww_class_mutex){+.+.}-{3:3}:
> [ 223.421084] __ww_mutex_lock.constprop.0+0xab/0x1560
> [ 223.421095] ww_mutex_lock+0x2b/0x90
> [ 223.421103] amdgpu_amdkfd_alloc_gtt_mem+0xcc/0x2b0 [amdgpu]
> [ 223.421361] add_queue_mes+0x3bc/0x440 [amdgpu]
> [ 223.421623] unhalt_cpsch+0x1ae/0x240 [amdgpu]
> [ 223.421888] kgd2kfd_start_sched+0x5e/0xd0 [amdgpu]
> [ 223.422148] amdgpu_amdkfd_start_sched+0x3d/0x50 [amdgpu]
> [ 223.422414] amdgpu_gfx_enforce_isolation_handler+0x132/0x270 [amdgpu]
> [ 223.422662] process_one_work+0x21e/0x680
> [ 223.422673] worker_thread+0x190/0x330
> [ 223.422682] kthread+0xe7/0x120
> [ 223.422690] ret_from_fork+0x3c/0x60
> [ 223.422699] ret_from_fork_asm+0x1a/0x30
> [ 223.422708]
This hold dqm_lock, then allocate pdd->proc_ctx_bo and map to GPU, it is
illegal usage.
Not sure but seems pdd->proc_ctx_bo/proc_ctx_gpu_addr is only needed by
debug mode path, as kfd_dbg_set_mes_debug_mode also allocate
proc_ctx_bo, the proc_ctx_bo allocation should remove from
add_queue_mes, and move the allocation into
kfd_create_process_device_data if this is also needed for non debug_mode
path.
> -> #0 (&dqm->lock_hidden){+.+.}-{3:3}:
> [ 223.422723] __lock_acquire+0x16f4/0x2810
> [ 223.422734] lock_acquire+0xd1/0x300
> [ 223.422742] __mutex_lock+0x85/0xe20
> [ 223.422751] mutex_lock_nested+0x1b/0x30
> [ 223.422760] evict_process_queues_cpsch+0x43/0x210 [amdgpu]
> [ 223.423025] kfd_process_evict_queues+0x8a/0x1d0 [amdgpu]
> [ 223.423285] kgd2kfd_quiesce_mm+0x43/0x90 [amdgpu]
> [ 223.423540] svm_range_cpu_invalidate_pagetables+0x4a7/0x850 [amdgpu]
> [ 223.423807] __mmu_notifier_invalidate_range_start+0x1f5/0x250
> [ 223.423819] copy_page_range+0x1e94/0x1ea0
> [ 223.423829] copy_process+0x172f/0x2ad0
> [ 223.423839] kernel_clone+0x9c/0x3f0
> [ 223.423847] __do_sys_clone+0x66/0x90
> [ 223.423856] __x64_sys_clone+0x25/0x30
> [ 223.423864] x64_sys_call+0x1d7c/0x20d0
> [ 223.423872] do_syscall_64+0x87/0x140
> [ 223.423880] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 223.423891]
> other info that might help us debug this:
>
> [ 223.423903] Chain exists of:
> &dqm->lock_hidden --> reservation_ww_class_mutex --> &prange->lock
>
> [ 223.423926] Possible unsafe locking scenario:
>
> [ 223.423935] CPU0 CPU1
> [ 223.423942] ---- ----
> [ 223.423949] lock(&prange->lock);
> [ 223.423958] lock(reservation_ww_class_mutex);
> [ 223.423970] lock(&prange->lock);
> [ 223.423981] lock(&dqm->lock_hidden);
> [ 223.423990]
> *** DEADLOCK ***
>
> [ 223.423999] 5 locks held by kfdtest/3939:
> [ 223.424006] #0: ffffffffb82b4fc0 (dup_mmap_sem){.+.+}-{0:0}, at: copy_process+0x1387/0x2ad0
> [ 223.424026] #1: ffff89575eda81b0 (&mm->mmap_lock){++++}-{3:3}, at: copy_process+0x13a8/0x2ad0
> [ 223.424046] #2: ffff89575edaf3b0 (&mm->mmap_lock/1){+.+.}-{3:3}, at: copy_process+0x13e4/0x2ad0
> [ 223.424066] #3: ffffffffb82e76e0 (mmu_notifier_invalidate_range_start){+.+.}-{0:0}, at: copy_page_range+0x1cea/0x1ea0
> [ 223.424088] #4: ffff8957556b83b0 (&prange->lock){+.+.}-{3:3}, at: svm_range_cpu_invalidate_pagetables+0x9d/0x850 [amdgpu]
> [ 223.424365]
> stack backtrace:
> [ 223.424374] CPU: 0 UID: 0 PID: 3939 Comm: kfdtest Tainted: G U OE 6.12.0-amdstaging-drm-next-lol-050225 #14
> [ 223.424392] Tainted: [U]=USER, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> [ 223.424401] Hardware name: Gigabyte Technology Co., Ltd. X570 AORUS PRO WIFI/X570 AORUS PRO WIFI, BIOS F36a 02/16/2022
> [ 223.424416] Call Trace:
> [ 223.424423] <TASK>
> [ 223.424430] dump_stack_lvl+0x9b/0xf0
> [ 223.424441] dump_stack+0x10/0x20
> [ 223.424449] print_circular_bug+0x275/0x350
> [ 223.424460] check_noncircular+0x157/0x170
> [ 223.424469] ? __bfs+0xfd/0x2c0
> [ 223.424481] __lock_acquire+0x16f4/0x2810
> [ 223.424490] ? srso_return_thunk+0x5/0x5f
> [ 223.424505] lock_acquire+0xd1/0x300
> [ 223.424514] ? evict_process_queues_cpsch+0x43/0x210 [amdgpu]
> [ 223.424783] __mutex_lock+0x85/0xe20
> [ 223.424792] ? evict_process_queues_cpsch+0x43/0x210 [amdgpu]
> [ 223.425058] ? srso_return_thunk+0x5/0x5f
> [ 223.425067] ? mark_held_locks+0x54/0x90
> [ 223.425076] ? evict_process_queues_cpsch+0x43/0x210 [amdgpu]
> [ 223.425339] ? srso_return_thunk+0x5/0x5f
> [ 223.425350] mutex_lock_nested+0x1b/0x30
> [ 223.425358] ? mutex_lock_nested+0x1b/0x30
> [ 223.425367] evict_process_queues_cpsch+0x43/0x210 [amdgpu]
> [ 223.425631] kfd_process_evict_queues+0x8a/0x1d0 [amdgpu]
> [ 223.425893] kgd2kfd_quiesce_mm+0x43/0x90 [amdgpu]
> [ 223.426156] svm_range_cpu_invalidate_pagetables+0x4a7/0x850 [amdgpu]
> [ 223.426423] ? srso_return_thunk+0x5/0x5f
> [ 223.426436] __mmu_notifier_invalidate_range_start+0x1f5/0x250
> [ 223.426450] copy_page_range+0x1e94/0x1ea0
> [ 223.426461] ? srso_return_thunk+0x5/0x5f
> [ 223.426474] ? srso_return_thunk+0x5/0x5f
> [ 223.426484] ? lock_acquire+0xd1/0x300
> [ 223.426494] ? copy_process+0x1718/0x2ad0
> [ 223.426502] ? srso_return_thunk+0x5/0x5f
> [ 223.426510] ? sched_clock_noinstr+0x9/0x10
> [ 223.426519] ? local_clock_noinstr+0xe/0xc0
> [ 223.426528] ? copy_process+0x1718/0x2ad0
> [ 223.426537] ? srso_return_thunk+0x5/0x5f
> [ 223.426550] copy_process+0x172f/0x2ad0
> [ 223.426569] kernel_clone+0x9c/0x3f0
> [ 223.426577] ? __schedule+0x4c9/0x1b00
> [ 223.426586] ? srso_return_thunk+0x5/0x5f
> [ 223.426594] ? sched_clock_noinstr+0x9/0x10
> [ 223.426602] ? srso_return_thunk+0x5/0x5f
> [ 223.426610] ? local_clock_noinstr+0xe/0xc0
> [ 223.426619] ? schedule+0x107/0x1a0
> [ 223.426629] __do_sys_clone+0x66/0x90
> [ 223.426643] __x64_sys_clone+0x25/0x30
> [ 223.426652] x64_sys_call+0x1d7c/0x20d0
> [ 223.426661] do_syscall_64+0x87/0x140
> [ 223.426671] ? srso_return_thunk+0x5/0x5f
> [ 223.426679] ? common_nsleep+0x44/0x50
> [ 223.426690] ? srso_return_thunk+0x5/0x5f
> [ 223.426698] ? trace_hardirqs_off+0x52/0xd0
> [ 223.426709] ? srso_return_thunk+0x5/0x5f
> [ 223.426717] ? syscall_exit_to_user_mode+0xcc/0x200
> [ 223.426727] ? srso_return_thunk+0x5/0x5f
> [ 223.426736] ? do_syscall_64+0x93/0x140
> [ 223.426748] ? srso_return_thunk+0x5/0x5f
> [ 223.426756] ? up_write+0x1c/0x1e0
> [ 223.426765] ? srso_return_thunk+0x5/0x5f
> [ 223.426775] ? srso_return_thunk+0x5/0x5f
> [ 223.426783] ? trace_hardirqs_off+0x52/0xd0
> [ 223.426792] ? srso_return_thunk+0x5/0x5f
> [ 223.426800] ? syscall_exit_to_user_mode+0xcc/0x200
> [ 223.426810] ? srso_return_thunk+0x5/0x5f
> [ 223.426818] ? do_syscall_64+0x93/0x140
> [ 223.426826] ? syscall_exit_to_user_mode+0xcc/0x200
> [ 223.426836] ? srso_return_thunk+0x5/0x5f
> [ 223.426844] ? do_syscall_64+0x93/0x140
> [ 223.426853] ? srso_return_thunk+0x5/0x5f
> [ 223.426861] ? irqentry_exit+0x6b/0x90
> [ 223.426869] ? srso_return_thunk+0x5/0x5f
> [ 223.426877] ? exc_page_fault+0xa7/0x2c0
> [ 223.426888] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 223.426898] RIP: 0033:0x7f46758eab57
> [ 223.426906] Code: ba 04 00 f3 0f 1e fa 64 48 8b 04 25 10 00 00 00 45 31 c0 31 d2 31 f6 bf 11 00 20 01 4c 8d 90 d0 02 00 00 b8 38 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 41 41 89 c0 85 c0 75 2c 64 48 8b 04 25 10 00
> [ 223.426930] RSP: 002b:00007fff5c3e5188 EFLAGS: 00000246 ORIG_RAX: 0000000000000038
> [ 223.426943] RAX: ffffffffffffffda RBX: 00007f4675f8c040 RCX: 00007f46758eab57
> [ 223.426954] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000001200011
> [ 223.426965] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> [ 223.426975] R10: 00007f4675e81a50 R11: 0000000000000246 R12: 0000000000000001
> [ 223.426986] R13: 00007fff5c3e5470 R14: 00007fff5c3e53e0 R15: 00007fff5c3e5410
> [ 223.427004] </TASK>
>
> Fixes: 4683cfecadeb ("drm/amdkfd: deregister svm range")
> Cc: Philip Yang <Philip.Yang at amd.com>
> Cc: Alex Sierra <alex.sierra at amd.com>
> Cc: Felix Kuehling <Felix.Kuehling at amd.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index db3034b00dac..a076269cce7f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2574,6 +2574,7 @@ svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
> struct svm_range *prange;
> unsigned long start;
> unsigned long last;
> + bool needs_unmap_or_evict = false;
>
> if (range->event == MMU_NOTIFY_RELEASE)
> return true;
> @@ -2597,14 +2598,21 @@ svm_range_cpu_invalidate_pagetables(struct mmu_interval_notifier *mni,
>
> switch (range->event) {
> case MMU_NOTIFY_UNMAP:
> - svm_range_unmap_from_cpu(mni->mm, prange, start, last);
> + needs_unmap_or_evict = true;
> break;
> default:
> - svm_range_evict(prange, mni->mm, start, last, range->event);
> + needs_unmap_or_evict = true;
> break;
> }
> -
> svm_range_unlock(prange);
> +
> + if (needs_unmap_or_evict) {
> + if (range->event == MMU_NOTIFY_UNMAP)
> + svm_range_unmap_from_cpu(mni->mm, prange, start, last);
This is incorrect, we should hold prange lock to split prange.
Regards,
Philip
> + else
> + svm_range_evict(prange, mni->mm, start, last, range->event);
> + }
> +
> mmput(mni->mm);
>
> return true;
More information about the amd-gfx
mailing list