[PATCH v5 0/2] xfs: resolve close() deadlocks on frozen filesystems

[PATCH v5 0/2] xfs: resolve close() deadlocks on frozen filesystems

[Aditya Srivastava]

From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

Hi Carlos and Christoph,

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

Based on Christoph's feedback, I have made the following improvements:
- Added Christoph Hellwig's Reviewed-by tag to Patch 1.
- Wrapped the overly long xfs_trans_alloc line inside xfs_free_eofblocks()
  to conform to Linux line length guidelines.
- Corrected the Patch 2 commit log phrasing to state that we are
  adding the trans_flags parameter rather than renaming it compared
  to upstream.

As requested, I have also submitted the corresponding regression test to
the xfstests mailing list (using tests/xfs/842 with a GPLv2-licensed
helper program). The fstests maintainer (Zorro Lang) reviewed the test
and has indicated that he will wait for this kernel patch to make it
through before merging the test suite additions.

THE REAL-WORLD IMPACT (BUGZILLA & DOWNSTREAM CASES)
==================================================

When speculative post-EOF blocks are closed on XFS, the release path
synchronously attempts to free them via xfs_free_eofblocks(). This
allocates a write transaction (xfs_trans_alloc) which blocks
indefinitely on the superblock freeze write lock (sb_start_intwrite)
under fsfreeze.

This behavior has a long history of causing severe system disruption:
- Downstream Red Hat Bugzilla 1474726 (dating back to 2017) details
  complete system hangs during system backups when rsync and fsfreeze
  are used. Even seemingly harmless read-only commands like
  'cat /var/log/messages' would hang on close() in __sb_start_write
  via xfs_free_eofblocks, requiring a hard reboot.
- Downstream LeApp integration test scenarios consistently hit this hang.

Hanging on close() frequently triggers container healthcheck failures,
systemd service timeouts, and cluster failover cascades, which is
disruptive to user-space applications that view close() as resource
reclamation. No other major Linux filesystem (ext4, btrfs, etc.)
synchronously allocates write transactions during close() system calls.

THE SOLUTION: NON-BLOCKING SUPERBLOCK TRYLOCK
=============================================

Instead of performing racy pre-checks, this series introduces
XFS_TRANS_WRITECOUNT_TRYLOCK. When specified, __xfs_trans_alloc()
attempts to obtain freeze protection using sb_start_intwrite_trylock().
If that fails, it aborts allocation gracefully and returns -EAGAIN.

We then pass XFS_TRANS_WRITECOUNT_TRYLOCK during xfs_file_release(). If
the truncation fails due to a frozen filesystem (-EAGAIN), we cleanly
bypass setting XFS_EOFBLOCKS_RELEASED on the inode, ensuring subsequent
releases or the background blockgc garbage collector can successfully
clean them up once thawed.

REPRODUCER DETAILS (GPLV2 LICENSED)
===================================

As requested, I have added a GPLv2-compatible license to the C
reproducer provided below, and I have also sent a corresponding patch to
the xfstests mailing list. The fstests maintainer (Zorro Lang) reviewed
the patch and indicated that he will wait for this kernel patch series
to be merged before pulling the test suite additions.

Compile with -pthread:

    /*
     * GPLv2-compatible XFS freeze close() hang reproducer.
     * Copyright (c) 2026 Aditya Prakash Srivastava. All Rights Reserved.
     */
    #define _GNU_SOURCE
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <unistd.h>
    #include <fcntl.h>
    #include <pthread.h>
    #include <sys/ioctl.h>
    #include <sys/vfs.h>
    #include <linux/fs.h>
    #include <libgen.h>

    volatile int close_started = 0;
    volatile int close_completed = 0;

    void *close_thread(void *arg) {
        int fd = *(int *)arg;
        close_started = 1;
        close(fd);
        close_completed = 1;
        return NULL;
    }

    int main(int argc, char *argv[]) {
        struct statfs sfs;
        if (statfs(argv[1], &sfs) < 0) {
            char *dir_buf = strdup(argv[1]);
            char *parent_dir = dirname(dir_buf);
            if (statfs(parent_dir, &sfs) < 0) {
                perror("statfs");
                free(dir_buf);
                return 1;
            }
            free(dir_buf);
        }
        if (sfs.f_type != 0x58465342) return 1;

        int freeze_fd = open(dirname(strdup(argv[1])), O_RDONLY);
        int write_fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC, 0644);
        char buf[65536] = {0};
        for (int i = 0; i < 320; i++) write(write_fd, buf, sizeof(buf));

        ioctl(freeze_fd, FIFREEZE, 0);
        pthread_t thread;
        pthread_create(&thread, NULL, close_thread, &write_fd);
        while (!close_started) usleep(1000);
        usleep(1000000); // Wait 1s
        if (!close_completed) printf("SUCCESS: close() hung!\n");
        ioctl(freeze_fd, FITHAW, 0);
        pthread_join(thread, NULL);
        unlink(argv[1]);
        return 0;
    }

Link: https://bugzilla.kernel.org/show_bug.cgi?id=205833
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1474726

Aditya Prakash Srivastava (2):
  xfs: add a XFS_TRANS_WRITECOUNT_TRYLOCK flag
  xfs: prevent close() from hanging on frozen filesystems

 fs/xfs/libxfs/xfs_shared.h |  3 +++
 fs/xfs/xfs_bmap_util.c     | 10 ++++++----
 fs/xfs/xfs_bmap_util.h     |  2 +-
 fs/xfs/xfs_file.c          |  8 +++++---
 fs/xfs/xfs_icache.c        |  2 +-
 fs/xfs/xfs_inode.c         |  2 +-
 fs/xfs/xfs_trans.c         | 12 +++++++++++-
 7 files changed, 28 insertions(+), 11 deletions(-)

-- 
2.47.3

[PATCH v5 1/2] xfs: add a XFS_TRANS_WRITECOUNT_TRYLOCK flag

[Aditya Srivastava]

From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

Introduce a new transaction allocation flag, XFS_TRANS_WRITECOUNT_TRYLOCK.
When this flag is specified, __xfs_trans_alloc() attempts to obtain
freeze protection using sb_start_intwrite_trylock() instead of blocking
indefinitely on sb_start_intwrite().

If the trylock fails, the allocation is aborted gracefully: the freshly
allocated transaction handle is freed, and the function returns the
appropriate error pointer ERR_PTR(-EAGAIN), which is then propagated
to the caller by xfs_trans_alloc().

Also add an assertion in __xfs_trans_alloc() to ensure that both
XFS_TRANS_NO_WRITECOUNT and XFS_TRANS_WRITECOUNT_TRYLOCK are never
specified at the same time, as they are mutually exclusive.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
 fs/xfs/libxfs/xfs_shared.h |  3 +++
 fs/xfs/xfs_trans.c         | 12 +++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
index b1e0d9bc1f7d..68d22b6cddd3 100644
--- a/fs/xfs/libxfs/xfs_shared.h
+++ b/fs/xfs/libxfs/xfs_shared.h
@@ -164,6 +164,9 @@ void	xfs_log_get_max_trans_res(struct xfs_mount *mp,
 /* Transaction has locked the rtbitmap and rtsum inodes */
 #define XFS_TRANS_RTBITMAP_LOCKED	(1u << 9)
 
+/* Try lock filesystem superblock for freeze protection */
+#define XFS_TRANS_WRITECOUNT_TRYLOCK	(1u << 10)
+
 /*
  * Field values for xfs_trans_mod_sb.
  */
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 148cc32449c1..3860e44d6439 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -216,10 +216,18 @@ __xfs_trans_alloc(
 	struct xfs_trans	*tp;
 
 	ASSERT(!(flags & XFS_TRANS_RES_FDBLKS) || xfs_has_lazysbcount(mp));
+	ASSERT(!((flags & XFS_TRANS_NO_WRITECOUNT) &&
+		 (flags & XFS_TRANS_WRITECOUNT_TRYLOCK)));
 
 	tp = kmem_cache_zalloc(xfs_trans_cache, GFP_KERNEL | __GFP_NOFAIL);
-	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
+	if (flags & XFS_TRANS_WRITECOUNT_TRYLOCK) {
+		if (!sb_start_intwrite_trylock(mp->m_super)) {
+			kmem_cache_free(xfs_trans_cache, tp);
+			return ERR_PTR(-EAGAIN);
+		}
+	} else if (!(flags & XFS_TRANS_NO_WRITECOUNT)) {
 		sb_start_intwrite(mp->m_super);
+	}
 	xfs_trans_set_context(tp);
 	tp->t_flags = flags;
 	tp->t_mountp = mp;
@@ -252,6 +260,8 @@ xfs_trans_alloc(
 	 */
 retry:
 	tp = __xfs_trans_alloc(mp, flags);
+	if (IS_ERR(tp))
+		return PTR_ERR(tp);
 	WARN_ON(mp->m_super->s_writers.frozen == SB_FREEZE_COMPLETE);
 	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
 	if (error == -ENOSPC && want_retry) {
-- 
2.47.3

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

[Aditya Srivastava]

From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>

When a file with active speculative post-EOF preallocations is closed,
xfs_file_release() synchronously triggers xfs_free_eofblocks() to clean
them up. This requires allocating a write transaction (xfs_trans_alloc),
which blocks indefinitely if the filesystem is currently frozen or in the
process of freezing, as it waits to acquire the superblock's write lock.

As a result, a close() system call on a read-write file descriptor can
hang indefinitely in percpu_rwsem_wait() until the filesystem is thawed,
even if the file is closed by a non-writer process or after all writing
activity has already ceased.

To fix this properly and avoid any potential race conditions where a freeze
might come in immediately after a writable check, pass the new
XFS_TRANS_WRITECOUNT_TRYLOCK flag to xfs_trans_alloc() when freeing
speculative preallocations in xfs_file_release().

If xfs_free_eofblocks() returns -EAGAIN on a trylock failure, we cleanly
bypass setting XFS_EOFBLOCKS_RELEASED on the inode, ensuring subsequent
releases or the background blockgc garbage collector can successfully retry
the cleanup once the filesystem thaws.

Also, add the new trans_flags parameter to xfs_free_eofblocks() to make
its usage stand out, and update existing callers to pass 0 to preserve
standard blocking paths.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=205833
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1474726
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
 fs/xfs/xfs_bmap_util.c | 10 ++++++----
 fs/xfs/xfs_bmap_util.h |  2 +-
 fs/xfs/xfs_file.c      |  8 +++++---
 fs/xfs/xfs_icache.c    |  2 +-
 fs/xfs/xfs_inode.c     |  2 +-
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 0ab00615f1ad..a99aae4a1631 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -574,7 +574,8 @@ xfs_can_free_eofblocks(
  */
 int
 xfs_free_eofblocks(
-	struct xfs_inode	*ip)
+	struct xfs_inode	*ip,
+	uint			trans_flags)
 {
 	struct xfs_trans	*tp;
 	struct xfs_mount	*mp = ip->i_mount;
@@ -604,9 +605,10 @@ xfs_free_eofblocks(
 		return 0;
 	}
 
-	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
+	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0,
+				trans_flags, &tp);
 	if (error) {
-		ASSERT(xfs_is_shutdown(mp));
+		ASSERT(error == -EAGAIN || xfs_is_shutdown(mp));
 		return error;
 	}
 
@@ -928,7 +930,7 @@ xfs_prepare_shift(
 	 * into the accessible region of the file.
 	 */
 	if (xfs_can_free_eofblocks(ip)) {
-		error = xfs_free_eofblocks(ip);
+		error = xfs_free_eofblocks(ip, 0);
 		if (error)
 			return error;
 	}
diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index c477b3361630..c13774aa0892 100644
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -66,7 +66,7 @@ int	xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
 
 /* EOF block manipulation functions */
 bool	xfs_can_free_eofblocks(struct xfs_inode *ip);
-int	xfs_free_eofblocks(struct xfs_inode *ip);
+int	xfs_free_eofblocks(struct xfs_inode *ip, uint trans_flags);
 
 int	xfs_swap_extents(struct xfs_inode *ip, struct xfs_inode *tip,
 			 struct xfs_swapext *sx);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 845a97c9b063..76c9b2fe7c51 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1806,9 +1806,11 @@ xfs_file_release(
 	 */
 	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);
+		if (!xfs_iflags_test(ip, XFS_EOFBLOCKS_RELEASED) &&
+		    xfs_can_free_eofblocks(ip) &&
+		    !xfs_free_eofblocks(ip, XFS_TRANS_WRITECOUNT_TRYLOCK))
+			xfs_iflags_set(ip, XFS_EOFBLOCKS_RELEASED);
+
 		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
 	}
 
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 2040a9292ee6..c575b4acb24c 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1259,7 +1259,7 @@ xfs_inode_free_eofblocks(
 	*lockflags |= XFS_IOLOCK_EXCL;
 
 	if (xfs_can_free_eofblocks(ip))
-		return xfs_free_eofblocks(ip);
+		return xfs_free_eofblocks(ip, 0);
 
 	/* inode could be preallocated */
 	trace_xfs_inode_free_eofblocks_invalid(ip);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index ddf2707c8894..14d3cd04a79f 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1423,7 +1423,7 @@ xfs_inactive(
 		 * reference to the inode at this point anyways.
 		 */
 		if (xfs_can_free_eofblocks(ip))
-			error = xfs_free_eofblocks(ip);
+			error = xfs_free_eofblocks(ip, 0);
 
 		goto out;
 	}
-- 
2.47.3

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

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Darrick J. Wong]

On Tue, Jun 16, 2026 at 05:38:50AM +0000, Aditya Srivastava wrote:
> From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> 
> When a file with active speculative post-EOF preallocations is closed,
> xfs_file_release() synchronously triggers xfs_free_eofblocks() to clean
> them up. This requires allocating a write transaction (xfs_trans_alloc),
> which blocks indefinitely if the filesystem is currently frozen or in the
> process of freezing, as it waits to acquire the superblock's write lock.
> 
> As a result, a close() system call on a read-write file descriptor can
> hang indefinitely in percpu_rwsem_wait() until the filesystem is thawed,
> even if the file is closed by a non-writer process or after all writing
> activity has already ceased.
> 
> To fix this properly and avoid any potential race conditions where a freeze
> might come in immediately after a writable check, pass the new
> XFS_TRANS_WRITECOUNT_TRYLOCK flag to xfs_trans_alloc() when freeing
> speculative preallocations in xfs_file_release().
> 
> If xfs_free_eofblocks() returns -EAGAIN on a trylock failure, we cleanly
> bypass setting XFS_EOFBLOCKS_RELEASED on the inode, ensuring subsequent
> releases or the background blockgc garbage collector can successfully retry
> the cleanup once the filesystem thaws.
> 
> Also, add the new trans_flags parameter to xfs_free_eofblocks() to make
> its usage stand out, and update existing callers to pass 0 to preserve
> standard blocking paths.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=205833
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1474726
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> ---
>  fs/xfs/xfs_bmap_util.c | 10 ++++++----
>  fs/xfs/xfs_bmap_util.h |  2 +-
>  fs/xfs/xfs_file.c      |  8 +++++---
>  fs/xfs/xfs_icache.c    |  2 +-
>  fs/xfs/xfs_inode.c     |  2 +-
>  5 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 0ab00615f1ad..a99aae4a1631 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -574,7 +574,8 @@ xfs_can_free_eofblocks(
>   */
>  int
>  xfs_free_eofblocks(
> -	struct xfs_inode	*ip)
> +	struct xfs_inode	*ip,
> +	uint			trans_flags)
>  {
>  	struct xfs_trans	*tp;
>  	struct xfs_mount	*mp = ip->i_mount;
> @@ -604,9 +605,10 @@ xfs_free_eofblocks(
>  		return 0;
>  	}
>  
> -	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0,
> +				trans_flags, &tp);
>  	if (error) {
> -		ASSERT(xfs_is_shutdown(mp));
> +		ASSERT(error == -EAGAIN || xfs_is_shutdown(mp));
>  		return error;
>  	}
>  
> @@ -928,7 +930,7 @@ xfs_prepare_shift(
>  	 * into the accessible region of the file.
>  	 */
>  	if (xfs_can_free_eofblocks(ip)) {
> -		error = xfs_free_eofblocks(ip);
> +		error = xfs_free_eofblocks(ip, 0);
>  		if (error)
>  			return error;
>  	}
> diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
> index c477b3361630..c13774aa0892 100644
> --- a/fs/xfs/xfs_bmap_util.h
> +++ b/fs/xfs/xfs_bmap_util.h
> @@ -66,7 +66,7 @@ int	xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
>  
>  /* EOF block manipulation functions */
>  bool	xfs_can_free_eofblocks(struct xfs_inode *ip);
> -int	xfs_free_eofblocks(struct xfs_inode *ip);
> +int	xfs_free_eofblocks(struct xfs_inode *ip, uint trans_flags);
>  
>  int	xfs_swap_extents(struct xfs_inode *ip, struct xfs_inode *tip,
>  			 struct xfs_swapext *sx);
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 845a97c9b063..76c9b2fe7c51 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1806,9 +1806,11 @@ xfs_file_release(
>  	 */
>  	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);
> +		if (!xfs_iflags_test(ip, XFS_EOFBLOCKS_RELEASED) &&
> +		    xfs_can_free_eofblocks(ip) &&
> +		    !xfs_free_eofblocks(ip, XFS_TRANS_WRITECOUNT_TRYLOCK))
> +			xfs_iflags_set(ip, XFS_EOFBLOCKS_RELEASED);

Could you prevent the close() stalls by surrounding this with
sb_start_write_trylock instead of passing transaction allocation flags
all the way down?

OFC that results in a messy if test:

		if (xfs_can_free_eofblocks(...) &&
		    !xfs_iflags_test(...RELEASED) &&
		    !sb_start_write_trylock(...)) {
			if (!xfs_iflags_test_and_set(...))
				xfs_free_eofblocks(ip);
			sb_end_write(...);
		}

<shrug> Sorry if this is noise, I've been on vacation.

--D

> +
>  		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
>  	}
>  
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 2040a9292ee6..c575b4acb24c 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1259,7 +1259,7 @@ xfs_inode_free_eofblocks(
>  	*lockflags |= XFS_IOLOCK_EXCL;
>  
>  	if (xfs_can_free_eofblocks(ip))
> -		return xfs_free_eofblocks(ip);
> +		return xfs_free_eofblocks(ip, 0);
>  
>  	/* inode could be preallocated */
>  	trace_xfs_inode_free_eofblocks_invalid(ip);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index ddf2707c8894..14d3cd04a79f 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1423,7 +1423,7 @@ xfs_inactive(
>  		 * reference to the inode at this point anyways.
>  		 */
>  		if (xfs_can_free_eofblocks(ip))
> -			error = xfs_free_eofblocks(ip);
> +			error = xfs_free_eofblocks(ip, 0);
>  
>  		goto out;
>  	}
> -- 
> 2.47.3
> 
> 

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

[Aditya Prakash Srivastava]

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

On Wed, Jun 24, 2026 at 11:05 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Tue, Jun 16, 2026 at 05:38:50AM +0000, Aditya Srivastava wrote:
> > From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> >
> > When a file with active speculative post-EOF preallocations is closed,
> > xfs_file_release() synchronously triggers xfs_free_eofblocks() to clean
> > them up. This requires allocating a write transaction (xfs_trans_alloc),
> > which blocks indefinitely if the filesystem is currently frozen or in the
> > process of freezing, as it waits to acquire the superblock's write lock.
> >
> > As a result, a close() system call on a read-write file descriptor can
> > hang indefinitely in percpu_rwsem_wait() until the filesystem is thawed,
> > even if the file is closed by a non-writer process or after all writing
> > activity has already ceased.
> >
> > To fix this properly and avoid any potential race conditions where a freeze
> > might come in immediately after a writable check, pass the new
> > XFS_TRANS_WRITECOUNT_TRYLOCK flag to xfs_trans_alloc() when freeing
> > speculative preallocations in xfs_file_release().
> >
> > If xfs_free_eofblocks() returns -EAGAIN on a trylock failure, we cleanly
> > bypass setting XFS_EOFBLOCKS_RELEASED on the inode, ensuring subsequent
> > releases or the background blockgc garbage collector can successfully retry
> > the cleanup once the filesystem thaws.
> >
> > Also, add the new trans_flags parameter to xfs_free_eofblocks() to make
> > its usage stand out, and update existing callers to pass 0 to preserve
> > standard blocking paths.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=205833
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1474726
> > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> > ---
> >  fs/xfs/xfs_bmap_util.c | 10 ++++++----
> >  fs/xfs/xfs_bmap_util.h |  2 +-
> >  fs/xfs/xfs_file.c      |  8 +++++---
> >  fs/xfs/xfs_icache.c    |  2 +-
> >  fs/xfs/xfs_inode.c     |  2 +-
> >  5 files changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> > index 0ab00615f1ad..a99aae4a1631 100644
> > --- a/fs/xfs/xfs_bmap_util.c
> > +++ b/fs/xfs/xfs_bmap_util.c
> > @@ -574,7 +574,8 @@ xfs_can_free_eofblocks(
> >   */
> >  int
> >  xfs_free_eofblocks(
> > -     struct xfs_inode        *ip)
> > +     struct xfs_inode        *ip,
> > +     uint                    trans_flags)
> >  {
> >       struct xfs_trans        *tp;
> >       struct xfs_mount        *mp = ip->i_mount;
> > @@ -604,9 +605,10 @@ xfs_free_eofblocks(
> >               return 0;
> >       }
> >
> > -     error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
> > +     error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0,
> > +                             trans_flags, &tp);
> >       if (error) {
> > -             ASSERT(xfs_is_shutdown(mp));
> > +             ASSERT(error == -EAGAIN || xfs_is_shutdown(mp));
> >               return error;
> >       }
> >
> > @@ -928,7 +930,7 @@ xfs_prepare_shift(
> >        * into the accessible region of the file.
> >        */
> >       if (xfs_can_free_eofblocks(ip)) {
> > -             error = xfs_free_eofblocks(ip);
> > +             error = xfs_free_eofblocks(ip, 0);
> >               if (error)
> >                       return error;
> >       }
> > diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
> > index c477b3361630..c13774aa0892 100644
> > --- a/fs/xfs/xfs_bmap_util.h
> > +++ b/fs/xfs/xfs_bmap_util.h
> > @@ -66,7 +66,7 @@ int xfs_insert_file_space(struct xfs_inode *, xfs_off_t offset,
> >
> >  /* EOF block manipulation functions */
> >  bool xfs_can_free_eofblocks(struct xfs_inode *ip);
> > -int  xfs_free_eofblocks(struct xfs_inode *ip);
> > +int  xfs_free_eofblocks(struct xfs_inode *ip, uint trans_flags);
> >
> >  int  xfs_swap_extents(struct xfs_inode *ip, struct xfs_inode *tip,
> >                        struct xfs_swapext *sx);
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 845a97c9b063..76c9b2fe7c51 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -1806,9 +1806,11 @@ xfs_file_release(
> >        */
> >       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);
> > +             if (!xfs_iflags_test(ip, XFS_EOFBLOCKS_RELEASED) &&
> > +                 xfs_can_free_eofblocks(ip) &&
> > +                 !xfs_free_eofblocks(ip, XFS_TRANS_WRITECOUNT_TRYLOCK))
> > +                     xfs_iflags_set(ip, XFS_EOFBLOCKS_RELEASED);
>
> Could you prevent the close() stalls by surrounding this with
> sb_start_write_trylock instead of passing transaction allocation flags
> all the way down?
>
> OFC that results in a messy if test:
>
>                 if (xfs_can_free_eofblocks(...) &&
>                     !xfs_iflags_test(...RELEASED) &&
>                     !sb_start_write_trylock(...)) {
>                         if (!xfs_iflags_test_and_set(...))
>                                 xfs_free_eofblocks(ip);
>                         sb_end_write(...);
>                 }
>
> <shrug> Sorry if this is noise, I've been on vacation.
>
> --D
>
> > +
> >               xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> >       }
> >
> > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > index 2040a9292ee6..c575b4acb24c 100644
> > --- a/fs/xfs/xfs_icache.c
> > +++ b/fs/xfs/xfs_icache.c
> > @@ -1259,7 +1259,7 @@ xfs_inode_free_eofblocks(
> >       *lockflags |= XFS_IOLOCK_EXCL;
> >
> >       if (xfs_can_free_eofblocks(ip))
> > -             return xfs_free_eofblocks(ip);
> > +             return xfs_free_eofblocks(ip, 0);
> >
> >       /* inode could be preallocated */
> >       trace_xfs_inode_free_eofblocks_invalid(ip);
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index ddf2707c8894..14d3cd04a79f 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -1423,7 +1423,7 @@ xfs_inactive(
> >                * reference to the inode at this point anyways.
> >                */
> >               if (xfs_can_free_eofblocks(ip))
> > -                     error = xfs_free_eofblocks(ip);
> > +                     error = xfs_free_eofblocks(ip, 0);
> >
> >               goto out;
> >       }
> > --
> > 2.47.3
> >
> >

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@gmail.com> 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

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

[Aditya Prakash Srivastava]

On Wed, Jul 8, 2026 at 6:49 PM Aditya Prakash Srivastava
<aditya.ansh182@gmail.com> wrote:
> 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

Hi,

Are there any remaining concerns or feedback that need
to be addressed with this patch?

Regards,
Aditya

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

[Christoph Hellwig]

On Thu, Aug 13, 2026 at 09:43:44PM +0530, Aditya Prakash Srivastava wrote:
> 
> Are there any remaining concerns or feedback that need
> to be addressed with this patch?

Still looks good to me, but I think it just go lost.  It's a bit close
for the 7.3 merge window now, but I'd suggest you resend it in a week
or two to get it onto the agenda for 7.4 early.

> 

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

[Carlos Maiolino]

On Fri, Aug 14, 2026 at 12:28:19AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 13, 2026 at 09:43:44PM +0530, Aditya Prakash Srivastava wrote:
> > 
> > Are there any remaining concerns or feedback that need
> > to be addressed with this patch?
> 
> Still looks good to me, but I think it just go lost.  It's a bit close
> for the 7.3 merge window now, but I'd suggest you resend it in a week
> or two to get it onto the agenda for 7.4 early.

Yup, I was waiting for Darrick to follow-up then it fell on limbo.
Please follow Christoph's advice, please rebase and re-send on top of
the tree after 7.3 merge window.

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

[Darrick J. Wong]

On Fri, Aug 14, 2026 at 06:25:41PM +0200, Carlos Maiolino wrote:
> On Fri, Aug 14, 2026 at 12:28:19AM -0700, Christoph Hellwig wrote:
> > On Thu, Aug 13, 2026 at 09:43:44PM +0530, Aditya Prakash Srivastava wrote:
> > > 
> > > Are there any remaining concerns or feedback that need
> > > to be addressed with this patch?
> > 
> > Still looks good to me, but I think it just go lost.  It's a bit close
> > for the 7.3 merge window now, but I'd suggest you resend it in a week
> > or two to get it onto the agenda for 7.4 early.
> 
> Yup, I was waiting for Darrick to follow-up then it fell on limbo.
> Please follow Christoph's advice, please rebase and re-send on top of
> the tree after 7.3 merge window.

I ignored the response because it's clearly AI slop, and obviously
wrong.

"In XFS, the canonical locking hierarchy is:

"sb_start_write (freeze lock) -> IOLOCK -> ILOCK ->
xfs_trans_alloc"

Nope.

"xfs_file_release() already holds XFS_IOLOCK_EXCL (acquired via
xfs_ilock_nowait())..."

Nope.  The snippet I suggested isn't fully formed, but this is obviously
wrong; go read xfs_file_release.

static inline void xfs_file_release_eofblocks(struct xfs_inode *ip)
{
	if (!xfs_can_free_eofblocks(ip))
		return;
	if (xfs_iflags_test(ip, XFS_EOFBLOCKS_RELEASED))
		return;

	/* take freeze protection and i_rwsem */
	if (!sb_start_write_trylock(...))
		return;
	if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
		goto out_sb_write;

	/*
	 * recheck if we want to trim eofblocks and someone else hasn't
	 * already done this
	 */
	if (!xfs_can_free_eofblocks(ip))
		goto out_iolock;
	if (xfs_iflags_test_and_set(ip, XFS_EOFBLOCKS_RELEASED))
		goto out_iolock;

	xfs_free_eofblocks(ip);

out_iolock:
	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
out_sb_write:
	sb_end_write(...);
}

"xfs_trans_alloc() is the canonical, centralized place in XFS to
manage write-count blocking semantics."

Nope.  This isn't even talking about the same vfs lock here.

--D