From: Vitaly Prosyak <[email protected]>
Move fs_reclaim_acquire() to before all lock acquisitions to eliminate
false positive circular locking dependency warning.
This is a 7.2-cycle regression fix suitable for stable backport.
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.
The acquire/release pair does not create a static lockdep edge by itself
(no locks are held between acquire and release), but it registers the
fs_reclaim lock class. The actual fs_reclaim -> notifier_lock ordering
is established at runtime when memory reclaim invokes MMU notifiers that
take notifier_lock. By moving the fs_reclaim registration before any
lock acquisition, we avoid teaching lockdep the false reverse dependency.
VERIFICATION (AFTER FIX):
Test reproducer (based on Mikhail 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 is 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.
v2: Address Mikhail Gavrilov's review feedback:
- Fix author name: Michael -> Mikhail Gavrilov in all trailers
- Add Fixes: tag to link regression to original commit
- Add Tested-by: Mikhail Gavrilov (tested on RX 7900 XTX)
- Rewrite fs_reclaim comment to be technically accurate about what
fs_reclaim_acquire/release actually does (registers lock class,
doesn't create static edge)
- Add note that this is a 7.2-cycle regression for stable routing
Fixes: 1d0f5838b126 ("drm/amdgpu: Add lockdep annotations for lock ordering
validation")
Reported-by: Mikhail Gavrilov <[email protected]>
Analyzed-by: Mikhail Gavrilov <[email protected]>
Test-case-by: Mikhail Gavrilov <[email protected]>
Tested-by: Mikhail 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]>
Cc: [email protected]
Signed-off-by: Vitaly Prosyak <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
index b251350b1fb2..718411f724f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_lockdep.c
@@ -150,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);
@@ -168,11 +180,6 @@ int amdgpu_lockdep_init(void)
/* Level 7: DRM file list mutex */
mutex_lock(&locks->filelist_mutex);
- /*
- * Mark potential memory reclaim boundary.
- * GPU operations might trigger memory allocation/reclaim.
- */
- fs_reclaim_acquire(GFP_KERNEL);
/* Level 8: SRBM register access */
mutex_lock(&locks->srbm_mutex);
@@ -190,7 +197,6 @@ 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);
--
2.54.0
