From: Qu Wenruo Date: Tue, 25 Nov 2025 21:50:21 +0000 (+1030) Subject: btrfs: introduce BTRFS_PATH_AUTO_RELEASE() helper X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fab0c0f03cfd0dfe793889e3374ebc68ecf18889;p=thirdparty%2Fkernel%2Flinux.git 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: Qu Wenruo Reviewed-by: David Sterba Signed-off-by: David Sterba --- diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 692370fc07b28..6de7ad191e048 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -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); diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c index b81e224d4a27e..bcc6656ad0343 100644 --- a/fs/btrfs/defrag.c +++ b/fs/btrfs/defrag.c @@ -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 @@ next: 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); } diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 5dceb03bee0a3..912343fc9a731 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -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; diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 1a60e631d801c..2372084cf6c52 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -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, - &extent_bitmap); -out: - btrfs_release_path(&extent_path); - btrfs_release_path(&csum_path); - return ret; + return scrub_raid56_cached_parity(sctx, scrub_dev, map, full_stripe_start, + &extent_bitmap); } /*