Commit fab0c0f0 authored by Qu Wenruo's avatar Qu Wenruo Committed by David Sterba
Browse files

btrfs: introduce BTRFS_PATH_AUTO_RELEASE() helper 



There are already several bugs with on-stack btrfs_path involved, even
it is already a little safer than btrfs_path pointers (only leaks the
extent buffers, not the btrfs_path structure itself)

- Patch "btrfs: make sure extent and csum paths are always released in
  scrub_raid56_parity_stripe()"

- Patch "btrfs: fix a potential path leak in print_data_reloc_error()"

Thus there is a real need to apply auto release for those on-stack paths.

Introduces a new macro, BTRFS_PATH_AUTO_RELEASE() which defines one
on-stack btrfs_path structure, initialize it all to 0, then call
btrfs_release_path() on it when exiting the scope.

This applies to current 3 on-stack path usages:

- defrag_get_extent() in defrag.c

- print_data_reloc_error() in inode.c
  There is a special case where we want to release the path early before
  the time consuming iterate_extent_inodes() call, thus that manual
  early release is kept as is, with an extra comment added.

- scrub_radi56_parity_stripe() in scrub.c

Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent ddea9178

fs/btrfs/ctree.h

+9 −0
Original line number Diff line number Diff line
@@ -85,6 +85,14 @@ struct btrfs_path { 
#define BTRFS_PATH_AUTO_FREE(path_name)					\

	struct btrfs_path *path_name __free(btrfs_free_path) = NULL



/*

 * This defines an on-stack path that will be auto released when exiting the scope.

 *

 * It is compatible with any existing manual btrfs_release_path() calls.

 */

#define BTRFS_PATH_AUTO_RELEASE(path_name)					\

	struct btrfs_path path_name __free(btrfs_release_path) = { 0 }



/*

 * The state of btrfs root

 */
@@ -601,6 +609,7 @@ void btrfs_release_path(struct btrfs_path *p); 
struct btrfs_path *btrfs_alloc_path(void);

void btrfs_free_path(struct btrfs_path *p);

DEFINE_FREE(btrfs_free_path, struct btrfs_path *, btrfs_free_path(_T))

DEFINE_FREE(btrfs_release_path, struct btrfs_path, btrfs_release_path(&_T))



int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,

		   struct btrfs_path *path, int slot, int nr);

fs/btrfs/defrag.c

+1 −4
Original line number Diff line number Diff line
@@ -609,7 +609,7 @@ static struct extent_map *defrag_get_extent(struct btrfs_inode *inode, 
{

	struct btrfs_root *root = inode->root;

	struct btrfs_file_extent_item *fi;

	struct btrfs_path path = { 0 };

	BTRFS_PATH_AUTO_RELEASE(path);

	struct extent_map *em;

	struct btrfs_key key;

	u64 ino = btrfs_ino(inode);
@@ -720,16 +720,13 @@ static struct extent_map *defrag_get_extent(struct btrfs_inode *inode, 
		if (ret > 0)

			goto not_found;

	}

	btrfs_release_path(&path);

	return em;



not_found:

	btrfs_release_path(&path);

	btrfs_free_extent_map(em);

	return NULL;



err:

	btrfs_release_path(&path);

	btrfs_free_extent_map(em);

	return ERR_PTR(ret);

}

fs/btrfs/inode.c

+5 −3
Original line number Diff line number Diff line
@@ -217,7 +217,7 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off 
				   int mirror_num)

{

	struct btrfs_fs_info *fs_info = inode->root->fs_info;

	struct btrfs_path path = { 0 };

	BTRFS_PATH_AUTO_RELEASE(path);

	struct btrfs_key found_key = { 0 };

	struct extent_buffer *eb;

	struct btrfs_extent_item *ei;
@@ -255,7 +255,6 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off 
	if (ret < 0) {

		btrfs_err_rl(fs_info, "failed to lookup extent item for logical %llu: %d",

			     logical, ret);

		btrfs_release_path(&path);

		return;

	}

	eb = path.nodes[0];
@@ -285,11 +284,14 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off 
				(ref_level ? "node" : "leaf"),

				ref_level, ref_root);

		}

		btrfs_release_path(&path);

	} else {

		struct btrfs_backref_walk_ctx ctx = { 0 };

		struct data_reloc_warn reloc_warn = { 0 };



		/*

		 * Do not hold the path as later iterate_extent_inodes() call

		 * can be time consuming.

		 */

		btrfs_release_path(&path);



		ctx.bytenr = found_key.objectid;

fs/btrfs/scrub.c

+8 −15
Original line number Diff line number Diff line
@@ -2171,8 +2171,8 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx, 
				      u64 full_stripe_start)

{

	struct btrfs_fs_info *fs_info = sctx->fs_info;

	struct btrfs_path extent_path = { 0 };

	struct btrfs_path csum_path = { 0 };

	BTRFS_PATH_AUTO_RELEASE(extent_path);

	BTRFS_PATH_AUTO_RELEASE(csum_path);

	struct scrub_stripe *stripe;

	bool all_empty = true;

	const int data_stripes = nr_data_stripes(map);
@@ -2224,7 +2224,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx, 
				full_stripe_start + btrfs_stripe_nr_to_offset(i),

				BTRFS_STRIPE_LEN, stripe);

		if (ret < 0)

			goto out;

			return ret;

		/*

		 * No extent in this data stripe, need to manually mark them

		 * initialized to make later read submission happy.
@@ -2246,10 +2246,8 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx, 
			break;

		}

	}

	if (all_empty) {

		ret = 0;

		goto out;

	}

	if (all_empty)

		return 0;



	for (int i = 0; i < data_stripes; i++) {

		stripe = &sctx->raid56_data_stripes[i];
@@ -2290,20 +2288,15 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx, 
"scrub: unrepaired sectors detected, full stripe %llu data stripe %u errors %*pbl",

				  full_stripe_start, i, stripe->nr_sectors,

				  &error);

			ret = -EIO;

			goto out;

			return ret;

		}

		bitmap_or(&extent_bitmap, &extent_bitmap, &has_extent,

			  stripe->nr_sectors);

	}



	/* Now we can check and regenerate the P/Q stripe. */

	ret = scrub_raid56_cached_parity(sctx, scrub_dev, map, full_stripe_start,

	return scrub_raid56_cached_parity(sctx, scrub_dev, map, full_stripe_start,

					  &extent_bitmap);

out:

	btrfs_release_path(&extent_path);

	btrfs_release_path(&csum_path);

	return ret;

}



/*