diff mbox series

[1/3] btrfs: remove the unnecessary path parameter for scrub_raid56_parity()

Message ID 20211213131054.102526-2-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: use btrfs_path::reada to replace the | expand

Commit Message

Qu Wenruo Dec. 13, 2021, 1:10 p.m. UTC 
In function scrub_stripe() we allocated two btrfs_path, one @path for
extent tree search and another @ppath for full stripe extent tree search
for RAID56.

This is totally uncessary, as the @ppath usage is completely inside
scrub_raid56_parity(), thus we can move the path allocation into
scrub_raid56_parity() completely.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/scrub.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Comments

Qu Wenruo Dec. 14, 2021, 12:44 p.m. UTC | #1 
On 2021/12/13 21:10, Qu Wenruo wrote:
> In function scrub_stripe() we allocated two btrfs_path, one @path for
> extent tree search and another @ppath for full stripe extent tree search
> for RAID56.
> 
> This is totally uncessary, as the @ppath usage is completely inside
> scrub_raid56_parity(), thus we can move the path allocation into
> scrub_raid56_parity() completely.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   fs/btrfs/scrub.c | 28 ++++++++++++++--------------
>   1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 15a123e67108..a3bd3dfd5e8b 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2892,7 +2892,6 @@ static void scrub_parity_put(struct scrub_parity *sparity)
>   static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>   						  struct map_lookup *map,
>   						  struct btrfs_device *sdev,
> -						  struct btrfs_path *path,
>   						  u64 logic_start,
>   						  u64 logic_end)
>   {
> @@ -2901,6 +2900,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>   	struct btrfs_root *csum_root;
>   	struct btrfs_extent_item *extent;
>   	struct btrfs_io_context *bioc = NULL;
> +	struct btrfs_path *path;
>   	u64 flags;
>   	int ret;
>   	int slot;
> @@ -2919,6 +2919,14 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>   	int extent_mirror_num;
>   	int stop_loop = 0;
>   
> +	path = btrfs_alloc_path();
> +	if (!path) {
> +		spin_lock(&sctx->stat_lock);
> +		sctx->stat.malloc_errors++;
> +		spin_unlock(&sctx->stat_lock);
> +		return -ENOMEM;
> +	}
> +
>   	ASSERT(map->stripe_len <= U32_MAX);
>   	nsectors = map->stripe_len >> fs_info->sectorsize_bits;
>   	bitmap_len = scrub_calc_parity_bitmap_len(nsectors);
> @@ -2928,6 +2936,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>   		spin_lock(&sctx->stat_lock);
>   		sctx->stat.malloc_errors++;
>   		spin_unlock(&sctx->stat_lock);
> +		btrfs_free_path(path);
>   		return -ENOMEM;
>   	}
>   
> @@ -3117,7 +3126,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
>   	scrub_wr_submit(sctx);
>   	mutex_unlock(&sctx->wr_lock);
>   
> -	btrfs_release_path(path);
> +	btrfs_free_path(path);
>   	return ret < 0 ? ret : 0;
>   }
>   
> @@ -3167,7 +3176,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   					   int num, u64 base, u64 length,
>   					   struct btrfs_block_group *cache)
>   {
> -	struct btrfs_path *path, *ppath;
> +	struct btrfs_path *path;
>   	struct btrfs_fs_info *fs_info = sctx->fs_info;
>   	struct btrfs_root *root;
>   	struct btrfs_root *csum_root;
> @@ -3229,12 +3238,6 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   	if (!path)
>   		return -ENOMEM;
>   
> -	ppath = btrfs_alloc_path();
> -	if (!ppath) {
> -		btrfs_free_path(path);
> -		return -ENOMEM;
> -	}
> -
>   	/*
>   	 * work on commit root. The related disk blocks are static as
>   	 * long as COW is applied. This means, it is save to rewrite
> @@ -3243,8 +3246,6 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   	path->search_commit_root = 1;
>   	path->skip_locking = 1;
>   
> -	ppath->search_commit_root = 1;
> -	ppath->skip_locking = 1;

The new path in the raid56 functions doesn't have this.

This is causing false alerts in raid56 dev-replace test cases, like 
btrfs/072.
Which reports csum mismatch for metadata.

Would fix it in next update.

Thanks,
Qu

>   	/*
>   	 * trigger the readahead for extent tree csum tree and wait for
>   	 * completion. During readahead, the scrub is officially paused
> @@ -3347,7 +3348,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   				stripe_logical += base;
>   				stripe_end = stripe_logical + increment;
>   				ret = scrub_raid56_parity(sctx, map, scrub_dev,
> -							  ppath, stripe_logical,
> +							  stripe_logical,
>   							  stripe_end);
>   				if (ret)
>   					goto out;
> @@ -3518,7 +3519,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   						stripe_end = stripe_logical +
>   								increment;
>   						ret = scrub_raid56_parity(sctx,
> -							map, scrub_dev, ppath,
> +							map, scrub_dev,
>   							stripe_logical,
>   							stripe_end);
>   						if (ret)
> @@ -3565,7 +3566,6 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   
>   	blk_finish_plug(&plug);
>   	btrfs_free_path(path);
> -	btrfs_free_path(ppath);
>   
>   	if (sctx->is_dev_replace && ret >= 0) {
>   		int ret2;
diff mbox series

Patch

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 15a123e67108..a3bd3dfd5e8b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2892,7 +2892,6 @@  static void scrub_parity_put(struct scrub_parity *sparity)
 static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
 						  struct map_lookup *map,
 						  struct btrfs_device *sdev,
-						  struct btrfs_path *path,
 						  u64 logic_start,
 						  u64 logic_end)
 {
@@ -2901,6 +2900,7 @@  static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
 	struct btrfs_root *csum_root;
 	struct btrfs_extent_item *extent;
 	struct btrfs_io_context *bioc = NULL;
+	struct btrfs_path *path;
 	u64 flags;
 	int ret;
 	int slot;
@@ -2919,6 +2919,14 @@  static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
 	int extent_mirror_num;
 	int stop_loop = 0;
 
+	path = btrfs_alloc_path();
+	if (!path) {
+		spin_lock(&sctx->stat_lock);
+		sctx->stat.malloc_errors++;
+		spin_unlock(&sctx->stat_lock);
+		return -ENOMEM;
+	}
+
 	ASSERT(map->stripe_len <= U32_MAX);
 	nsectors = map->stripe_len >> fs_info->sectorsize_bits;
 	bitmap_len = scrub_calc_parity_bitmap_len(nsectors);
@@ -2928,6 +2936,7 @@  static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
 		spin_lock(&sctx->stat_lock);
 		sctx->stat.malloc_errors++;
 		spin_unlock(&sctx->stat_lock);
+		btrfs_free_path(path);
 		return -ENOMEM;
 	}
 
@@ -3117,7 +3126,7 @@  static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx,
 	scrub_wr_submit(sctx);
 	mutex_unlock(&sctx->wr_lock);
 
-	btrfs_release_path(path);
+	btrfs_free_path(path);
 	return ret < 0 ? ret : 0;
 }
 
@@ -3167,7 +3176,7 @@  static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 					   int num, u64 base, u64 length,
 					   struct btrfs_block_group *cache)
 {
-	struct btrfs_path *path, *ppath;
+	struct btrfs_path *path;
 	struct btrfs_fs_info *fs_info = sctx->fs_info;
 	struct btrfs_root *root;
 	struct btrfs_root *csum_root;
@@ -3229,12 +3238,6 @@  static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	if (!path)
 		return -ENOMEM;
 
-	ppath = btrfs_alloc_path();
-	if (!ppath) {
-		btrfs_free_path(path);
-		return -ENOMEM;
-	}
-
 	/*
 	 * work on commit root. The related disk blocks are static as
 	 * long as COW is applied. This means, it is save to rewrite
@@ -3243,8 +3246,6 @@  static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	path->search_commit_root = 1;
 	path->skip_locking = 1;
 
-	ppath->search_commit_root = 1;
-	ppath->skip_locking = 1;
 	/*
 	 * trigger the readahead for extent tree csum tree and wait for
 	 * completion. During readahead, the scrub is officially paused
@@ -3347,7 +3348,7 @@  static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 				stripe_logical += base;
 				stripe_end = stripe_logical + increment;
 				ret = scrub_raid56_parity(sctx, map, scrub_dev,
-							  ppath, stripe_logical,
+							  stripe_logical,
 							  stripe_end);
 				if (ret)
 					goto out;
@@ -3518,7 +3519,7 @@  static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 						stripe_end = stripe_logical +
 								increment;
 						ret = scrub_raid56_parity(sctx,
-							map, scrub_dev, ppath,
+							map, scrub_dev,
 							stripe_logical,
 							stripe_end);
 						if (ret)
@@ -3565,7 +3566,6 @@  static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 
 	blk_finish_plug(&plug);
 	btrfs_free_path(path);
-	btrfs_free_path(ppath);
 
 	if (sctx->is_dev_replace && ret >= 0) {
 		int ret2;