Re: [PATCH 4/6] btrfs: defrag: use auto kfree in defrag_one_range() for folios array

On Mon, Jun 29, 2026 at 11:55 PM David Sterba <[email protected]> wrote:
>
> On Thu, Jun 25, 2026 at 08:20:14PM +0100, [email protected] wrote:
> > From: Filipe Manana <[email protected]>
> >
> > Use AUTO_KFREE() for the folios array, avoiding two kfree() calls, one of
> > them in a very specific error path.
> >
> > Signed-off-by: Filipe Manana <[email protected]>
> > ---
> >  fs/btrfs/defrag.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
> > index ad1d04d8f165..e454b59d6477 100644
> > --- a/fs/btrfs/defrag.c
> > +++ b/fs/btrfs/defrag.c
> > @@ -1169,7 +1169,7 @@ static int defrag_one_range(struct btrfs_inode *inode, u64 start, u32 len,
> >       struct defrag_target_range *entry;
> >       struct defrag_target_range *tmp;
> >       LIST_HEAD(target_list);
> > -     struct folio **folios;
> > +     struct folio AUTO_KFREE(*folios);
> >       const u32 sectorsize = inode->root->fs_info->sectorsize;
> >       u64 cur = start;
> >       const unsigned int nr_pages = ((start + len - 1) >> PAGE_SHIFT) -
> > @@ -1196,10 +1196,8 @@ static int defrag_one_range(struct btrfs_inode *inode, u64 start, u32 len,
> >        * range or the extent lock.
> >        */
> >       ret = btrfs_delalloc_reserve_space(inode, &data_reserved, start, len);
> > -     if (ret < 0) {
> > -             kfree(folios);
> > +     if (ret < 0)
> >               return ret;
> > -     }
> >
> >       /* Prepare all pages */
> >       for (int i = 0; cur < start + len && i < nr_pages; i++) {
> > @@ -1251,7 +1249,6 @@ static int defrag_one_range(struct btrfs_inode *inode, u64 start, u32 len,
> >               folio_unlock(folios[i]);
> >               folio_put(folios[i]);
> >       }
> > -     kfree(folios);
>
> There's non-trivial work following this freeing, down from

I'd rather disagree it's non-trivial work.
Everything below kfree() is trivial, and it was placed there in patch
1 by chance.

> btrfs_delalloc_release_space(). I consider using auto freeing in such
> cases as anti-pattern and have skipped converting them.
>
> In this function there's only one early exit from the function so the
> auto freeing is underused, I'd rather not do it.
>
> >       btrfs_delalloc_release_extents(inode, len);
> >       if (last_defrag_end < start + len)
> >               btrfs_delalloc_release_space(inode, data_reserved, last_defrag_end,
> > --
> > 2.47.2
> >