[GFS2 PATCH 7/7] GFS2: Prevent deadlock in gfs2_lookup_by_inum

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Andreas Gruenbacher <agruenba@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux