lkml.org 
[lkml]   [2014]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: fs: lockup on rename_mutex in fs/dcache.c:1035
On Sat, Oct 25, 2014 at 09:39:23PM -0400, Sasha Levin wrote:
> [ 6172.870045] trinity-c55/12752 is trying to acquire lock:
> [ 6172.870045] (rename_lock){+.+...}, at: d_walk (include/linux/spinlock.h:309 fs/dcache.c:1035)
> [ 6172.870045]
> [ 6172.870045] but task is already holding lock:
> [ 6172.870045] (rename_lock){+.+...}, at: d_walk (include/linux/spinlock.h:309 fs/dcache.c:1035)

Umm... So we either have left d_walk() without dropping rename_lock, or
we have called d_walk() from something called from d_walk()? And the
trace would seem to point towards the former...

Ouch. For that to happen, we would need to
* get to rename_retry with retry being true
* after that get D_WALK_NORETRY from enter()
* somehow get to rename_retry *again*

Moreover, we couldn't get there via
if (need_seqretry(&rename_lock, seq)) {
spin_unlock(&this_parent->d_lock);
goto rename_retry;
- seq is 1 by that point, and need_seqretry() returns 0 in that case. IOW,
it must have been this:
/*
* might go back up the wrong parent if we have had a rename
* or deletion
*/
if (this_parent != child->d_parent ||
(child->d_flags & DCACHE_DENTRY_KILLED) ||
need_seqretry(&rename_lock, seq)) {
spin_unlock(&this_parent->d_lock);
rcu_read_unlock();
goto rename_retry;
}
And we had been holding rename_lock in that walk, so d_move() should've
been excluded... Which leaves us with
(child->d_flags & DCACHE_DENTRY_KILLED)
being true... Hrm. AFAICS, it *is* possible to hit that one - just have
the last reference to child dropped between
spin_unlock(&child->d_lock);
and
spin_lock(&this_parent->d_lock);
a few lines above. And yes, if that happens we are in shit - rename_retry
will see retry being false and return, without noticing that on this pass
we had been holding rename_lock. Easily fixed, fortunately - delta below
ought to take care of that...

Comments? AFAICS, it's -stable fodder, the bug going all way back to
at least 3.7...

diff --git a/fs/dcache.c b/fs/dcache.c
index 3ffef7f..65f4aff 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1114,12 +1114,13 @@ resume:

out_unlock:
spin_unlock(&this_parent->d_lock);
+out:
done_seqretry(&rename_lock, seq);
return;

rename_retry:
if (!retry)
- return;
+ goto out;
seq = 1;
goto again;
}

\
 
 \ /
  Last update: 2014-10-26 04:21    [W:0.079 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site