Re: [PATCH] xfs: fix deadlock between busy flushing and t_busy

[Dave Chinner <david@fromorbit.com>]

[ Please reply on list, added linux-xfs back to cc list. ]

On Sat, Nov 22, 2025 at 05:28:25PM +0800, haoqin huang wrote:
> Hi Dave,
> 
> Thanks for your reviews, and sorry for response lately.
> 
> I’m very agree that deferred frees largely resolved the deadlock issue.
> 
> Maybe I should split two parts of this patch to show my idea:
> Part 1. fix fallback of xfs_refcount_split_extent()
> It seems better to  fix a logic bug in xfs_refcount_split_extent().
> When splitting an extent, we update the existing record before
> inserting the new one. If the insertion fails, we currently return
> without restoring the original record, leaving the btree in an
> inconsistent state.

Yes, because we can't actually recover from such a failure. We have
dirtied the transaction, so when it gets cancelled on an error
return, it will shut down the filesystem. Hence there is no point in
trying to unwind previous modifications in the transaction on
error...

> This part does not seem to be necessarily related to the
> aforementioned deadlock.

It isn't, but it is also unnecessary.

> Part 2. Robustify the rollback path to prevent deadlocks
> The change to xfs_extent_busy_flush() is just added as a secondary
> hardening measure for edge cases.
> I’m not sure, but theoretically, the alloc_flag to be zero, then
> entering a cycle with a high probability of deadlock.

We definitely allow alloc_flags to be zero - this is not a bug or
a behaviour that needs to be worked around. Those callers aren't
likely to also hold overlapping busy extents in the transaction
themselves, so there shouldn't be any deadlocks in those callers.

Really, it's the nature of the btree modifications (multiple record
updates (and hence block free/alloc) per transaction) that leads to
the potential deadlock. Only the BMBT and REFCNTBT trees have this
issue because they require allocation from the free space btrees
rather than the AGFL pool. However, multi-record updates of these
btrees are no longer deadlock prone because of the deferred freeing
of extents.

Hence, in all other contexts, it is generally safe to call
xfs_extent_busy_flush(0) and block waiting on busy extent processing
to complete. There is no reason to force callers like this to
implement an -EAGAIN loop around xfs_extent_busy_flush() in an
attempt to prevent deadlocks. i.e. if there is a deadlock, they'll
now just have to busy-loop trying to flush instead of sleeping
waiting for flush completion...

-Dave.
-- 
Dave Chinner
david@fromorbit.com