| Message ID | 1413782921-22224-1-git-send-email-wen.gang.wang@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
This fix will cause problems for regular file unlinks. If the inode is not cleared on node 2, then both node 1 and node 2 will fail to get try open lock and fail to clear the orphan inodes. I am assuming the deadlock you have seen is because of quota's enabled and fix e3a767b60fd8a9f5e133f42f4970cff77ec43173 should have avoided this deadlock by delaying the dropping of the dquot reference. That didn't happen ? Why else is downconvert task taking the inode lock ? Thanks, --Srini On 10/19/2014 10:28 PM, Wengang Wang wrote: > There is dead lock case like this(the Dentry is a dentry to the Inode): > > node 1 node 2 > ------------------- -------------------------- > > granted inode lock > > requesting dentry lock > responding to node 1 request on dentry lock, > it calls ocfs2_do_drop_dentry_lock with the > call trace: > #4 wait_for_completion > #5 __ocfs2_cluster_lock > #6 ocfs2_inode_lock_full_nested > #7 ocfs2_evict_inode > #8 evict > #9 iput > #10 ocfs2_do_drop_dentry_lock > > You can see node 2 is requesting the inode lock before unlocking dentry lock > thus a two nodes dead lock formed. > > The story is like this: > > Responding node 1's request, Node 2, ocfs2_dentry_convert_worker, returned > UNBLOCK_STOP_POST which means don't downconvert but do post_unlock action. > It was doing so because it thought it doesn't need to downconvert but will do > an unlock instead in the post_unlock action ocfs2_dentry_post_unlock(). > By doing so it can save a clusted downconvert request for performace > consideration. But in ocfs2_dentry_post_unlock() before unlock the dentry lock, > it was requesting the inode lock forming the deadlock expectedly. > > Fix: > unlock dentry lock first before the requesting inode lock in > ocfs2_do_drop_dentry_lock. > > Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com> > --- > fs/ocfs2/dcache.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c > index e2e05a1..19a7d6c 100644 > --- a/fs/ocfs2/dcache.c > +++ b/fs/ocfs2/dcache.c > @@ -369,9 +369,9 @@ out_attach: > static void ocfs2_drop_dentry_lock(struct ocfs2_super *osb, > struct ocfs2_dentry_lock *dl) > { > - iput(dl->dl_inode); > ocfs2_simple_drop_lockres(osb, &dl->dl_lockres); > ocfs2_lock_res_free(&dl->dl_lockres); > + iput(dl->dl_inode); > kfree(dl); > } >
diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c index e2e05a1..19a7d6c 100644 --- a/fs/ocfs2/dcache.c +++ b/fs/ocfs2/dcache.c @@ -369,9 +369,9 @@ out_attach: static void ocfs2_drop_dentry_lock(struct ocfs2_super *osb, struct ocfs2_dentry_lock *dl) { - iput(dl->dl_inode); ocfs2_simple_drop_lockres(osb, &dl->dl_lockres); ocfs2_lock_res_free(&dl->dl_lockres); + iput(dl->dl_inode); kfree(dl); }
There is dead lock case like this(the Dentry is a dentry to the Inode): node 1 node 2 ------------------- -------------------------- granted inode lock requesting dentry lock responding to node 1 request on dentry lock, it calls ocfs2_do_drop_dentry_lock with the call trace: #4 wait_for_completion #5 __ocfs2_cluster_lock #6 ocfs2_inode_lock_full_nested #7 ocfs2_evict_inode #8 evict #9 iput #10 ocfs2_do_drop_dentry_lock You can see node 2 is requesting the inode lock before unlocking dentry lock thus a two nodes dead lock formed. The story is like this: Responding node 1's request, Node 2, ocfs2_dentry_convert_worker, returned UNBLOCK_STOP_POST which means don't downconvert but do post_unlock action. It was doing so because it thought it doesn't need to downconvert but will do an unlock instead in the post_unlock action ocfs2_dentry_post_unlock(). By doing so it can save a clusted downconvert request for performace consideration. But in ocfs2_dentry_post_unlock() before unlock the dentry lock, it was requesting the inode lock forming the deadlock expectedly. Fix: unlock dentry lock first before the requesting inode lock in ocfs2_do_drop_dentry_lock. Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com> --- fs/ocfs2/dcache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)