diff mbox series

[1/7] btrfs: do more trivial BTRFS_PATH_AUTO_FREE conversions

Message ID b4a20aa684dc9f0324c7fe4728c1829a8b996f71.1743549291.git.dsterba@suse.com (mailing list archive)
State New
Headers show
Series More btrfs_path auto cleaning | expand

Commit Message

David Sterba April 1, 2025, 11:18 p.m. UTC 
The most trivial pattern for the auto freeing when the variable is
declared with the macro and the final btrfs_free_path() is removed.
There are almost none goto -> return conversions and there's no other
function cleanup.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/block-group.c      |  3 +--
 fs/btrfs/fiemap.c           |  3 +--
 fs/btrfs/file-item.c        |  3 +--
 fs/btrfs/file.c             |  3 +--
 fs/btrfs/free-space-cache.c |  3 +--
 fs/btrfs/inode.c            | 27 +++++++++------------------
 6 files changed, 14 insertions(+), 28 deletions(-)

Comments

David Disseldorp April 2, 2025, 12:39 a.m. UTC | #1 
Hi David

On Wed,  2 Apr 2025 01:18:06 +0200, David Sterba wrote:

> @@ -308,7 +308,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
>  	bool locked = false;
>  
>  	if (block_group) {
> -		struct btrfs_path *path = btrfs_alloc_path();
> +		BTRFS_PATH_AUTO_FREE(path);
>  
>  		if (!path) {
>  			ret = -ENOMEM;

This one looks broken. btrfs_search_slot() needs it allocated.
David Sterba April 2, 2025, 10:02 p.m. UTC | #2 
On Wed, Apr 02, 2025 at 11:39:51AM +1100, David Disseldorp wrote:
> Hi David
> 
> On Wed,  2 Apr 2025 01:18:06 +0200, David Sterba wrote:
> 
> > @@ -308,7 +308,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
> >  	bool locked = false;
> >  
> >  	if (block_group) {
> > -		struct btrfs_path *path = btrfs_alloc_path();
> > +		BTRFS_PATH_AUTO_FREE(path);
> >  
> >  		if (!path) {
> >  			ret = -ENOMEM;
> 
> This one looks broken. btrfs_search_slot() needs it allocated.

Sorry, I don't see what you mean. There's no btrfs_search_slot() in
btrfs_truncate_free_space_cache(), perhaps you mean a different
function?

The macro BTRFS_PATH_AUTO_FREE still declares a pointer, only adds the
__cleanup parameter that calls btrfs_free_path() when the variable
leaves its scope. It's not declaring 'path' as plain variable, if I
interpret the comment 'needs it allocated' in that context.
David Disseldorp April 2, 2025, 11:14 p.m. UTC | #3 
On Thu, 3 Apr 2025 00:02:25 +0200, David Sterba wrote:

> On Wed, Apr 02, 2025 at 11:39:51AM +1100, David Disseldorp wrote:
> > Hi David
> > 
> > On Wed,  2 Apr 2025 01:18:06 +0200, David Sterba wrote:
> >   
> > > @@ -308,7 +308,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
> > >  	bool locked = false;
> > >  
> > >  	if (block_group) {
> > > -		struct btrfs_path *path = btrfs_alloc_path();
> > > +		BTRFS_PATH_AUTO_FREE(path);
> > >  
> > >  		if (!path) {
> > >  			ret = -ENOMEM;  
> > 
> > This one looks broken. btrfs_search_slot() needs it allocated.  
> 
> Sorry, I don't see what you mean. There's no btrfs_search_slot() in
> btrfs_truncate_free_space_cache(), perhaps you mean a different
> function?

This will jump straight through to the -ENOMEM goto fail path... What am
I missing here?
With 91e5bfe317d8f8471fbaa3e70cf66cae1314a516 I see:
#define BTRFS_PATH_AUTO_FREE(path_name)                                 \              
        struct btrfs_path *path_name __free(btrfs_free_path) = NULL 

I would expect your change to instead be something like:

   BTRFS_PATH_AUTO_FREE(path);
   path = btrfs_alloc_path();
David Sterba April 2, 2025, 11:45 p.m. UTC | #4 
On Thu, Apr 03, 2025 at 10:14:09AM +1100, David Disseldorp wrote:
> On Thu, 3 Apr 2025 00:02:25 +0200, David Sterba wrote:
> 
> > On Wed, Apr 02, 2025 at 11:39:51AM +1100, David Disseldorp wrote:
> > > Hi David
> > > 
> > > On Wed,  2 Apr 2025 01:18:06 +0200, David Sterba wrote:
> > >   
> > > > @@ -308,7 +308,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
> > > >  	bool locked = false;
> > > >  
> > > >  	if (block_group) {
> > > > -		struct btrfs_path *path = btrfs_alloc_path();
> > > > +		BTRFS_PATH_AUTO_FREE(path);
> > > >  
> > > >  		if (!path) {
> > > >  			ret = -ENOMEM;  
> > > 
> > > This one looks broken. btrfs_search_slot() needs it allocated.  
> > 
> > Sorry, I don't see what you mean. There's no btrfs_search_slot() in
> > btrfs_truncate_free_space_cache(), perhaps you mean a different
> > function?
> 
> This will jump straight through to the -ENOMEM goto fail path... What am
> I missing here?
> With 91e5bfe317d8f8471fbaa3e70cf66cae1314a516 I see:
> #define BTRFS_PATH_AUTO_FREE(path_name)                                 \              
>         struct btrfs_path *path_name __free(btrfs_free_path) = NULL 
> 
> I would expect your change to instead be something like:
> 
>    BTRFS_PATH_AUTO_FREE(path);
>    path = btrfs_alloc_path();

Duh, of course, you're right, thanks for catching it.
diff mbox series

Patch

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index a38578c60f34e6..3eba00da9fc7a5 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -700,7 +700,7 @@  static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 	struct btrfs_block_group *block_group = caching_ctl->block_group;
 	struct btrfs_fs_info *fs_info = block_group->fs_info;
 	struct btrfs_root *extent_root;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
 	u64 total_found = 0;
@@ -827,7 +827,6 @@  static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 				       block_group->start + block_group->length,
 				       NULL);
 out:
-	btrfs_free_path(path);
 	return ret;
 }
 
diff --git a/fs/btrfs/fiemap.c b/fs/btrfs/fiemap.c
index b80c07ad8c5e71..7715e30508c575 100644
--- a/fs/btrfs/fiemap.c
+++ b/fs/btrfs/fiemap.c
@@ -634,7 +634,7 @@  static int extent_fiemap(struct btrfs_inode *inode,
 	const u64 ino = btrfs_ino(inode);
 	struct extent_state *cached_state = NULL;
 	struct extent_state *delalloc_cached_state = NULL;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct fiemap_cache cache = { 0 };
 	struct btrfs_backref_share_check_ctx *backref_ctx;
 	u64 last_extent_end = 0;
@@ -874,7 +874,6 @@  static int extent_fiemap(struct btrfs_inode *inode,
 	free_extent_state(delalloc_cached_state);
 	kfree(cache.entries);
 	btrfs_free_backref_share_ctx(backref_ctx);
-	btrfs_free_path(path);
 	return ret;
 }
 
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 344b4db487a0c6..c191be6bcefbd2 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -1048,7 +1048,7 @@  int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_key file_key;
 	struct btrfs_key found_key;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct btrfs_csum_item *item;
 	struct btrfs_csum_item *item_end;
 	struct extent_buffer *leaf = NULL;
@@ -1259,7 +1259,6 @@  int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 		goto again;
 	}
 out:
-	btrfs_free_path(path);
 	return ret;
 }
 
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fe81129cfbf1b7..e7e8c477f83701 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -553,7 +553,7 @@  int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
 {
 	struct btrfs_root *root = inode->root;
 	struct extent_buffer *leaf;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct btrfs_file_extent_item *fi;
 	struct btrfs_ref ref = { 0 };
 	struct btrfs_key key;
@@ -791,7 +791,6 @@  int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
 		}
 	}
 out:
-	btrfs_free_path(path);
 	return ret;
 }
 
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 1b3082e81220d2..f66a0a6e505079 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -308,7 +308,7 @@  int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
 	bool locked = false;
 
 	if (block_group) {
-		struct btrfs_path *path = btrfs_alloc_path();
+		BTRFS_PATH_AUTO_FREE(path);
 
 		if (!path) {
 			ret = -ENOMEM;
@@ -330,7 +330,6 @@  int btrfs_truncate_free_space_cache(struct btrfs_trans_handle *trans,
 		spin_lock(&block_group->lock);
 		block_group->disk_cache_state = BTRFS_DC_CLEAR;
 		spin_unlock(&block_group->lock);
-		btrfs_free_path(path);
 	}
 
 	btrfs_i_size_write(inode, 0);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 520b12638ee49c..db989572cba419 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2933,7 +2933,7 @@  static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
 {
 	struct btrfs_root *root = inode->root;
 	const u64 sectorsize = root->fs_info->sectorsize;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct extent_buffer *leaf;
 	struct btrfs_key ins;
 	u64 disk_num_bytes = btrfs_stack_file_extent_disk_num_bytes(stack_fi);
@@ -3015,8 +3015,6 @@  static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
 					       file_pos - offset,
 					       qgroup_reserved, &ins);
 out:
-	btrfs_free_path(path);
-
 	return ret;
 }
 
@@ -3540,7 +3538,7 @@  static int btrfs_orphan_del(struct btrfs_trans_handle *trans,
 int btrfs_orphan_cleanup(struct btrfs_root *root)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct extent_buffer *leaf;
 	struct btrfs_key key, found_key;
 	struct btrfs_trans_handle *trans;
@@ -3730,7 +3728,6 @@  int btrfs_orphan_cleanup(struct btrfs_root *root)
 out:
 	if (ret)
 		btrfs_err(fs_info, "could not do orphan cleanup %d", ret);
-	btrfs_free_path(path);
 	return ret;
 }
 
@@ -4123,7 +4120,7 @@  static noinline int btrfs_update_inode_item(struct btrfs_trans_handle *trans,
 					    struct btrfs_inode *inode)
 {
 	struct btrfs_inode_item *inode_item;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
 	int ret;
@@ -4137,7 +4134,7 @@  static noinline int btrfs_update_inode_item(struct btrfs_trans_handle *trans,
 	if (ret) {
 		if (ret > 0)
 			ret = -ENOENT;
-		goto failed;
+		return ret;
 	}
 
 	leaf = path->nodes[0];
@@ -4146,10 +4143,7 @@  static noinline int btrfs_update_inode_item(struct btrfs_trans_handle *trans,
 
 	fill_inode_item(trans, leaf, inode_item, &inode->vfs_inode);
 	btrfs_set_inode_last_trans(trans, inode);
-	ret = 0;
-failed:
-	btrfs_free_path(path);
-	return ret;
+	return 0;
 }
 
 /*
@@ -5456,7 +5450,7 @@  static int btrfs_inode_by_name(struct btrfs_inode *dir, struct dentry *dentry,
 			       struct btrfs_key *location, u8 *type)
 {
 	struct btrfs_dir_item *di;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct btrfs_root *root = dir->root;
 	int ret = 0;
 	struct fscrypt_name fname;
@@ -5467,7 +5461,7 @@  static int btrfs_inode_by_name(struct btrfs_inode *dir, struct dentry *dentry,
 
 	ret = fscrypt_setup_filename(&dir->vfs_inode, &dentry->d_name, 1, &fname);
 	if (ret < 0)
-		goto out;
+		return ret;
 	/*
 	 * fscrypt_setup_filename() should never return a positive value, but
 	 * gcc on sparc/parisc thinks it can, so assert that doesn't happen.
@@ -5496,7 +5490,6 @@  static int btrfs_inode_by_name(struct btrfs_inode *dir, struct dentry *dentry,
 		*type = btrfs_dir_ftype(path->nodes[0], di);
 out:
 	fscrypt_free_filename(&fname);
-	btrfs_free_path(path);
 	return ret;
 }
 
@@ -5511,7 +5504,7 @@  static int fixup_tree_root_location(struct btrfs_fs_info *fs_info,
 				    struct btrfs_key *location,
 				    struct btrfs_root **sub_root)
 {
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	struct btrfs_root *new_root;
 	struct btrfs_root_ref *ref;
 	struct extent_buffer *leaf;
@@ -5567,7 +5560,6 @@  static int fixup_tree_root_location(struct btrfs_fs_info *fs_info,
 	location->offset = 0;
 	err = 0;
 out:
-	btrfs_free_path(path);
 	fscrypt_free_filename(&fname);
 	return err;
 }
@@ -5988,7 +5980,7 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 	struct btrfs_dir_item *di;
 	struct btrfs_key key;
 	struct btrfs_key found_key;
-	struct btrfs_path *path;
+	BTRFS_PATH_AUTO_FREE(path);
 	void *addr;
 	LIST_HEAD(ins_list);
 	LIST_HEAD(del_list);
@@ -6101,7 +6093,6 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 err:
 	if (put)
 		btrfs_readdir_put_delayed_items(BTRFS_I(inode), &ins_list, &del_list);
-	btrfs_free_path(path);
 	return ret;
 }