[PATCH] xfs: resample the data fork mapping after cycling ILOCK

[PATCH] xfs: resample the data fork mapping after cycling ILOCK

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

xfs_reflink_fill_{cow_hole,delalloc} are both presented with an inode,
a data fork mapping, and a cow fork mapping.  Unfortunately, these two
helpers cycle the ILOCK to grab a transaction, which means that the
mappings are stale as soon as we reacquire the ILOCK.  Currently we
refresh the cow fork mapping by re-calling xfs_find_trim_cow_extent, but
we don't refresh the data fork mapping beforehand, which means that the
xfs_bmap_trim_cow in that function queries the refcount btree about the
wrong physical blocks and returns an inaccurate value in *shared.

If *shared is now false, the directio write proceeds with a stale data
fork mapping.  Fix this by querying the data fork mapping if the
sequence counter changes across the ILOCK cycle.

Cc: <hch@lst.de>
Cc: <stable@vger.kernel.org> # v4.11
Fixes: 3c68d44a2b49a0 ("xfs: allocate direct I/O COW blocks in iomap_begin")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_reflink.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index d81510912be58b..489ecd4cf10aff 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -440,6 +440,7 @@ xfs_reflink_fill_cow_hole(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp;
 	xfs_filblks_t		resaligned;
+	unsigned int		seq_before = READ_ONCE(ip->i_df.if_seq);
 	unsigned int		dblocks = 0, rblocks = 0;
 	int			nimaps;
 	int			error;
@@ -465,6 +466,22 @@ xfs_reflink_fill_cow_hole(
 
 	*lockmode = XFS_ILOCK_EXCL;
 
+	/*
+	 * The data fork mapping may have changed while we dropped the ILOCK
+	 * (a racing O_DIRECT writer under IOLOCK_SHARED can complete a full
+	 * CoW cycle including xfs_reflink_end_cow(), which remaps this offset
+	 * and drops the refcount of the old shared block).  Re-read it so the
+	 * shared-status recheck below and the caller's in-place iomap both
+	 * operate on the current mapping rather than a stale physical block.
+	 */
+	if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
+		nimaps = 1;
+		error = xfs_bmapi_read(ip, imap->br_startoff,
+				imap->br_blockcount, imap, &nimaps, 0);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
 	if (error || !*shared)
 		goto out_trans_cancel;
@@ -511,6 +528,8 @@ xfs_reflink_fill_delalloc(
 	bool			found;
 
 	do {
+		unsigned int	seq_before = READ_ONCE(ip->i_df.if_seq);
+
 		xfs_iunlock(ip, *lockmode);
 		*lockmode = 0;
 
@@ -521,6 +540,23 @@ xfs_reflink_fill_delalloc(
 
 		*lockmode = XFS_ILOCK_EXCL;
 
+		/*
+		 * The data fork mapping may have changed while we dropped the
+		 * ILOCK (a racing O_DIRECT writer under IOLOCK_SHARED can
+		 * complete a full CoW cycle including xfs_reflink_end_cow(),
+		 * which remaps this offset and drops the refcount of the old
+		 * shared block).  Re-read it so the shared-status recheck
+		 * below and the caller's in-place iomap both operate on the
+		 * current mapping rather than a stale physical block.
+		 */
+		if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
+			nimaps = 1;
+			error = xfs_bmapi_read(ip, imap->br_startoff,
+					imap->br_blockcount, imap, &nimaps, 0);
+			if (error)
+				goto out_trans_cancel;
+		}
+
 		error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
 				&found);
 		if (error || !*shared)

Re: [PATCH] xfs: resample the data fork mapping after cycling ILOCK

[Christoph Hellwig]

Looks good:

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

Re: [PATCH] xfs: resample the data fork mapping after cycling ILOCK

[Carlos Maiolino]

On Mon, Jul 13, 2026 at 11:03:44PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xfs_reflink_fill_{cow_hole,delalloc} are both presented with an inode,
> a data fork mapping, and a cow fork mapping.  Unfortunately, these two
> helpers cycle the ILOCK to grab a transaction, which means that the
> mappings are stale as soon as we reacquire the ILOCK.  Currently we
> refresh the cow fork mapping by re-calling xfs_find_trim_cow_extent, but
> we don't refresh the data fork mapping beforehand, which means that the
> xfs_bmap_trim_cow in that function queries the refcount btree about the
> wrong physical blocks and returns an inaccurate value in *shared.
> 
> If *shared is now false, the directio write proceeds with a stale data
> fork mapping.  Fix this by querying the data fork mapping if the
> sequence counter changes across the ILOCK cycle.
> 
> Cc: <hch@lst.de>
> Cc: <stable@vger.kernel.org> # v4.11
> Fixes: 3c68d44a2b49a0 ("xfs: allocate direct I/O COW blocks in iomap_begin")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_reflink.c |   36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index d81510912be58b..489ecd4cf10aff 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -440,6 +440,7 @@ xfs_reflink_fill_cow_hole(
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_trans	*tp;
>  	xfs_filblks_t		resaligned;
> +	unsigned int		seq_before = READ_ONCE(ip->i_df.if_seq);
>  	unsigned int		dblocks = 0, rblocks = 0;
>  	int			nimaps;
>  	int			error;
> @@ -465,6 +466,22 @@ xfs_reflink_fill_cow_hole(
>  
>  	*lockmode = XFS_ILOCK_EXCL;
>  
> +	/*
> +	 * The data fork mapping may have changed while we dropped the ILOCK
> +	 * (a racing O_DIRECT writer under IOLOCK_SHARED can complete a full
> +	 * CoW cycle including xfs_reflink_end_cow(), which remaps this offset
> +	 * and drops the refcount of the old shared block).  Re-read it so the
> +	 * shared-status recheck below and the caller's in-place iomap both
> +	 * operate on the current mapping rather than a stale physical block.
> +	 */
> +	if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
> +		nimaps = 1;
> +		error = xfs_bmapi_read(ip, imap->br_startoff,
> +				imap->br_blockcount, imap, &nimaps, 0);
> +		if (error)
> +			goto out_trans_cancel;
> +	}
> +
>  	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
>  	if (error || !*shared)
>  		goto out_trans_cancel;
> @@ -511,6 +528,8 @@ xfs_reflink_fill_delalloc(
>  	bool			found;
>  
>  	do {
> +		unsigned int	seq_before = READ_ONCE(ip->i_df.if_seq);
> +
>  		xfs_iunlock(ip, *lockmode);
>  		*lockmode = 0;
>  
> @@ -521,6 +540,23 @@ xfs_reflink_fill_delalloc(
>  
>  		*lockmode = XFS_ILOCK_EXCL;
>  
> +		/*
> +		 * The data fork mapping may have changed while we dropped the
> +		 * ILOCK (a racing O_DIRECT writer under IOLOCK_SHARED can
> +		 * complete a full CoW cycle including xfs_reflink_end_cow(),
> +		 * which remaps this offset and drops the refcount of the old
> +		 * shared block).  Re-read it so the shared-status recheck
> +		 * below and the caller's in-place iomap both operate on the
> +		 * current mapping rather than a stale physical block.
> +		 */
> +		if (seq_before != READ_ONCE(ip->i_df.if_seq)) {
> +			nimaps = 1;
> +			error = xfs_bmapi_read(ip, imap->br_startoff,
> +					imap->br_blockcount, imap, &nimaps, 0);
> +			if (error)
> +				goto out_trans_cancel;
> +		}
> +
>  		error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
>  				&found);
>  		if (error || !*shared)
>