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>

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 692370fc07b280ff75b1511aa57d654ee56f6f2a..6de7ad191e0483ddaa98d496cea5aea40ef7a880 100644 (file)
--- 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 b81e224d4a27e6d13e8c47b394db4067eed2ddf1..bcc6656ad034399af12dbe1b5f94c2a421b84cf8 100644 (file)
--- 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 5dceb03bee0a3be6959c35066475c3fc274ea85b..912343fc9a731c5db2589a750d8abd4ef6a2d016 100644 (file)
--- 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 1a60e631d801c726db9d5aa2ecdb8ce68ec04472..2372084cf6c52fcbeae06ce7fd1b63cf98bddf66 100644 (file)
--- 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);
 }
 
 /*