linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: cluster-devel <cluster-devel@redhat.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	<linux-fsdevel@vger.kernel.org>
Subject: [GFS2 PATCH 7/7] GFS2: Prevent deadlock in gfs2_lookup_by_inum
Date: Tue, 24 May 2016 14:12:39 -0500	[thread overview]
Message-ID: <1464117159-17874-8-git-send-email-rpeterso@redhat.com> (raw)
In-Reply-To: <1464117159-17874-1-git-send-email-rpeterso@redhat.com>

From: Andreas Gruenbacher <agruenba@redhat.com>

In gfs2_lookup_by_inum, we must take the glock of a presumed inode
before we can determine if the block indeed still contains an inode,
then look up the inode.  When a lookup finds an inode that is being
freed, it usually waits until that inodes has gone before returning.
However, freeing the inode requires taking the inode glock, so we end up
deadlocking.

Fix that by changing gfs2_inode_lookup: instead of waiting for inodes
that are being freed, return the context necessary for waiting.  Then,
in gfs2_lookup_by_inum, drop the glock before waiting and retrying the
lookup.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/dir.c        |  2 +-
 fs/gfs2/inode.c      | 42 +++++++++++++++++++++++++-----------------
 fs/gfs2/inode.h      |  2 +-
 fs/gfs2/ops_fstype.c |  2 +-
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index d4014af..eb54888 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -1660,7 +1660,7 @@ struct inode *gfs2_dir_search(struct inode *dir, const struct qstr *name,
 		brelse(bh);
 		if (fail_on_exist)
 			return ERR_PTR(-EEXIST);
-		inode = gfs2_inode_lookup(dir->i_sb, dtype, addr, formal_ino, 0);
+		inode = gfs2_inode_lookup(dir->i_sb, dtype, addr, formal_ino, NULL);
 		if (!IS_ERR(inode))
 			GFS2_I(inode)->i_rahead = rahead;
 		return inode;
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index d1dde39..e61ba5b 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -85,30 +85,29 @@ struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr)
 }
 
 static struct inode *gfs2_iget(struct super_block *sb, u64 no_addr,
-			       int non_block)
+			       struct gfs2_freeing_inode *freeing)
 {
-	struct gfs2_freeing_inode freeing;
-	struct gfs2_match match = {
-		.no_addr = no_addr,
-		.freeing = &freeing,
-	};
+	struct gfs2_freeing_inode wait_here;
+	struct gfs2_match match;
 	struct inode *inode;
 
+	if (!freeing)
+		freeing = &wait_here;
+	match.no_addr = no_addr;
+	match.freeing = freeing;
 	while (1) {
-		freeing.wq = NULL;
+		freeing->wq = NULL;
 		inode = find_inode_nowait(sb, no_addr,
 					  gfs2_match_inode, &match);
 		if (inode) {
 			wait_on_inode(inode);
 			return inode;
 		}
-		if (freeing.wq) {
-			if (non_block) {
-				finish_wait(freeing.wq, &freeing.bit_wait.wait);
+		if (freeing->wq) {
+			if (freeing != &wait_here)
 				return ERR_PTR(-EAGAIN);
-			}
 			schedule();
-			finish_wait(freeing.wq, &freeing.bit_wait.wait);
+			finish_wait(freeing->wq, &freeing->bit_wait.wait);
 			continue;
 		}
 
@@ -162,22 +161,23 @@ static void gfs2_set_iop(struct inode *inode)
 /**
  * gfs2_inode_lookup - Lookup an inode
  * @sb: The super block
- * @no_addr: The inode number
  * @type: The type of the inode
- * non_block: Can we block on inodes that are being freed?
+ * @no_addr: The inode number
+ * @freeing: Filled in when inode is being freed
  *
  * Returns: A VFS inode, or an error
  */
 
 struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
-				u64 no_addr, u64 no_formal_ino, int non_block)
+				u64 no_addr, u64 no_formal_ino,
+				struct gfs2_freeing_inode *freeing)
 {
 	struct inode *inode;
 	struct gfs2_inode *ip;
 	struct gfs2_glock *io_gl = NULL;
 	int error;
 
-	inode = gfs2_iget(sb, no_addr, non_block);
+	inode = gfs2_iget(sb, no_addr, freeing);
 	if (IS_ERR(inode))
 		return inode;
 	ip = GFS2_I(inode);
@@ -237,11 +237,13 @@ fail:
 struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr,
 				  u64 *no_formal_ino, unsigned int blktype)
 {
+	struct gfs2_freeing_inode freeing;
 	struct super_block *sb = sdp->sd_vfs;
 	struct gfs2_holder i_gh;
 	struct inode *inode = NULL;
 	int error;
 
+repeat:
 	/* Must not read in block until block type is verified */
 	error = gfs2_glock_nq_num(sdp, no_addr, &gfs2_inode_glops,
 				  LM_ST_EXCLUSIVE, GL_SKIP, &i_gh);
@@ -252,7 +254,13 @@ struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr,
 	if (error)
 		goto fail;
 
-	inode = gfs2_inode_lookup(sb, DT_UNKNOWN, no_addr, 0, 1);
+	inode = gfs2_inode_lookup(sb, DT_UNKNOWN, no_addr, 0, &freeing);
+	if (inode == ERR_PTR(-EAGAIN)) {
+		gfs2_glock_dq_uninit(&i_gh);
+		schedule();
+		finish_wait(freeing.wq, &freeing.bit_wait.wait);
+		goto repeat;
+	}
 	if (IS_ERR(inode))
 		goto fail;
 
diff --git a/fs/gfs2/inode.h b/fs/gfs2/inode.h
index 4863513..f043601 100644
--- a/fs/gfs2/inode.h
+++ b/fs/gfs2/inode.h
@@ -100,7 +100,7 @@ struct gfs2_freeing_inode {
 
 extern struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned type, 
 				       u64 no_addr, u64 no_formal_ino,
-				       int non_block);
+				       struct gfs2_freeing_inode *freeing);
 extern struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr,
 					 u64 *no_formal_ino,
 					 unsigned int blktype);
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 89472c4..8fef52d 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -454,7 +454,7 @@ static int gfs2_lookup_root(struct super_block *sb, struct dentry **dptr,
 	struct dentry *dentry;
 	struct inode *inode;
 
-	inode = gfs2_inode_lookup(sb, DT_DIR, no_addr, 0, 0);
+	inode = gfs2_inode_lookup(sb, DT_DIR, no_addr, 0, NULL);
 	if (IS_ERR(inode)) {
 		fs_err(sdp, "can't read in %s inode: %ld\n", name, PTR_ERR(inode));
 		return PTR_ERR(inode);
-- 
2.5.5


  parent reply	other threads:[~2016-05-24 19:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-24 19:12 [GFS2 PATCH 0/7] Fix glock deadlocks with deleted inodes Bob Peterson
2016-05-24 19:12 ` [GFS2 PATCH 1/7] Revert "GFS2: Eliminate parameter non_block on gfs2_inode_lookup" Bob Peterson
2016-05-24 19:12 ` [GFS2 PATCH 2/7] Revert "GFS2: Don't filter out I_FREEING inodes anymore" Bob Peterson
2016-05-24 19:12 ` [GFS2 PATCH 3/7] GFS2: Remove superfluous assignment Bob Peterson
2016-05-24 19:12 ` [GFS2 PATCH 4/7] GFS2: No need for non-blocking gfs2_ilookup in delete_work_func Bob Peterson
2016-05-24 19:12 ` [GFS2 PATCH 5/7] vfs: Introduce prepare_wait_on_freeing_inode Bob Peterson
2016-05-24 19:12 ` [GFS2 PATCH 6/7] GFS2: Use non-blocking wait in gfs2_iget Bob Peterson
2016-05-24 19:12 ` Bob Peterson [this message]
2016-05-24 21:58 ` [Cluster-devel] [GFS2 PATCH 0/7] Fix glock deadlocks with deleted inodes Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1464117159-17874-8-git-send-email-rpeterso@redhat.com \
    --to=rpeterso@redhat.com \
    --cc=cluster-devel@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).