cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [Cluster-devel] [GFS2] fsck.gfs2: replace recent i_goal fixes with simple logic
@ 2015-04-15 13:37 Abhi Das
  2015-04-15 15:23 ` Andrew Price
  0 siblings, 1 reply; 2+ messages in thread
From: Abhi Das @ 2015-04-15 13:37 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch reverses the recent set of i_goal fixes for fsck.gfs2.
This is because of two problems.
1. It is not possible to determine if a valid block within the fs
is the correct goal block for a given inode.
2. Conversely, given an inode, it is also not possible to accurately
determine what its goal block should be.

The previous patches assumed that the last block of a file is its
goal block, but that is not true if the file is a directory or if
its blocks are not allocated sequentially. fsck.gfs2 would flag
these inodes incorrectly as having bad i_goal values.

This patch takes a simple approach. It checks if the i_goal of a
given inode is out of bounds of the fs. If so, we can be certain
that it is wrong and we set it to the inode metadata block. This
is a safe starting point for gfs2 to determine where to allocate
from next.

Resolves: rhbz#1186515
Signed-off-by: Abhi Das <adas@redhat.com>
---
 gfs2/fsck/metawalk.c   | 92 +++-----------------------------------------------
 gfs2/fsck/metawalk.h   |  5 ---
 gfs2/fsck/pass1.c      | 35 ++++++++++++++++---
 gfs2/libgfs2/libgfs2.h |  1 -
 4 files changed, 34 insertions(+), 99 deletions(-)

diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index f05fb51..4d5a660 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1428,8 +1428,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
  */
 static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
 		      struct gfs2_buffer_head *bh, int head_size,
-		      uint64_t *last_block, uint64_t *blks_checked,
-		      uint64_t *error_blk)
+		      uint64_t *blks_checked, uint64_t *error_blk)
 {
 	int error = 0, rc = 0;
 	uint64_t block, *ptr;
@@ -1444,7 +1443,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
 
 		if (skip_this_pass || fsck_abort)
 			return error;
-		*last_block = block =  be64_to_cpu(*ptr);
+		block =  be64_to_cpu(*ptr);
 		/* It's important that we don't call valid_block() and
 		   bypass calling check_data on invalid blocks because that
 		   would defeat the rangecheck_block related functions in
@@ -1548,15 +1547,12 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 	struct gfs2_buffer_head *bh;
 	uint32_t height = ip->i_di.di_height;
 	int  i, head_size;
-	uint64_t blks_checked = 0, last_block = 0;
+	uint64_t blks_checked = 0;
 	int error, rc;
 	int metadata_clean = 0;
 	uint64_t error_blk = 0;
 	int hit_error_blk = 0;
 
-	if (!height && pass->check_i_goal)
-		pass->check_i_goal(ip, ip->i_di.di_num.no_addr,
-				   pass->private);
 	if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1))
 		return 0;
 
@@ -1575,9 +1571,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 	 * comprise the directory hash table, so we perform the directory
 	 * checks and exit. */
         if (is_dir(&ip->i_di, ip->i_sbd->gfs1)) {
-		last_block = ip->i_di.di_num.no_addr;
-		if (pass->check_i_goal)
-			pass->check_i_goal(ip, last_block, pass->private);
 		if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH))
 			goto out;
 		/* check validity of leaf blocks and leaf chains */
@@ -1604,7 +1597,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 
 		if (pass->check_data)
 			error = check_data(ip, pass, bh, head_size,
-					   &last_block, &blks_checked, &error_blk);
+					   &blks_checked, &error_blk);
 		if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS)
 			pass->big_file_msg(ip, blks_checked);
 	}
@@ -1616,8 +1609,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 			    (unsigned long long)ip->i_di.di_num.no_addr);
 		fflush(stdout);
 	}
-	if (!error && pass->check_i_goal)
-		pass->check_i_goal(ip, last_block, pass->private);
 undo_metalist:
 	if (!error)
 		goto out;
@@ -1958,80 +1949,6 @@ static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
 	return 0;
 }
 
-/**
- * rgrp_contains_block - Check if the rgrp provided contains the
- * given block. Taken directly from the gfs2 kernel code
- * @rgd: The rgrp to search within
- * @block: The block to search for
- *
- * Returns: 1 if present, 0 if not.
- */
-static inline int rgrp_contains_block(struct rgrp_tree *rgd, uint64_t block)
-{
-	uint64_t first = rgd->ri.ri_data0;
-	uint64_t last = first + rgd->ri.ri_data;
-	return first <= block && block < last;
-}
-
-/**
- * check_i_goal
- * @ip
- * @goal_blk: What the goal block should be for this inode
- *
- * The goal block for a regular file is typically the last
- * data block of the file. If we can't get the right value,
- * the inode metadata block is the next best thing.
- *
- * Returns: 0 if corrected, 1 if not corrected
- */
-int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
-			void *private)
-{
-	struct gfs2_sbd *sdp = ip->i_sbd;
-	uint64_t i_block = ip->i_di.di_num.no_addr;
-
-	/* Don't fix gfs1 inodes, system inodes or inodes whose goal blocks are
-	 * set to the inode blocks themselves. */
-	if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM ||
-		ip->i_di.di_goal_meta == i_block)
-		return 0;
-	/* Don't fix directory goal blocks unless we know they're wrong.
-	 * i.e. out of bounds of the fs. Directories can easily have blocks
-	 * outside of the dinode's rgrp and thus we have no way of knowing
-	 * if the goal block is bogus or not. */
-	if (is_dir(&ip->i_di, ip->i_sbd->gfs1) &&
-	    (ip->i_di.di_goal_meta > sdp->sb_addr &&
-	     ip->i_di.di_goal_meta <= sdp->fssize))
-		return 0;
-	/* We default to the inode block */
-	if (!goal_blk)
-		goal_blk = i_block;
-
-	if (ip->i_di.di_goal_meta != goal_blk) {
-		/* If the existing goal block is in the same rgrp as the inode,
-		 * we give the benefit of doubt and assume the value is correct */
-		if (ip->i_rgd &&
-		    rgrp_contains_block(ip->i_rgd, ip->i_di.di_goal_meta))
-			goto skip;
-		log_err( _("Error: inode %llu (0x%llx) has invalid "
-			   "allocation goal block %llu (0x%llx). Should"
-			   " be %llu (0x%llx)\n"),
-			 (unsigned long long)i_block, (unsigned long long)i_block,
-			 (unsigned long long)ip->i_di.di_goal_meta,
-			 (unsigned long long)ip->i_di.di_goal_meta,
-			 (unsigned long long)goal_blk, (unsigned long long)goal_blk);
-		if (query( _("Fix the invalid goal block? (y/n) "))) {
-			ip->i_di.di_goal_meta = ip->i_di.di_goal_data = goal_blk;
-			bmodified(ip->i_bh);
-		} else {
-			log_err(_("Invalid goal block not fixed.\n"));
-			return 1;
-		}
-	}
-skip:
-	return 0;
-}
-
 struct metawalk_fxns alloc_fxns = {
 	.private = NULL,
 	.check_leaf = alloc_leaf,
@@ -2042,7 +1959,6 @@ struct metawalk_fxns alloc_fxns = {
 	.check_dentry = NULL,
 	.check_eattr_entry = NULL,
 	.check_eattr_extentry = NULL,
-	.check_i_goal = check_i_goal,
 	.finish_eattr_indir = NULL,
 };
 
diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
index 779360e..fa4c850 100644
--- a/gfs2/fsck/metawalk.h
+++ b/gfs2/fsck/metawalk.h
@@ -50,8 +50,6 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock,
 			      const char *caller, int line);
 extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
 			      int error_on_dinode, int new_blockmap_state);
-extern int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
-			void *private);
 extern void reprocess_inode(struct gfs2_inode *ip, const char *desc);
 extern struct duptree *dupfind(uint64_t block);
 extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp,
@@ -91,7 +89,6 @@ enum meta_check_rc {
  * check_dentry:
  * check_eattr_entry:
  * check_eattr_extentry:
- * check_i_goal:
  */
 struct metawalk_fxns {
 	void *private;
@@ -143,8 +140,6 @@ struct metawalk_fxns {
 				     struct gfs2_ea_header *ea_hdr,
 				     struct gfs2_ea_header *ea_hdr_prev,
 				     void *private);
-	int (*check_i_goal) (struct gfs2_inode *ip, uint64_t goal_blk,
-			     void *private);
 	int (*finish_eattr_indir) (struct gfs2_inode *ip, int leaf_pointers,
 				   int leaf_pointer_errors, void *private);
 	void (*big_file_msg) (struct gfs2_inode *ip, uint64_t blks_checked);
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 69c88f4..0909873 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -100,7 +100,6 @@ struct metawalk_fxns pass1_fxns = {
 	.check_dentry = NULL,
 	.check_eattr_entry = check_eattr_entries,
 	.check_eattr_extentry = check_extended_leaf_eattr,
-	.check_i_goal = check_i_goal,
 	.finish_eattr_indir = finish_eattr_indir,
 	.big_file_msg = big_file_comfort,
 	.repair_leaf = pass1_repair_leaf,
@@ -1205,12 +1204,37 @@ bad_dinode:
 	return -1;
 }
 
+static void check_i_goal(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
+{
+	if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM)
+		return;
+
+	if (ip->i_di.di_goal_meta <= sdp->sb_addr ||
+	    ip->i_di.di_goal_meta > sdp->fssize) {
+		log_err(_("Inode #%llu (0x%llx): Bad allocation goal block "
+			  "found: %llu (0x%llx)\n"),
+			(unsigned long long)ip->i_di.di_num.no_addr,
+			(unsigned long long)ip->i_di.di_num.no_addr,
+			(unsigned long long)ip->i_di.di_goal_meta,
+			(unsigned long long)ip->i_di.di_goal_meta);
+		if (query( _("Fix goal block in inode #%llu (0x%llx)? (y/n) "),
+			   (unsigned long long)ip->i_di.di_num.no_addr,
+			   (unsigned long long)ip->i_di.di_num.no_addr)) {
+			ip->i_di.di_goal_meta = ip->i_di.di_num.no_addr;
+			bmodified(ip->i_bh);
+		} else
+			log_err(_("Allocation goal block in inode #%lld "
+				  "(0x%llx) not fixed\n"),
+				(unsigned long long)ip->i_di.di_num.no_addr,
+				(unsigned long long)ip->i_di.di_num.no_addr);
+	}
+}
+
 /*
  * handle_di - This is now a wrapper function that takes a gfs2_buffer_head
  *             and calls handle_ip, which takes an in-code dinode structure.
  */
-static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
-		     struct rgrp_tree *rgd)
+static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
 {
 	int error = 0;
 	uint64_t block = bh->b_blocknr;
@@ -1252,7 +1276,7 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
 				 (unsigned long long)block,
 				 (unsigned long long)block);
 	}
-	ip->i_rgd = rgd;
+	check_i_goal(sdp, ip);
 	error = handle_ip(sdp, ip);
 	fsck_inode_put(&ip);
 	return error;
@@ -1378,6 +1402,7 @@ static int check_system_inode(struct gfs2_sbd *sdp,
 					  "directory entries.\n"), filename);
 		}
 	}
+	check_i_goal(sdp, *sysinode);
 	error = handle_ip(sdp, *sysinode);
 	return error ? error : err;
 }
@@ -1602,7 +1627,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
 				 (unsigned long long)block,
 				 (unsigned long long)block);
 			check_n_fix_bitmap(sdp, block, 0, GFS2_BLKST_FREE);
-		} else if (handle_di(sdp, bh, rgd) < 0) {
+		} else if (handle_di(sdp, bh) < 0) {
 			stack;
 			brelse(bh);
 			gfs2_special_free(&gfs1_rindex_blks);
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index f1f81d3..ccae721 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -233,7 +233,6 @@ struct gfs2_inode {
 	struct gfs2_dinode i_di;
 	struct gfs2_buffer_head *i_bh;
 	struct gfs2_sbd *i_sbd;
-	struct rgrp_tree *i_rgd; /* The rgrp this inode is in */
 };
 
 struct master_dir
-- 
1.8.1.4



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Cluster-devel] [GFS2] fsck.gfs2: replace recent i_goal fixes with simple logic
  2015-04-15 13:37 [Cluster-devel] [GFS2] fsck.gfs2: replace recent i_goal fixes with simple logic Abhi Das
@ 2015-04-15 15:23 ` Andrew Price
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Price @ 2015-04-15 15:23 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi Abhi,

On 15/04/15 14:37, Abhi Das wrote:
> This patch reverses the recent set of i_goal fixes for fsck.gfs2.
> This is because of two problems.
> 1. It is not possible to determine if a valid block within the fs
> is the correct goal block for a given inode.
> 2. Conversely, given an inode, it is also not possible to accurately
> determine what its goal block should be.
>
> The previous patches assumed that the last block of a file is its
> goal block, but that is not true if the file is a directory or if
> its blocks are not allocated sequentially. fsck.gfs2 would flag
> these inodes incorrectly as having bad i_goal values.
>
> This patch takes a simple approach. It checks if the i_goal of a
> given inode is out of bounds of the fs. If so, we can be certain
> that it is wrong and we set it to the inode metadata block. This
> is a safe starting point for gfs2 to determine where to allocate
> from next.
>
> Resolves: rhbz#1186515
> Signed-off-by: Abhi Das <adas@redhat.com>

This looks good to me, and it fixes the test failures that I was seeing 
due to the non-sequential last block issue.

Thanks,
Andy

> ---
>   gfs2/fsck/metawalk.c   | 92 +++-----------------------------------------------
>   gfs2/fsck/metawalk.h   |  5 ---
>   gfs2/fsck/pass1.c      | 35 ++++++++++++++++---
>   gfs2/libgfs2/libgfs2.h |  1 -
>   4 files changed, 34 insertions(+), 99 deletions(-)
>
> diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
> index f05fb51..4d5a660 100644
> --- a/gfs2/fsck/metawalk.c
> +++ b/gfs2/fsck/metawalk.c
> @@ -1428,8 +1428,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
>    */
>   static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
>   		      struct gfs2_buffer_head *bh, int head_size,
> -		      uint64_t *last_block, uint64_t *blks_checked,
> -		      uint64_t *error_blk)
> +		      uint64_t *blks_checked, uint64_t *error_blk)
>   {
>   	int error = 0, rc = 0;
>   	uint64_t block, *ptr;
> @@ -1444,7 +1443,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
>
>   		if (skip_this_pass || fsck_abort)
>   			return error;
> -		*last_block = block =  be64_to_cpu(*ptr);
> +		block =  be64_to_cpu(*ptr);
>   		/* It's important that we don't call valid_block() and
>   		   bypass calling check_data on invalid blocks because that
>   		   would defeat the rangecheck_block related functions in
> @@ -1548,15 +1547,12 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
>   	struct gfs2_buffer_head *bh;
>   	uint32_t height = ip->i_di.di_height;
>   	int  i, head_size;
> -	uint64_t blks_checked = 0, last_block = 0;
> +	uint64_t blks_checked = 0;
>   	int error, rc;
>   	int metadata_clean = 0;
>   	uint64_t error_blk = 0;
>   	int hit_error_blk = 0;
>
> -	if (!height && pass->check_i_goal)
> -		pass->check_i_goal(ip, ip->i_di.di_num.no_addr,
> -				   pass->private);
>   	if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1))
>   		return 0;
>
> @@ -1575,9 +1571,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
>   	 * comprise the directory hash table, so we perform the directory
>   	 * checks and exit. */
>           if (is_dir(&ip->i_di, ip->i_sbd->gfs1)) {
> -		last_block = ip->i_di.di_num.no_addr;
> -		if (pass->check_i_goal)
> -			pass->check_i_goal(ip, last_block, pass->private);
>   		if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH))
>   			goto out;
>   		/* check validity of leaf blocks and leaf chains */
> @@ -1604,7 +1597,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
>
>   		if (pass->check_data)
>   			error = check_data(ip, pass, bh, head_size,
> -					   &last_block, &blks_checked, &error_blk);
> +					   &blks_checked, &error_blk);
>   		if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS)
>   			pass->big_file_msg(ip, blks_checked);
>   	}
> @@ -1616,8 +1609,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
>   			    (unsigned long long)ip->i_di.di_num.no_addr);
>   		fflush(stdout);
>   	}
> -	if (!error && pass->check_i_goal)
> -		pass->check_i_goal(ip, last_block, pass->private);
>   undo_metalist:
>   	if (!error)
>   		goto out;
> @@ -1958,80 +1949,6 @@ static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
>   	return 0;
>   }
>
> -/**
> - * rgrp_contains_block - Check if the rgrp provided contains the
> - * given block. Taken directly from the gfs2 kernel code
> - * @rgd: The rgrp to search within
> - * @block: The block to search for
> - *
> - * Returns: 1 if present, 0 if not.
> - */
> -static inline int rgrp_contains_block(struct rgrp_tree *rgd, uint64_t block)
> -{
> -	uint64_t first = rgd->ri.ri_data0;
> -	uint64_t last = first + rgd->ri.ri_data;
> -	return first <= block && block < last;
> -}
> -
> -/**
> - * check_i_goal
> - * @ip
> - * @goal_blk: What the goal block should be for this inode
> - *
> - * The goal block for a regular file is typically the last
> - * data block of the file. If we can't get the right value,
> - * the inode metadata block is the next best thing.
> - *
> - * Returns: 0 if corrected, 1 if not corrected
> - */
> -int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
> -			void *private)
> -{
> -	struct gfs2_sbd *sdp = ip->i_sbd;
> -	uint64_t i_block = ip->i_di.di_num.no_addr;
> -
> -	/* Don't fix gfs1 inodes, system inodes or inodes whose goal blocks are
> -	 * set to the inode blocks themselves. */
> -	if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM ||
> -		ip->i_di.di_goal_meta == i_block)
> -		return 0;
> -	/* Don't fix directory goal blocks unless we know they're wrong.
> -	 * i.e. out of bounds of the fs. Directories can easily have blocks
> -	 * outside of the dinode's rgrp and thus we have no way of knowing
> -	 * if the goal block is bogus or not. */
> -	if (is_dir(&ip->i_di, ip->i_sbd->gfs1) &&
> -	    (ip->i_di.di_goal_meta > sdp->sb_addr &&
> -	     ip->i_di.di_goal_meta <= sdp->fssize))
> -		return 0;
> -	/* We default to the inode block */
> -	if (!goal_blk)
> -		goal_blk = i_block;
> -
> -	if (ip->i_di.di_goal_meta != goal_blk) {
> -		/* If the existing goal block is in the same rgrp as the inode,
> -		 * we give the benefit of doubt and assume the value is correct */
> -		if (ip->i_rgd &&
> -		    rgrp_contains_block(ip->i_rgd, ip->i_di.di_goal_meta))
> -			goto skip;
> -		log_err( _("Error: inode %llu (0x%llx) has invalid "
> -			   "allocation goal block %llu (0x%llx). Should"
> -			   " be %llu (0x%llx)\n"),
> -			 (unsigned long long)i_block, (unsigned long long)i_block,
> -			 (unsigned long long)ip->i_di.di_goal_meta,
> -			 (unsigned long long)ip->i_di.di_goal_meta,
> -			 (unsigned long long)goal_blk, (unsigned long long)goal_blk);
> -		if (query( _("Fix the invalid goal block? (y/n) "))) {
> -			ip->i_di.di_goal_meta = ip->i_di.di_goal_data = goal_blk;
> -			bmodified(ip->i_bh);
> -		} else {
> -			log_err(_("Invalid goal block not fixed.\n"));
> -			return 1;
> -		}
> -	}
> -skip:
> -	return 0;
> -}
> -
>   struct metawalk_fxns alloc_fxns = {
>   	.private = NULL,
>   	.check_leaf = alloc_leaf,
> @@ -2042,7 +1959,6 @@ struct metawalk_fxns alloc_fxns = {
>   	.check_dentry = NULL,
>   	.check_eattr_entry = NULL,
>   	.check_eattr_extentry = NULL,
> -	.check_i_goal = check_i_goal,
>   	.finish_eattr_indir = NULL,
>   };
>
> diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
> index 779360e..fa4c850 100644
> --- a/gfs2/fsck/metawalk.h
> +++ b/gfs2/fsck/metawalk.h
> @@ -50,8 +50,6 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock,
>   			      const char *caller, int line);
>   extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
>   			      int error_on_dinode, int new_blockmap_state);
> -extern int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
> -			void *private);
>   extern void reprocess_inode(struct gfs2_inode *ip, const char *desc);
>   extern struct duptree *dupfind(uint64_t block);
>   extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp,
> @@ -91,7 +89,6 @@ enum meta_check_rc {
>    * check_dentry:
>    * check_eattr_entry:
>    * check_eattr_extentry:
> - * check_i_goal:
>    */
>   struct metawalk_fxns {
>   	void *private;
> @@ -143,8 +140,6 @@ struct metawalk_fxns {
>   				     struct gfs2_ea_header *ea_hdr,
>   				     struct gfs2_ea_header *ea_hdr_prev,
>   				     void *private);
> -	int (*check_i_goal) (struct gfs2_inode *ip, uint64_t goal_blk,
> -			     void *private);
>   	int (*finish_eattr_indir) (struct gfs2_inode *ip, int leaf_pointers,
>   				   int leaf_pointer_errors, void *private);
>   	void (*big_file_msg) (struct gfs2_inode *ip, uint64_t blks_checked);
> diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
> index 69c88f4..0909873 100644
> --- a/gfs2/fsck/pass1.c
> +++ b/gfs2/fsck/pass1.c
> @@ -100,7 +100,6 @@ struct metawalk_fxns pass1_fxns = {
>   	.check_dentry = NULL,
>   	.check_eattr_entry = check_eattr_entries,
>   	.check_eattr_extentry = check_extended_leaf_eattr,
> -	.check_i_goal = check_i_goal,
>   	.finish_eattr_indir = finish_eattr_indir,
>   	.big_file_msg = big_file_comfort,
>   	.repair_leaf = pass1_repair_leaf,
> @@ -1205,12 +1204,37 @@ bad_dinode:
>   	return -1;
>   }
>
> +static void check_i_goal(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
> +{
> +	if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM)
> +		return;
> +
> +	if (ip->i_di.di_goal_meta <= sdp->sb_addr ||
> +	    ip->i_di.di_goal_meta > sdp->fssize) {
> +		log_err(_("Inode #%llu (0x%llx): Bad allocation goal block "
> +			  "found: %llu (0x%llx)\n"),
> +			(unsigned long long)ip->i_di.di_num.no_addr,
> +			(unsigned long long)ip->i_di.di_num.no_addr,
> +			(unsigned long long)ip->i_di.di_goal_meta,
> +			(unsigned long long)ip->i_di.di_goal_meta);
> +		if (query( _("Fix goal block in inode #%llu (0x%llx)? (y/n) "),
> +			   (unsigned long long)ip->i_di.di_num.no_addr,
> +			   (unsigned long long)ip->i_di.di_num.no_addr)) {
> +			ip->i_di.di_goal_meta = ip->i_di.di_num.no_addr;
> +			bmodified(ip->i_bh);
> +		} else
> +			log_err(_("Allocation goal block in inode #%lld "
> +				  "(0x%llx) not fixed\n"),
> +				(unsigned long long)ip->i_di.di_num.no_addr,
> +				(unsigned long long)ip->i_di.di_num.no_addr);
> +	}
> +}
> +
>   /*
>    * handle_di - This is now a wrapper function that takes a gfs2_buffer_head
>    *             and calls handle_ip, which takes an in-code dinode structure.
>    */
> -static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
> -		     struct rgrp_tree *rgd)
> +static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
>   {
>   	int error = 0;
>   	uint64_t block = bh->b_blocknr;
> @@ -1252,7 +1276,7 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
>   				 (unsigned long long)block,
>   				 (unsigned long long)block);
>   	}
> -	ip->i_rgd = rgd;
> +	check_i_goal(sdp, ip);
>   	error = handle_ip(sdp, ip);
>   	fsck_inode_put(&ip);
>   	return error;
> @@ -1378,6 +1402,7 @@ static int check_system_inode(struct gfs2_sbd *sdp,
>   					  "directory entries.\n"), filename);
>   		}
>   	}
> +	check_i_goal(sdp, *sysinode);
>   	error = handle_ip(sdp, *sysinode);
>   	return error ? error : err;
>   }
> @@ -1602,7 +1627,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
>   				 (unsigned long long)block,
>   				 (unsigned long long)block);
>   			check_n_fix_bitmap(sdp, block, 0, GFS2_BLKST_FREE);
> -		} else if (handle_di(sdp, bh, rgd) < 0) {
> +		} else if (handle_di(sdp, bh) < 0) {
>   			stack;
>   			brelse(bh);
>   			gfs2_special_free(&gfs1_rindex_blks);
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index f1f81d3..ccae721 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -233,7 +233,6 @@ struct gfs2_inode {
>   	struct gfs2_dinode i_di;
>   	struct gfs2_buffer_head *i_bh;
>   	struct gfs2_sbd *i_sbd;
> -	struct rgrp_tree *i_rgd; /* The rgrp this inode is in */
>   };
>
>   struct master_dir
>



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-04-15 15:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-15 13:37 [Cluster-devel] [GFS2] fsck.gfs2: replace recent i_goal fixes with simple logic Abhi Das
2015-04-15 15:23 ` Andrew Price

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).