From: Vitaly Prosyak <[email protected]>
Move fs_reclaim_acquire() to before all lock acquisitions to teach
lockdep the correct dependency order and eliminate false positive
circular locking dependency warning.
PROBLEM:
amdgpu_lockdep_init() trains lockdep by acquiring locks in sequence
while calling fs_reclaim_acquire() in the middle. The fs_reclaim call
was placed while notifier_lock was held, teaching lockdep:
notifier_lock -> fs_reclaim
However, at runtime, MMU notifier callbacks run from inside memory
reclaim, establishing the opposite dependency:
fs_reclaim -> mmu_notifier -> notifier_lock
These form a false cycle that lockdep reports when reclaim unmaps a
page covered by an amdgpu userptr notifier.
LOCKDEP WARNING EXAMPLE (BEFORE FIX):
WARNING: possible circular locking dependency detected
6.19.0+ #48 Not tainted
amd_lockdep/3762 is trying to acquire lock:
ffff8881756f3c30 (&amdgpu_notifier_lock_key), at: amdgpu_hmm_invalidate_gfx
but task is already holding lock:
ffffffff92e4b5c0 (mmu_notifier_invalidate_range_start), at: try_to_unmap_one
Chain exists of:
&amdgpu_notifier_lock_key --> fs_reclaim -->
mmu_notifier_invalidate_range_start
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(mmu_notifier_invalidate_range_start);
lock(fs_reclaim);
lock(mmu_notifier_invalidate_range_start);
lock(&amdgpu_notifier_lock_key);
*** DEADLOCK ***
The dependency chain shows:
-> #5 (fs_reclaim):
amdgpu_lockdep_init+0x61e/0x730 [amdgpu] <- FALSE EDGE
-> #0 (&amdgpu_notifier_lock_key):
amdgpu_hmm_invalidate_gfx+0x77/0x110 [amdgpu] <- RUNTIME PATH
SOLUTION:
Move fs_reclaim_acquire/release to BEFORE all lock acquisitions.
This teaches lockdep that fs_reclaim is outermost, matching runtime
where reclaim calls MMU notifiers which then take notifier_lock.
VERIFICATION (AFTER FIX):
Test reproducer (based on Michael Gavrilov's standalone test case)
creates a 64MB GPU userptr memory buffer, then repeatedly forces the
kernel to reclaim it by moving pages to disk (simulating memory
pressure). Each reclaim cycle triggers the MMU notifier callback path.
The test would be available as an IGT subtest:
tests/amdgpu/amd_lockdep.c::notifier-reclaim-splat
Results:
Before fix: Lockdep false positive warning on first reclaim attempt
After fix: Completed 8 reclaim cycles with no lockdep warnings
dmesg after fix shows only test success messages, no circular dependency
warnings. The fix is minimal (relocate 2 lines) with no runtime overhead,
only corrects lockdep's static analysis model.
Reported-by: Michael Gavrilov <[email protected]>
Analyzed-by: Michael Gavrilov <[email protected]>
Test-case-by: Michael Gavrilov <[email protected]>
Suggested-by: Christian König <[email protected]>
Tested-by: Vitaly Prosyak <[email protected]>
Cc: Christian König <[email protected]>
Cc: Alex Deucher <[email protected]>
Signed-off-by: Vitaly Prosyak <[email protected]>
Change-Id: I584f833aaeac39d7eb777980f49ee503e70eb058
---
drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c | 47 +++++++++++++++------
1 file changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
index 61450af539a6..718411f724f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
@@ -18,6 +18,7 @@
struct amdgpu_lockdep_dummy_locks {
struct mutex reset_lock;
+ struct mutex filelist_mutex;
struct mutex userq_sch_mutex;
struct mutex userq_mutex;
struct mutex notifier_lock;
@@ -34,6 +35,7 @@ static struct lock_class_key amdgpu_notifier_lock_key;
static struct lock_class_key amdgpu_vram_lock_key;
static struct lock_class_key amdgpu_reset_sem_key;
static struct lock_class_key amdgpu_reset_lock_key;
+static struct lock_class_key amdgpu_filelist_mutex_key;
static struct lock_class_key amdgpu_srbm_lock_key;
static struct lock_class_key amdgpu_grbm_lock_key;
static struct lock_class_key amdgpu_mmio_lock_key;
@@ -57,6 +59,9 @@ void amdgpu_lockdep_set_class(struct amdgpu_device *adev)
if (adev->reset_domain)
lockdep_set_class(&adev->reset_domain->sem,
&amdgpu_reset_sem_key);
+
+ lockdep_set_class(&adev_to_drm(adev)->filelist_mutex,
+ &amdgpu_filelist_mutex_key);
}
/**
@@ -74,9 +79,10 @@ void amdgpu_lockdep_set_class(struct amdgpu_device *adev)
* 4. vram_lock - VRAM allocator lock
* 5. reset_domain->sem - GPU reset synchronization
* 6. reset_lock - Reset control lock
- * 7. srbm_mutex - SRBM register access
- * 8. grbm_idx_mutex - GRBM index access
- * 9. mmio_idx_lock - MMIO index access (spinlock)
+ * 7. filelist_mutex - DRM file list iteration (ddev->filelist_mutex)
+ * 8. srbm_mutex - SRBM register access
+ * 9. grbm_idx_mutex - GRBM index access
+ * 10. mmio_idx_lock - MMIO index access (spinlock)
*
* Evidence:
* - userq_sch_mutex -> userq_mutex: amdgpu_gfx_kfd_sch_ctrl() calls
@@ -88,6 +94,9 @@ void amdgpu_lockdep_set_class(struct amdgpu_device *adev)
* must be outer to reset_domain->sem
* - vram_lock -> reset_domain->sem: VRAM management paths may need to
* wait for ongoing reset to complete
+ * - reset_domain->sem -> filelist_mutex: GPU reset path
+ * (amdgpu_device_gpu_recover) holds reset_domain->sem and calls
+ * amdgpu_device_eventfd_signal_gpu_reset() which takes filelist_mutex
*
* Note: mmap_lock ordering relative to GPU locks is already taught
* by dma-resv (drivers/dma-buf/dma-resv.c).
@@ -117,6 +126,7 @@ int amdgpu_lockdep_init(void)
mutex_init(&locks->notifier_lock);
mutex_init(&locks->vram_lock);
mutex_init(&locks->reset_lock);
+ mutex_init(&locks->filelist_mutex);
mutex_init(&locks->srbm_mutex);
mutex_init(&locks->grbm_idx_mutex);
spin_lock_init(&locks->mmio_idx_lock);
@@ -132,6 +142,7 @@ int amdgpu_lockdep_init(void)
lockdep_set_class(&locks->vram_lock, &amdgpu_vram_lock_key);
lockdep_set_class(&reset_domain->sem, &amdgpu_reset_sem_key);
lockdep_set_class(&locks->reset_lock, &amdgpu_reset_lock_key);
+ lockdep_set_class(&locks->filelist_mutex, &amdgpu_filelist_mutex_key);
lockdep_set_class(&locks->srbm_mutex, &amdgpu_srbm_lock_key);
lockdep_set_class(&locks->grbm_idx_mutex, &amdgpu_grbm_lock_key);
lockdep_set_class(&locks->mmio_idx_lock, &amdgpu_mmio_lock_key);
@@ -139,6 +150,18 @@ int amdgpu_lockdep_init(void)
* Take locks in the correct order to train lockdep.
* This establishes the dependency chain.
*/
+ /*
+ * Train fs_reclaim FIRST, before taking any locks.
+ * This teaches lockdep that fs_reclaim is the OUTERMOST context.
+ * When memory reclaim later calls MMU notifiers (which take
notifier_lock),
+ * lockdep will see: fs_reclaim -> notifier_lock (correct runtime edge).
+ *
+ * Bug fix: Previously fs_reclaim_acquire() was called while holding
+ * notifier_lock, teaching lockdep the FALSE edge: notifier_lock ->
fs_reclaim.
+ * This caused false positive circular dependency warnings.
+ */
+ fs_reclaim_acquire(GFP_KERNEL);
+ fs_reclaim_release(GFP_KERNEL);
/* Level 1: Global userq scheduler mutex (outermost) */
mutex_lock(&locks->userq_sch_mutex);
@@ -154,18 +177,16 @@ int amdgpu_lockdep_init(void)
/* Level 6: Reset control lock */
mutex_lock(&locks->reset_lock);
- /*
- * Mark potential memory reclaim boundary.
- * GPU operations might trigger memory allocation/reclaim.
- */
- fs_reclaim_acquire(GFP_KERNEL);
- /* Level 7: SRBM register access */
+ /* Level 7: DRM file list mutex */
+ mutex_lock(&locks->filelist_mutex);
+
+ /* Level 8: SRBM register access */
mutex_lock(&locks->srbm_mutex);
- /* Level 8: GRBM index access */
+ /* Level 9: GRBM index access */
mutex_lock(&locks->grbm_idx_mutex);
- /* Level 9: MMIO index access (innermost lock, spinlock) */
+ /* Level 10: MMIO index access (innermost lock, spinlock) */
spin_lock_irqsave(&locks->mmio_idx_lock, flags);
/*
* All locks acquired in order.
@@ -176,8 +197,8 @@ int amdgpu_lockdep_init(void)
spin_unlock_irqrestore(&locks->mmio_idx_lock, flags);
mutex_unlock(&locks->grbm_idx_mutex);
mutex_unlock(&locks->srbm_mutex);
- fs_reclaim_release(GFP_KERNEL);
+ mutex_unlock(&locks->filelist_mutex);
mutex_unlock(&locks->reset_lock);
up_read(&reset_domain->sem);
@@ -190,7 +211,7 @@ int amdgpu_lockdep_init(void)
amdgpu_reset_put_reset_domain(reset_domain);
kfree(locks);
- pr_info("AMDGPU: Lockdep annotations initialized (9 lock levels)\n");
+ pr_info("AMDGPU: Lockdep annotations initialized (10 lock levels)\n");
return 0;
}
--
2.54.0
