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@xxxxxxxxx>
> 
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@xxxxxxxxx>
> ---
>  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
> 
>