diff mbox

[v2,12/12] btrfs-progs: cmds-check.c: walk_down_tree_v2 break cause of leaf process

Message ID 20170206054735.5227-13-quwenruo@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Qu Wenruo Feb. 6, 2017, 5:47 a.m. UTC
From: Su Yue <suy.fnst@cn.fujitsu.com>

In lowmem mode, 'walk_down_tree_v2' returns negative values wheather
the error is fatal or not. It causes the loop where 'walk_down_tree_v2'
is to break even the error is tolerated and then subsequent nodes process
will be skipped.

Fix it by redefining meanings of values 'walk_down_tree_v2' returns.
Do a similar fix for 'process_one_leaf_v2'.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

Comments

Qu Wenruo Feb. 6, 2017, 8:51 a.m. UTC | #1
At 02/06/2017 01:47 PM, Qu Wenruo wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
>
> In lowmem mode, 'walk_down_tree_v2' returns negative values wheather
> the error is fatal or not. It causes the loop where 'walk_down_tree_v2'
> is to break even the error is tolerated and then subsequent nodes process
> will be skipped.
>
> Fix it by redefining meanings of values 'walk_down_tree_v2' returns.
> Do a similar fix for 'process_one_leaf_v2'.
>
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/cmds-check.c b/cmds-check.c
> index 7d273623..fae18e00 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -1868,6 +1868,11 @@ static int update_nodes_refs(struct btrfs_root *root, u64 bytenr,
>  static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>  			    unsigned int ext_ref);
>
> +/*
> + * Returns >0  Found error, not fatal, should continue process
> + * Returns <0  Fata error, must exit the whole check
> + * returns 0   No error is found
> + */
>  static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
>  			       struct node_refs *nrefs, int *level, int ext_ref)
>  {
> @@ -1937,13 +1942,8 @@ again:
>  	}
>  out:
>  	err &= ~LAST_ITEM;
> -	/*
> -	 * Convert any error bitmap to -EIO, as we should avoid
> -	 * mixing positive and negative return value to represent
> -	 * error
> -	 */
>  	if (err && !ret)
> -		ret = -EIO;
> +		ret = err;
>  	return ret;
>  }
>
> @@ -2213,6 +2213,11 @@ out:
>  static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>  			    unsigned int ext_ref);
>
> +/*
> + * Returns >0  Found error, should continue
> + * Returns 0   No error is found
> + * Returns <0  Fatal error, must exit the whole check
> + */
>  static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
>  			     int *level, struct node_refs *nrefs, int ext_ref)
>  {
> @@ -5028,8 +5033,9 @@ static int check_fs_root_v2(struct btrfs_root *root, unsigned int ext_ref)
>  	struct btrfs_path path;
>  	struct node_refs nrefs;
>  	struct btrfs_root_item *root_item = &root->root_item;
> -	int ret, wret;
> +	int ret;
>  	int level;
> +	int err = 0;
>
>  	/*
>  	 * We need to manually check the first inode item(256)
> @@ -5063,16 +5069,17 @@ static int check_fs_root_v2(struct btrfs_root *root, unsigned int ext_ref)
>  	}
>
>  	while (1) {
> -		wret = walk_down_tree_v2(root, &path, &level, &nrefs, ext_ref);
> -		if (wret < 0)
> -			ret = wret;
> -		if (wret != 0)
> +		ret = walk_down_tree_v2(root, &path, &level, &nrefs, ext_ref);
> +		err |= !!ret;
> +
> +		/* if ret is negative, walk shall stop */
> +		if (ret < 0) {
> +			ret = err;
>  			break;
> +		}
>
> -		wret = walk_up_tree_v2(root, &path, &level);
> -		if (wret < 0)
> -			ret = wret;
> -		if (wret != 0)
> +		ret = walk_up_tree_v2(root, &path, &level);
> +		if (ret != 0)
>  			break;

Here @ret can be 1, to indicate a normal traversal end.

In that case we need to change @ret to 0, as it's a normal end.

Or it will be caught as error.

Updated in github repo.

Thanks,
Qu

>  	}
>
>


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

Patch

diff --git a/cmds-check.c b/cmds-check.c
index 7d273623..fae18e00 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -1868,6 +1868,11 @@  static int update_nodes_refs(struct btrfs_root *root, u64 bytenr,
 static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
 			    unsigned int ext_ref);
 
+/*
+ * Returns >0  Found error, not fatal, should continue process
+ * Returns <0  Fata error, must exit the whole check
+ * returns 0   No error is found
+ */
 static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
 			       struct node_refs *nrefs, int *level, int ext_ref)
 {
@@ -1937,13 +1942,8 @@  again:
 	}
 out:
 	err &= ~LAST_ITEM;
-	/*
-	 * Convert any error bitmap to -EIO, as we should avoid
-	 * mixing positive and negative return value to represent
-	 * error
-	 */
 	if (err && !ret)
-		ret = -EIO;
+		ret = err;
 	return ret;
 }
 
@@ -2213,6 +2213,11 @@  out:
 static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
 			    unsigned int ext_ref);
 
+/*
+ * Returns >0  Found error, should continue
+ * Returns 0   No error is found
+ * Returns <0  Fatal error, must exit the whole check
+ */
 static int walk_down_tree_v2(struct btrfs_root *root, struct btrfs_path *path,
 			     int *level, struct node_refs *nrefs, int ext_ref)
 {
@@ -5028,8 +5033,9 @@  static int check_fs_root_v2(struct btrfs_root *root, unsigned int ext_ref)
 	struct btrfs_path path;
 	struct node_refs nrefs;
 	struct btrfs_root_item *root_item = &root->root_item;
-	int ret, wret;
+	int ret;
 	int level;
+	int err = 0;
 
 	/*
 	 * We need to manually check the first inode item(256)
@@ -5063,16 +5069,17 @@  static int check_fs_root_v2(struct btrfs_root *root, unsigned int ext_ref)
 	}
 
 	while (1) {
-		wret = walk_down_tree_v2(root, &path, &level, &nrefs, ext_ref);
-		if (wret < 0)
-			ret = wret;
-		if (wret != 0)
+		ret = walk_down_tree_v2(root, &path, &level, &nrefs, ext_ref);
+		err |= !!ret;
+
+		/* if ret is negative, walk shall stop */
+		if (ret < 0) {
+			ret = err;
 			break;
+		}
 
-		wret = walk_up_tree_v2(root, &path, &level);
-		if (wret < 0)
-			ret = wret;
-		if (wret != 0)
+		ret = walk_up_tree_v2(root, &path, &level);
+		if (ret != 0)
 			break;
 	}