Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Removing the `fs_reclaim` lockdep annotation silences a true-positive 
warning about a real ABBA deadlock between memory reclaim and GPU reset, rather 
than fixing the underlying bug.
--

commit 4896f663f62249014930226b2bc964c2419486d0
Author: Mikhail Gavrilov <[email protected]>

drm/amdgpu: do not enter fs_reclaim under notifier_lock in lockdep training

The commit removes the fs_reclaim_acquire() and fs_reclaim_release() calls
from amdgpu_lockdep_init() to silence a circular locking dependency warning,
claiming it is a false positive.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
> index d5d71fd7c70d0..eeb3b5007f80f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
> @@ -150,12 +150,6 @@ int amdgpu_lockdep_init(void)
>       /* Level 6: Reset control lock */
>       mutex_lock(&reset_ctl.reset_lock);
>  
> -     /*
> -      * Mark potential memory reclaim boundary.
> -      * GPU operations might trigger memory allocation/reclaim.
> -      */
> -     fs_reclaim_acquire(GFP_KERNEL);
> -

[Severity: High]
This is a pre-existing issue, but does removing this lockdep annotation
silence a true-positive warning about a real ABBA deadlock?

While the circular dependency might not happen in the same thread, couldn't a
cross-thread deadlock occur between memory reclaim and GPU reset?

For example, if Thread A executes the MMU notifier amdgpu_hmm_invalidate_gfx()
(e.g., during direct reclaim), it acquires adev->notifier_lock and blocks in
dma_resv_wait_timeout() waiting for a hardware fence:

drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c:amdgpu_hmm_invalidate_gfx() {
        ...
        mutex_lock(&adev->notifier_lock);
        ...
        r = dma_resv_wait_timeout(vm_root->tbo.base.resv,
                                  DMA_RESV_USAGE_BOOKKEEP, false,
                                  MAX_SCHEDULE_TIMEOUT);
}

If Thread B (the GPU reset thread) takes down_write(&adev->reset_domain->sem)
to reset the GPU and signal the fences, and performs GFP_KERNEL allocations,
it could be forced into direct memory reclaim (fs_reclaim). This would invoke
the MMU notifier, attempting to acquire adev->notifier_lock, forming a cycle.

Would it be better to fix the underlying deadlock by wrapping the GPU reset
path with memalloc_noreclaim_save() or memalloc_noio_save() to prevent it
from entering memory reclaim, instead of removing the lockdep annotation?

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to