Re: [PATCH v5 2/2] xfs: prevent close() from hanging on frozen filesystems

[Aditya Prakash Srivastava]

On Wed, Jun 24, 2026 at 11:58 PM Aditya Prakash Srivastava
<aditya.ansh182@xxxxxxxxx> wrote:
>
> Hi Darrick,
>
> Thanks for the feedback and review! I hope you had a great
> vacation.
>
> I looked closely at wrapping the block inside xfs_file_release()
> with sb_start_write_trylock(), but I believe sticking with the
> transaction flag approach is much more robust for a few reasons:
>
> Lock Ordering Safety & Lockdep:
> In XFS, the canonical locking hierarchy is:
>
> sb_start_write (freeze lock) -> IOLOCK -> ILOCK ->
> xfs_trans_alloc
>
> xfs_file_release() already holds XFS_IOLOCK_EXCL (acquired via
> xfs_ilock_nowait()). Attempting to acquire
> sb_start_write_trylock() inside that block would mean acquiring
> the freeze lock while holding the IOLOCK. Even though a trylock
> itself does not block, this is a direct inversion of the
> canonical locking hierarchy and is highly likely to trigger
> lockdep dependency warnings.
>
> Keeping the check inside xfs_trans_alloc() relies on
> well-defined, centralized write-count evaluation, avoiding any
> lockdep headaches.
>
> Encapsulation and Readability:
> Open-coding the freeze trylock at the VFS release boundary makes
> xfs_file_release() quite fragile and harder to reason about
> over time. The resulting nested conditional block is prone to
> subtle bugs during future refactoring of the file release and
> blockgc paths.
>
> Architectural Consistency:
> xfs_trans_alloc() is the canonical, centralized place in XFS to
> manage write-count blocking semantics. The
> XFS_TRANS_WRITECOUNT_TRYLOCK flag provides an explicit, clean
> interface to tell the transaction layer exactly how to handle a
> freeze, rather than bypassing it at a higher layer.
>
> Given that this keeps the VFS/XFS interface clean and Christoph has
> already reviewed/endorsed this version, I'd prefer to move forward
> with the current iteration unless there is a strict functional
> downside you see to the flag approach.
>
> Thanks again for the review and guidance!
>
> Best regards,
> Aditya
>

Hi,

Just a gentle ping on this patch.

I replied to Darrick's suggestion regarding the
sb_start_write_trylock() approach, but I haven't seen any
further feedback since then.

Are there any remaining concerns with the current approach,
or is the patch suitable for merging as-is?

Thanks!

Regards,
Aditya