Re: [PATCH 7/8] btrfs: relocation: use on-stack iterator in build_backref_tree

[David Sterba <dsterba@suse.cz>]

On Sat, Sep 23, 2023 at 08:05:25AM +0930, Qu Wenruo wrote:
> On 2023/9/22 20:37, David Sterba wrote:
> > build_backref_tree() is called in a loop by relocate_tree_blocks()
> > for each relocated block. The iterator is allocated and freed repeatedly
> > while we could simply use an on-stack variable to avoid the allocation
> > and remove one more failure case. The stack grows by 48 bytes.
> >
> > This was the only use of btrfs_backref_iter_alloc() so it's changed to
> > be an initializer and btrfs_backref_iter_free() can be removed
> > completely.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/backref.c    | 26 ++++++++++----------------
> >   fs/btrfs/backref.h    | 11 ++---------
> >   fs/btrfs/relocation.c | 12 ++++++------
> >   3 files changed, 18 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index 0dc91bf654b5..691b20b47065 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -2828,26 +2828,20 @@ void free_ipath(struct inode_fs_paths *ipath)
> >   	kfree(ipath);
> >   }
> >
> > -struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
> > +int btrfs_backref_iter_init(struct btrfs_fs_info *fs_info,
> > +			    struct btrfs_backref_iter *iter)
> >   {
> > -	struct btrfs_backref_iter *ret;
> > -
> > -	ret = kzalloc(sizeof(*ret), GFP_NOFS);
> > -	if (!ret)
> > -		return NULL;
> > -
> > -	ret->path = btrfs_alloc_path();
> > -	if (!ret->path) {
> > -		kfree(ret);
> > -		return NULL;
> > -	}
> > +	memset(iter, 0, sizeof(struct btrfs_backref_iter));
> > +	iter->path = btrfs_alloc_path();
> > +	if (!iter->path)
> > +		return -ENOMEM;
> 
> We can do one step further, by integrating the btrfs_path into @iter,
> other than re-allocating it again and again.

Possible but needs to be evaluated separately, the size of path is 112
and this starts to become noticeable on the stack.