On Tue, Jul 21, 2026 at 5:19 AM <[email protected]> wrote: > > 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 >
v2 looks good, thanks for fixing the name and adding the Fixes tag. No objection from me. One question on the comment above the fs_reclaim_acquire()/fs_reclaim_release() pair, just so the annotation file stays accurate. As written the pair is: fs_reclaim_acquire(GFP_KERNEL); fs_reclaim_release(GFP_KERNEL); mutex_lock(&locks->userq_sch_mutex); ... Since fs_reclaim is released again before any hierarchy lock is taken, is it right that this pair does not actually create an fs_reclaim -> notifier_lock edge? As far as I understand lockdep records an edge only when a lock is acquired while another is held, so with nothing taken between acquire and release the pair only registers the fs_reclaim class. The real fs_reclaim -> notifier_lock edge then comes from runtime, when reclaim invokes the MMU notifier that takes notifier_lock (which your comment also calls the "correct runtime edge"). If that is correct, the pair is functionally equivalent to just dropping the old misplaced acquire, and the comment's "teaches lockdep that fs_reclaim is the OUTERMOST context" slightly overstates what the annotation does. Just wanted to check my understanding. -- Best Regards, Mike Gavrilov.
