Skip to content

Commit 377ae92

Browse files
adam900710kdave
authored andcommitted
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 <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent ac7fe3a commit 377ae92

File tree

4 files changed

+23
-22
lines changed

4 files changed

+23
-22
lines changed

fs/btrfs/ctree.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ struct btrfs_path {
8585
#define BTRFS_PATH_AUTO_FREE(path_name) \
8686
struct btrfs_path *path_name __free(btrfs_free_path) = NULL
8787

88+
/*
89+
* This defines an on-stack path that will be auto released when exiting the scope.
90+
*
91+
* It is compatible with any existing manual btrfs_release_path() calls.
92+
*/
93+
#define BTRFS_PATH_AUTO_RELEASE(path_name) \
94+
struct btrfs_path path_name __free(btrfs_release_path) = { 0 }
95+
8896
/*
8997
* The state of btrfs root
9098
*/
@@ -601,6 +609,7 @@ void btrfs_release_path(struct btrfs_path *p);
601609
struct btrfs_path *btrfs_alloc_path(void);
602610
void btrfs_free_path(struct btrfs_path *p);
603611
DEFINE_FREE(btrfs_free_path, struct btrfs_path *, btrfs_free_path(_T))
612+
DEFINE_FREE(btrfs_release_path, struct btrfs_path, btrfs_release_path(&_T))
604613

605614
int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,
606615
struct btrfs_path *path, int slot, int nr);

fs/btrfs/defrag.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ static struct extent_map *defrag_get_extent(struct btrfs_inode *inode,
609609
{
610610
struct btrfs_root *root = inode->root;
611611
struct btrfs_file_extent_item *fi;
612-
struct btrfs_path path = { 0 };
612+
BTRFS_PATH_AUTO_RELEASE(path);
613613
struct extent_map *em;
614614
struct btrfs_key key;
615615
u64 ino = btrfs_ino(inode);
@@ -720,16 +720,13 @@ static struct extent_map *defrag_get_extent(struct btrfs_inode *inode,
720720
if (ret > 0)
721721
goto not_found;
722722
}
723-
btrfs_release_path(&path);
724723
return em;
725724

726725
not_found:
727-
btrfs_release_path(&path);
728726
btrfs_free_extent_map(em);
729727
return NULL;
730728

731729
err:
732-
btrfs_release_path(&path);
733730
btrfs_free_extent_map(em);
734731
return ERR_PTR(ret);
735732
}

fs/btrfs/inode.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off
217217
int mirror_num)
218218
{
219219
struct btrfs_fs_info *fs_info = inode->root->fs_info;
220-
struct btrfs_path path = { 0 };
220+
BTRFS_PATH_AUTO_RELEASE(path);
221221
struct btrfs_key found_key = { 0 };
222222
struct extent_buffer *eb;
223223
struct btrfs_extent_item *ei;
@@ -255,7 +255,6 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off
255255
if (ret < 0) {
256256
btrfs_err_rl(fs_info, "failed to lookup extent item for logical %llu: %d",
257257
logical, ret);
258-
btrfs_release_path(&path);
259258
return;
260259
}
261260
eb = path.nodes[0];
@@ -285,11 +284,14 @@ static void print_data_reloc_error(const struct btrfs_inode *inode, u64 file_off
285284
(ref_level ? "node" : "leaf"),
286285
ref_level, ref_root);
287286
}
288-
btrfs_release_path(&path);
289287
} else {
290288
struct btrfs_backref_walk_ctx ctx = { 0 };
291289
struct data_reloc_warn reloc_warn = { 0 };
292290

291+
/*
292+
* Do not hold the path as later iterate_extent_inodes() call
293+
* can be time consuming.
294+
*/
293295
btrfs_release_path(&path);
294296

295297
ctx.bytenr = found_key.objectid;

fs/btrfs/scrub.c

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2171,8 +2171,8 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
21712171
u64 full_stripe_start)
21722172
{
21732173
struct btrfs_fs_info *fs_info = sctx->fs_info;
2174-
struct btrfs_path extent_path = { 0 };
2175-
struct btrfs_path csum_path = { 0 };
2174+
BTRFS_PATH_AUTO_RELEASE(extent_path);
2175+
BTRFS_PATH_AUTO_RELEASE(csum_path);
21762176
struct scrub_stripe *stripe;
21772177
bool all_empty = true;
21782178
const int data_stripes = nr_data_stripes(map);
@@ -2224,7 +2224,7 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
22242224
full_stripe_start + btrfs_stripe_nr_to_offset(i),
22252225
BTRFS_STRIPE_LEN, stripe);
22262226
if (ret < 0)
2227-
goto out;
2227+
return ret;
22282228
/*
22292229
* No extent in this data stripe, need to manually mark them
22302230
* initialized to make later read submission happy.
@@ -2246,10 +2246,8 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
22462246
break;
22472247
}
22482248
}
2249-
if (all_empty) {
2250-
ret = 0;
2251-
goto out;
2252-
}
2249+
if (all_empty)
2250+
return 0;
22532251

22542252
for (int i = 0; i < data_stripes; i++) {
22552253
stripe = &sctx->raid56_data_stripes[i];
@@ -2290,20 +2288,15 @@ static int scrub_raid56_parity_stripe(struct scrub_ctx *sctx,
22902288
"scrub: unrepaired sectors detected, full stripe %llu data stripe %u errors %*pbl",
22912289
full_stripe_start, i, stripe->nr_sectors,
22922290
&error);
2293-
ret = -EIO;
2294-
goto out;
2291+
return ret;
22952292
}
22962293
bitmap_or(&extent_bitmap, &extent_bitmap, &has_extent,
22972294
stripe->nr_sectors);
22982295
}
22992296

23002297
/* Now we can check and regenerate the P/Q stripe. */
2301-
ret = scrub_raid56_cached_parity(sctx, scrub_dev, map, full_stripe_start,
2302-
&extent_bitmap);
2303-
out:
2304-
btrfs_release_path(&extent_path);
2305-
btrfs_release_path(&csum_path);
2306-
return ret;
2298+
return scrub_raid56_cached_parity(sctx, scrub_dev, map, full_stripe_start,
2299+
&extent_bitmap);
23072300
}
23082301

23092302
/*

0 commit comments

Comments
 (0)