[PATCH 17/38] xfs: convert xfs_reflink_fill_delalloc to use rolling transactions

Dave Chinner <[email protected]>
Newsgroups org.kernel.vger.linux-xfs
Message-ID <[email protected]>
Convert xfs_reflink_fill_delalloc() to use xfs_defer_finish() to
process deferred operations and roll the transaction instead of
cycling the ILOCK with separate commit/alloc pairs for each delalloc
extent conversion.

The previous code dropped and reacquired the ILOCK for every
iteration of the delalloc conversion loop. This left a window where
a racing DIO write to the same range could complete its COW, have IO
completion move the extent from the COW fork to the data fork, and
leave a hole in the COW fork that the original writer would then
trip over.

Using xfs_defer_finish() keeps the ILOCK held across the entire
conversion loop and correctly processes deferred operations (such as
rmap updates) that are generated by xfs_bmapi_write() during each
iteration.

Assisted-by: LLM
Signed-off-by: Dave Chinner <[email protected]>
---
 fs/xfs/xfs_reflink.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 6ca99ddf9976..3e8eb16ce076 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -493,6 +493,7 @@ xfs_reflink_fill_cow_hole(
 
 static int
 xfs_reflink_fill_delalloc(
+	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip,
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
@@ -501,12 +502,12 @@ xfs_reflink_fill_delalloc(
 	bool			convert_now)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*tp;
+	struct xfs_trans	*tp = *tpp;
 	int			nimaps;
 	int			error;
 	bool			found;
 
-	do {
+	if (!tp) {
 		unsigned int	seq_before = READ_ONCE(ip->i_df.if_seq);
 
 		xfs_iunlock(ip, *lockmode);
@@ -535,16 +536,16 @@ xfs_reflink_fill_delalloc(
 			if (error)
 				goto out_trans_cancel;
 		}
+	}
 
+	do {
 		error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
 				&found);
 		if (error || !*shared)
 			goto out_trans_cancel;
 
-		if (found) {
-			xfs_trans_cancel(tp);
+		if (found)
 			break;
-		}
 
 		ASSERT(isnullstartblock(cmap->br_startblock) ||
 		       cmap->br_startblock == DELAYSTARTBLOCK);
@@ -561,15 +562,27 @@ xfs_reflink_fill_delalloc(
 			goto out_trans_cancel;
 
 		xfs_inode_set_cowblocks_tag(ip);
+
+		error = xfs_defer_finish(&tp);
+		if (error)
+			goto out_trans_cancel;
+	} while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
+
+	if (*tpp) {
+		*tpp = tp;
+	} else {
 		error = xfs_trans_commit(tp);
 		if (error)
 			return error;
-	} while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
+	}
 
 	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
 
 out_trans_cancel:
-	xfs_trans_cancel(tp);
+	if (*tpp)
+		*tpp = tp;
+	else
+		xfs_trans_cancel(tp);
 	return error;
 }
 
@@ -622,7 +635,7 @@ xfs_reflink_allocate_cow(
 	ASSERT(!*tpp);
 	if (isnullstartblock(cmap->br_startblock) ||
 	    cmap->br_startblock == DELAYSTARTBLOCK)
-		return xfs_reflink_fill_delalloc(ip, imap, cmap, shared,
+		return xfs_reflink_fill_delalloc(tpp, ip, imap, cmap, shared,
 				lockmode, convert_now);
 
 	/* Shouldn't get here. */
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.