| Message ID | 20250411034425.173648-1-frank.li@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_truncate_inode_items() | expand |
On Fri, 11 Apr 2025 at 05:25, Yangtao Li <frank.li@vivo.com> wrote: > > All cleanup paths lead to btrfs_path_free so we can define path with the > automatic free callback. > > And David Sterba point out that: > We may still find cases worth converting, the typical pattern is > btrfs_path_alloc() somewhere near top of the function and > btrfs_free_path() called right before a return. > > So let's convert it. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > fs/btrfs/inode-item.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) And what about the other functions in that file? We could even get rid of two allocations passing the path from ..._inode_ref() to ..._inode_extref(). > diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c > index 3530de0618c8..c9d37f6bb099 100644 > --- a/fs/btrfs/inode-item.c > +++ b/fs/btrfs/inode-item.c > @@ -456,7 +456,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans, > struct btrfs_truncate_control *control) > { > struct btrfs_fs_info *fs_info = root->fs_info; > - struct btrfs_path *path; > + BTRFS_PATH_AUTO_FREE(path); > struct extent_buffer *leaf; > struct btrfs_file_extent_item *fi; > struct btrfs_key key; > @@ -743,6 +743,5 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans, > if (!ret && control->last_size > new_size) > control->last_size = new_size; > > - btrfs_free_path(path); > return ret; > } > -- > 2.39.0 > >
On Fri, Apr 11, 2025 at 04:17:38PM +0200, Daniel Vacek wrote: > On Fri, 11 Apr 2025 at 05:25, Yangtao Li <frank.li@vivo.com> wrote: > > > > All cleanup paths lead to btrfs_path_free so we can define path with the > > automatic free callback. > > > > And David Sterba point out that: > > We may still find cases worth converting, the typical pattern is > > btrfs_path_alloc() somewhere near top of the function and > > btrfs_free_path() called right before a return. > > > > So let's convert it. > > > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > > --- > > fs/btrfs/inode-item.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > And what about the other functions in that file? We could even get rid > of two allocations passing the path from ..._inode_ref() to > ..._inode_extref(). If you mean to pass the path object from btrfs_del_inode_ref() to btrfs_del_inode_extref() yeah this looks like a good optimization and reducing the allocations (and potential failures). The other cases in the "..._inode_ref" is btrfs_insert_inode_ref() -> btrfs_insert_inode_extref().
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c index 3530de0618c8..c9d37f6bb099 100644 --- a/fs/btrfs/inode-item.c +++ b/fs/btrfs/inode-item.c @@ -456,7 +456,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans, struct btrfs_truncate_control *control) { struct btrfs_fs_info *fs_info = root->fs_info; - struct btrfs_path *path; + BTRFS_PATH_AUTO_FREE(path); struct extent_buffer *leaf; struct btrfs_file_extent_item *fi; struct btrfs_key key; @@ -743,6 +743,5 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans, if (!ret && control->last_size > new_size) control->last_size = new_size; - btrfs_free_path(path); return ret; }
All cleanup paths lead to btrfs_path_free so we can define path with the automatic free callback. And David Sterba point out that: We may still find cases worth converting, the typical pattern is btrfs_path_alloc() somewhere near top of the function and btrfs_free_path() called right before a return. So let's convert it. Signed-off-by: Yangtao Li <frank.li@vivo.com> --- fs/btrfs/inode-item.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)