cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 4/8] gfs2: Always check block type in gfs2_evict_inode
Date: Wed, 31 May 2017 17:03:08 +0200	[thread overview]
Message-ID: <1496242992-1607-5-git-send-email-agruenba@redhat.com> (raw)
In-Reply-To: <1496242992-1607-1-git-send-email-agruenba@redhat.com>

Commit 40ac218f introduces flag GIF_ALLOC_FAILED which indicates that
the inode has likely been allocated in the bitmap already (type
GFS2_BLKST_DINODE), but allocating the inode has eventually failed.
This is used to skip the block type check when destroying inodes in
gfs2_evict_inode.  This is problematic because we do not keep lock
protection between gfs2_create_inode and gfs2_evict_inode.

Fix this by changing the logic in gfs2_create_inode so that the
GIF_FREE_VFS_INODE / GIF_ALLOC_FAILED flags indicate the actual expected
block type, and always check the block type before deleting / freeing an
inode in in gfs2_evict_inode.

(In addition, fix a related typo in a comment of gfs2_set_iop.)

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/inode.c | 17 +++++++++--------
 fs/gfs2/super.c | 11 ++++++-----
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 50108fa..96ebb06 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -109,7 +109,7 @@ static void gfs2_set_iop(struct inode *inode)
  * @no_addr: The inode number
  * @no_formal_ino: The inode generation number
  * @blktype: Requested block type (GFS2_BLKST_DINODE or GFS2_BLKST_UNLINKED;
- *           GFS2_BLKST_FREE do indicate not to verify)
+ *           GFS2_BLKST_FREE to indicate not to verify)
  *
  * If @type is DT_UNKNOWN, the inode type is fetched from disk.
  *
@@ -589,7 +589,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 	struct gfs2_inode *dip = GFS2_I(dir), *ip;
 	struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
 	struct gfs2_glock *io_gl = NULL;
-	int error, free_vfs_inode = 1;
+	int error, fail_bit = GIF_FREE_VFS_INODE;
 	u32 aflags = 0;
 	unsigned blocks = 1;
 	struct gfs2_diradd da = { .bh = NULL, .save_loc = 1, };
@@ -699,6 +699,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 	error = alloc_dinode(ip, aflags, &blocks);
 	if (error)
 		goto fail_free_inode;
+	fail_bit = GIF_ALLOC_FAILED;
 
 	gfs2_set_inode_blocks(inode, blocks);
 
@@ -737,9 +738,9 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 	gfs2_set_iop(inode);
 	insert_inode_hash(inode);
 
-	free_vfs_inode = 0; /* After this point, the inode is no longer
-			       considered free. Any failures need to undo
-			       the gfs2 structures. */
+	fail_bit = 0; /* After this point, the inode is no longer considered
+			 free. Any failures need to undo the gfs2 structures. */
+
 	if (default_acl) {
 		error = __gfs2_set_acl(inode, default_acl, ACL_TYPE_DEFAULT);
 		posix_acl_release(default_acl);
@@ -794,10 +795,10 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 	gfs2_glock_dq_uninit(ghs);
 	if (inode && !IS_ERR(inode)) {
 		clear_nlink(inode);
-		if (!free_vfs_inode)
+		if (fail_bit != GIF_FREE_VFS_INODE)
 			mark_inode_dirty(inode);
-		set_bit(free_vfs_inode ? GIF_FREE_VFS_INODE : GIF_ALLOC_FAILED,
-			&GFS2_I(inode)->i_flags);
+		if (fail_bit)
+			set_bit(fail_bit, &GFS2_I(inode)->i_flags);
 		iput(inode);
 	}
 fail:
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 554bd0c..c651983 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1530,6 +1530,7 @@ static void gfs2_evict_inode(struct inode *inode)
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
 	struct address_space *metamapping;
+	unsigned int blk_type;
 	int error;
 
 	if (test_bit(GIF_FREE_VFS_INODE, &ip->i_flags)) {
@@ -1548,11 +1549,11 @@ static void gfs2_evict_inode(struct inode *inode)
 		goto out;
 	}
 
-	if (!test_bit(GIF_ALLOC_FAILED, &ip->i_flags)) {
-		error = gfs2_check_blk_type(sdp, ip->i_no_addr, GFS2_BLKST_UNLINKED);
-		if (error)
-			goto out_truncate;
-	}
+	blk_type = test_bit(GIF_ALLOC_FAILED, &ip->i_flags) ?
+		GFS2_BLKST_DINODE : GFS2_BLKST_UNLINKED;
+	error = gfs2_check_blk_type(sdp, ip->i_no_addr, blk_type);
+	if (error)
+		goto out_truncate;
 
 	if (test_bit(GIF_INVALID, &ip->i_flags)) {
 		error = gfs2_inode_refresh(ip);
-- 
2.7.4



  parent reply	other threads:[~2017-05-31 15:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-31 15:03 [Cluster-devel] [PATCH 0/8] GFS2 shrinker deadlock Andreas Gruenbacher
2017-05-31 15:03 ` [Cluster-devel] [PATCH 1/8] gfs2: Get rid of flush_delayed_work in gfs2_evict_inode Andreas Gruenbacher
2017-06-01 15:13   ` Steven Whitehouse
2017-05-31 15:03 ` [Cluster-devel] [PATCH 2/8] gfs2: Protect gl->gl_object by spin lock Andreas Gruenbacher
2017-06-01 15:16   ` Steven Whitehouse
2017-05-31 15:03 ` [Cluster-devel] [PATCH 3/8] gfs2: Clean up glock work enqueuing Andreas Gruenbacher
2017-06-01 15:21   ` Steven Whitehouse
2017-06-01 15:43     ` Andreas Gruenbacher
2017-06-01 15:46       ` Steven Whitehouse
2017-06-02  9:19         ` Andreas Gruenbacher
2017-06-02  9:29           ` Steven Whitehouse
2017-05-31 15:03 ` Andreas Gruenbacher [this message]
2017-05-31 15:03 ` [Cluster-devel] [PATCH 5/8] gfs2: gfs2_glock_get: Wait on freeing glocks Andreas Gruenbacher
2017-05-31 15:03 ` [Cluster-devel] [PATCH 6/8] gfs2: Get rid of gfs2_set_nlink Andreas Gruenbacher
2017-05-31 15:03 ` [Cluster-devel] [PATCH 7/8] gfs2: gfs2_evict_inode: Put glocks asynchronously Andreas Gruenbacher
2017-06-01 10:51   ` Andreas Gruenbacher
2017-06-01 15:37   ` Steven Whitehouse
2017-06-02 13:57     ` Andreas Gruenbacher
2017-05-31 15:03 ` [Cluster-devel] [PATCH 8/8] gfs2: Warn when not deleting inodes under memory pressure Andreas Gruenbacher
2017-06-01 15:39   ` Steven Whitehouse

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=1496242992-1607-5-git-send-email-agruenba@redhat.com \
    --to=agruenba@redhat.com \
    /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).