[PATCH v6 0/1] xfs: prevent close() hangs on frozen filesystems

[PATCH v6 0/1] xfs: prevent close() hangs on frozen filesystems

[Aditya Prakash Srivastava]

Hi Christoph, Darrick, and Carlos,

This is version 6 of the patch series addressing the close() system
call hanging indefinitely on frozen XFS filesystems (Bugzilla #205833).

In v5, I introduced a transaction allocation flag
(XFS_TRANS_WRITECOUNT_TRYLOCK) to handle the trylock inside
__xfs_trans_alloc() and propagated it down to xfs_free_eofblocks().
Christoph suggested this design and reviewed the implementation.

However, Darrick suggested that the deadlock can be resolved much
more simply at the VFS-layer within xfs_file_release() by surrounding
the existing check with sb_start_write_trylock() instead of passing
transaction allocation flags all the way down.

This v6 implements that VFS-layer trylock approach. It modifies only a
single file (fs/xfs/xfs_file.c). If sb_start_write_trylock() fails (meaning
the filesystem is frozen or freezing), we simply skip the speculative
preallocation trim.

I would appreciate your guidance on which of the two architectural
paths is preferred for XFS:
1. The v5 transaction-flag approach (centralized within xfs_trans_alloc).
2. The v6 VFS-level trylock approach (localized within xfs_file_release).

If the v5 approach is preferred, I will rebase the transaction-flag
patches onto the current master and submit them as a new version.

Thanks,
Aditya

Aditya Prakash Srivastava (1):
  xfs: prevent close() from hanging on frozen filesystems

 fs/xfs/xfs_file.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

-- 
2.47.3

[PATCH v6 1/1] xfs: prevent close() from hanging on frozen filesystems

[Aditya Prakash Srivastava]

When a file is closed, xfs_file_release() attempts to trim speculative
post-EOF blocks. This requires allocating a transaction, which blocks
indefinitely if the filesystem is frozen.

Fix the hang by wrapping the preallocation cleanup block with
sb_start_write_trylock() and xfs_ilock_nowait() to bypass the trim
best-effort when the filesystem is frozen or locking fails.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
 fs/xfs/xfs_file.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index d8202da15aca..dd6d2e08faff 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1872,17 +1872,21 @@ xfs_file_release(
 		return 0;
 
 	/*
-	 * If we can't get the iolock just skip truncating the blocks past EOF
-	 * because we could deadlock with the mmap_lock otherwise. We'll get
-	 * another chance to drop them once the last reference to the inode is
-	 * dropped, so we'll never leak blocks permanently.
+	 * If we can't get the iolock or if the filesystem is frozen, just skip
+	 * truncating the blocks past EOF because we could deadlock with the
+	 * mmap_lock or hang the close() call. We'll get another chance to drop
+	 * them once the last reference to the inode is dropped, so we'll never
+	 * leak blocks permanently.
 	 */
 	if (!xfs_iflags_test(ip, XFS_EOFBLOCKS_RELEASED) &&
-	    xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
-		if (xfs_can_free_eofblocks(ip) &&
-		    !xfs_iflags_test_and_set(ip, XFS_EOFBLOCKS_RELEASED))
-			xfs_free_eofblocks(ip);
-		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+	    sb_start_write_trylock(mp->m_super)) {
+		if (xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
+			if (xfs_can_free_eofblocks(ip) &&
+			    !xfs_iflags_test_and_set(ip, XFS_EOFBLOCKS_RELEASED))
+				xfs_free_eofblocks(ip);
+			xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+		}
+		sb_end_write(mp->m_super);
 	}
 
 	return 0;
-- 
2.47.3