Re: [PATCH] btrfs: Clean up delayed inode items on transaction abort

[Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>]

On Tue, Nov 10, 2026 at 08:37:16AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2026/11/10 06:59, pavankumaryalagada@gmail.com 写道:
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > 
> > When a transaction is aborted, btrfs_commit_inode_delayed_inode()
> > fails because btrfs_join_transaction() returns an error. This makes
> > btrfs_evict_inode() skip btrfs_kill_delayed_inode_items(), leaving
> > pending items in the delayed node.
> > 
> > Move the error check after btrfs_kill_delayed_inode_items() to
> > ensure pending items are removed when the transaction is aborted.
> > 
> > Reported-by: syzbot+55d1814e49f1fc0ba542@syzkaller.appspotmail.com
> > Link: https://lore.kernel.org/all/6af131f2.44d55313.247daf.0001.GAE@google.com/T/
> > Fixes: 2adc75d61203 ("btrfs: move btrfs_kill_delayed_inode_items into evict")
> > Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > ---
> >   fs/btrfs/inode.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index abaa33471456..397aa64b4816 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -5623,8 +5623,6 @@ void btrfs_evict_inode(struct inode *inode)
> >   	 * the inode update is released.
> >   	 */
> >   	ret = btrfs_commit_inode_delayed_inode(BTRFS_I(inode));
> 
> What if btrfs_commit_inode_delayed_inode() failed but without a transaction
> abort?

If the transaction has not been aborted, we shouldn't remove pending delayed
items.

> 
> E.g. btrfs_alloc_path() failed inside btrfs_commit_inode_delayed_inode(),
> then the transaction is just ended regularly without aborting.

Agreed. In this case, -ENOMEM is returned, but the transaction hasn't been
aborted, so we should keep the pending delayed items.

> 
> In that case, won't the btrfs_kill_delayed_inode_items() call causing
> problems?

Yes, calling it unconditionally could remove pending operations even when
the transaction hasn't been aborted. I'm thinking of making the cleanup
conditional so that delayed items are removed when the commit succeeds or
the transaction has been aborted, but are left untouched on other errors
like -ENOMEM.

	ret = btrfs_commit_inode_delayed_inode(BTRFS_I(inode));

	if (!ret || BTRFS_FS_ERROR(fs_info) ||
	    test_bit(BTRFS_FS_STATE_TRANS_ABORTED, &fs_info->fs_state))
		btrfs_kill_delayed_inode_items(BTRFS_I(inode));

	if (ret)
		goto out;

I included both checks because TRANS_ABORTED detects whether a transaction
has been aborted, while BTRFS_FS_ERROR() checks whether a filesystem error
has been recorded.

Since the TRANS_ABORTED bit can be set before the filesystem error is
recorded, checking both ensures that we also catch that case.

This way, we clean up the delayed items if the commit suceeds or the transaction
is aborted, but keep them on recoverable errors like -ENOMEM.

I'll send this as v2. Please let me know if you see any issues with this
approach.

Thanks,
Pavan

> > -	if (ret)
> > -		goto out;
> >   	/*
> >   	 * This drops any pending insert or delete operations we have for this
> > @@ -5634,6 +5632,9 @@ void btrfs_evict_inode(struct inode *inode)
> >   	 */
> >   	btrfs_kill_delayed_inode_items(BTRFS_I(inode));
> > +	if (ret)
> > +		goto out;
> > +
> >   	btrfs_init_metadata_block_rsv(fs_info, &rsv, BTRFS_BLOCK_RSV_TEMP);
> >   	rsv.size = btrfs_calc_metadata_size(fs_info, 1);
> >   	rsv.failfast = true;
>