Re: [PATCH 1/3] btrfs: clear BTRFS_ROOT_IN_TRANS_SETUP on early exit from record_root_in_trans()

[Boris Burkov <boris@bur.io>]

On Fri, Sep 18, 2026 at 06:32:05PM +0100, Filipe Manana wrote:
> On Fri, Sep 18, 2026 at 6:18 PM Boris Burkov <boris@bur.io> wrote:
> >
> > On Fri, Sep 18, 2026 at 01:28:09PM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > If we exit early because the transaction that last used the root already
> > > matches the current transaction, we leave the BTRFS_ROOT_IN_TRANS_SETUP
> > > bit set in the root (which we just set right before the exit). While this
> > > does not cause any functional issue, it makes callers of
> > > btrfs_record_root_in_trans() always lock fs_info->reloc_mutex and call
> >
> > I found "always" kind of confusing here. It's until one succeeds and
> > clears the bit right? It kind of makes it sound like it leaks it
> > forever.
> 
> Yes, I can reword the sentence to:
> 
> "While this does not cause any functional issue, it makes callers of
>   btrfs_record_root_in_trans() lock fs_info->reloc_mutex and call
>   record_root_in_trans() for nothing, causing unnecessary lock contention,
>   until one of them clears the bit in record_root_in_trans()."
> 
> Thanks.
> 

That version sounds great!

> >
> > > record_root_in_trans() for nothing, causing unnecessary lock contention.
> > > One caller of btrfs_record_root_in_trans() is start_transaction(), used to
> > > start new transaction or joining an existing one, which is a hot path.
> > >
> > > So clear BTRFS_ROOT_IN_TRANS_SETUP on early exit.
> > >
> > > Assisted-by: LLM
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > >  fs/btrfs/transaction.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> > > index ca114235bbe1..c1555621ae4e 100644
> > > --- a/fs/btrfs/transaction.c
> > > +++ b/fs/btrfs/transaction.c
> > > @@ -432,6 +432,7 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
> > >               spin_lock(&fs_info->fs_roots_radix_lock);
> > >               if (btrfs_get_root_last_trans(root) == trans->transid && !force) {
> > >                       spin_unlock(&fs_info->fs_roots_radix_lock);
> > > +                     clear_bit(BTRFS_ROOT_IN_TRANS_SETUP, &root->state);
> > >                       return 0;
> > >               }
> > >               radix_tree_tag_set(&fs_info->fs_roots_radix,
> > > --
> > > 2.47.2
> > >