Thread (7 messages) 7 messages, 3 authors, 2021-12-14

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

From: Qu Wenruo <hidden>
Date: 2021-12-14 12:44:32


On 2021/12/13 21:10, Qu Wenruo wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
  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
quoted hunk ↗ jump to hunk
  	/*
  	 * 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;
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help