Linux XFS filesystem development
 help / color / mirror / Atom feed
From: Dave Chinner <dgc@kernel.org>
To: linux-xfs@vger.kernel.org
Cc: cem@kernel.org
Subject: [PATCH 31/33] xfs: clean up xfs_direct_write_cow_iomap_begin after restructure
Date: Wed, 29 Jul 2026 20:02:15 +1000	[thread overview]
Message-ID: <20260729100629.1943710-32-dgc@kernel.org> (raw)
In-Reply-To: <20260729100629.1943710-1-dgc@kernel.org>

Now that xfs_direct_write_iomap_begin() handles all locking,
transaction allocation, extent reading, and retry logic, clean up
xfs_direct_write_cow_iomap_begin() to remove the dead transitional
code.

Remove the needs_tp parameter and all the internal lock management,
transaction allocation, retry loop, and local_tp handling. The
function now assumes the caller holds the ILOCK and has already
determined that COW is needed via imap_needs_cow().

Lift the imap_needs_cow() check from cow_iomap_begin up to
xfs_direct_write_iomap_begin(), replacing the xfs_is_cow_inode()
check after the xfs_bmapi_read(). This is safe because
imap_needs_cow() checks xfs_is_cow_inode() internally as its first
test.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c | 115 +++++++--------------------------------------
 1 file changed, 16 insertions(+), 99 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index d58ecc05a4d1..4b2860e80d9f 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -892,116 +892,44 @@ xfs_bmap_hw_atomic_write_possible(
 
 /*
  * Handle COW extent allocation and iomap setup for direct writes to reflinked
- * files.
+ * files. The caller holds the ILOCK and has already determined that COW is
+ * needed via imap_needs_cow().
  *
- * Transitional tpp handling:
- *	!tpp = do everything internally using local_tp
- *	tpp & !*tpp = caller did locking, wants -EAGAIN if transaction required
- *	tpp && *tpp = caller did locking and transaction allocation
+ * On return, args->nimaps indicates what the caller should do next:
+ *   nimaps == 0: COW was handled, iomap/srcmap are filled in.
+ *                Caller should commit args->tp, unlock, and return.
+ *   nimaps > 0:  Extent is not shared. Caller should continue with the
+ *                normal IO path using the imap.
  *
- * The caller passes in an imap and nimaps that the COW allocation will fill
- * with the data fork extent mapping. On return, *nimaps indicates whether the
- * caller needs to continue with the normal IO path:
- *
- *   *nimaps == 0: COW was handled, iomap/srcmap are filled in, ILOCK released.
- *                 Caller should return 0 immediately.
- *   *nimaps > 0:  Extent is not shared, imap is valid, ILOCK is still held.
- *                 Caller should continue with the normal IO path.
+ * Returns -EAGAIN if a transaction is needed but args->tp is NULL.
  */
 static int
 xfs_direct_write_cow_iomap_begin(
-	struct xfs_direct_write_args *args,
-	bool			needs_tp)
+	struct xfs_direct_write_args *args)
 {
 	struct xfs_inode	*ip = args->ip;
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*local_tp = NULL;
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, args->offset);
 	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, args->offset,
 					args->length);
 	int			error;
 	u64			seq;
 
-	if (needs_tp) {
-		args->lockmode = XFS_ILOCK_EXCL;
-
-		error = xfs_ilock_for_iomap(ip, args->flags, &args->lockmode);
-		if (error)
-			return error;
-
-retry:
-		args->nimaps = 1;
-		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
-				&args->imap, &args->nimaps, 0);
-		if (error)
-			goto out_unlock;
-	}
-
-	if (!imap_needs_cow(ip, args->flags, &args->imap, args->nimaps)) {
-		if (local_tp) {
-			xfs_trans_cancel(local_tp);
-			args->tp = NULL;
-		}
-		return 0;
-	}
-
-	error = -EAGAIN;
-	if (args->flags & IOMAP_NOWAIT)
-		goto out_unlock;
-
 	error = xfs_reflink_allocate_cow(args);
-	if (error == -EAGAIN) {
-		ASSERT(!args->tp);
-
-		if (!needs_tp)
-			goto out_unlock;
-
-		/*
-		 * Handle the retry internally. Drop the ILOCK and allocate a
-		 * zero-block reservation transaction, which will re-acquire
-		 * the ILOCK. We cannot determine what extent type will be
-		 * found once we've regained the ILOCK, so the callees will
-		 * use xfs_trans_reserve_more_inode() directly to reserve any
-		 * blocks they require before they start modifications. This
-		 * allows ENOSPC to be returned and the transaction cancelled
-		 * safely if the block reservation cannot be made.
-		 *
-		 * Retry the imap lookup since the extent tree may have changed
-		 * while the ILOCK was not held.
-		 */
-		xfs_iunlock(ip, args->lockmode);
-
-		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
-				0, 0, false, &args->tp);
-		if (error)
-			return error;
-		local_tp = args->tp;
-
-		goto retry;
-	}
 	if (error)
-		goto out_unlock;
-
-	if (local_tp) {
-		error = xfs_trans_commit(local_tp);
-		args->tp = NULL;
-		if (error)
-			goto out_unlock;
-	}
+		return error;
 
 	if (!args->shared)
 		return 0;
 
 	if ((args->flags & IOMAP_ATOMIC) &&
 	    !xfs_bmap_hw_atomic_write_possible(ip, &args->cmap,
-			offset_fsb, end_fsb)) {
-		error = -ENOPROTOOPT;
-		goto out_unlock;
-	}
+			offset_fsb, end_fsb))
+		return -ENOPROTOOPT;
 
 	/*
 	 * COW extent found and allocated. Set up iomap/srcmap and return
-	 * with *nimaps = 0 to tell the caller the COW path is complete.
+	 * with nimaps = 0 to tell the caller the COW path is complete.
 	 */
 	args->nimaps = 0;
 	args->length = XFS_FSB_TO_B(mp,
@@ -1014,22 +942,11 @@ xfs_direct_write_cow_iomap_begin(
 		error = xfs_bmbt_to_iomap(ip, args->srcmap, &args->imap,
 				args->flags, 0, seq);
 		if (error)
-			goto out_unlock;
+			return error;
 	}
 	seq = xfs_iomap_inode_sequence(ip, IOMAP_F_SHARED);
-	if (needs_tp)
-		xfs_iunlock(ip, args->lockmode);
 	return xfs_bmbt_to_iomap(ip, args->iomap, &args->cmap, args->flags,
 			IOMAP_F_SHARED, seq);
-
-out_unlock:
-	if (local_tp) {
-		if (args->tp)
-			xfs_trans_cancel(local_tp);
-		args->tp = NULL;
-		xfs_iunlock(ip, args->lockmode);
-	}
-	return error;
 }
 
 static int
@@ -1094,8 +1011,8 @@ xfs_direct_write_iomap_begin(
 		if (error)
 			goto out_unlock;
 
-		if (xfs_is_cow_inode(ip)) {
-			error = xfs_direct_write_cow_iomap_begin(&dwa, false);
+		if (imap_needs_cow(ip, flags, &dwa.imap, dwa.nimaps)) {
+			error = xfs_direct_write_cow_iomap_begin(&dwa);
 			if (error == -EAGAIN)
 				goto alloc_trans;
 			if (error)
-- 
2.55.0


  parent reply	other threads:[~2026-07-29 10:07 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 10:01 [RFC PATCH 00/33] XFS: Atomic multi-extent operations via rolling transactions Dave Chinner
2026-07-29 10:01 ` [PATCH 01/33] xfs: fix dirty transaction cancellation in xfs_bmapi_convert_one_delalloc Dave Chinner
2026-07-29 10:01 ` [PATCH 02/33] xfs: fix isize update in xfs_iomap_write_unwritten to track conversion progress Dave Chinner
2026-07-29 10:01 ` [PATCH 03/33] xfs: fix block reservation for zoned RT extent remapping Dave Chinner
2026-07-29 10:01 ` [PATCH 04/33] xfs: factor out COW iomap handling from xfs_direct_write_iomap_begin() Dave Chinner
2026-07-29 10:01 ` [PATCH 05/33] xfs: plumb xfs_trans through xfs_reflink_allocate_cow and fill_cow_hole Dave Chinner
2026-07-29 10:01 ` [PATCH 06/33] xfs: teach xfs_reflink_fill_cow_hole() to use a caller-supplied transaction Dave Chinner
2026-07-29 10:01 ` [PATCH 07/33] xfs: add transaction retry infrastructure to xfs_direct_write_cow_iomap_begin Dave Chinner
2026-07-29 10:01 ` [PATCH 08/33] xfs: return -EAGAIN from xfs_reflink_allocate_cow for COW hole without transaction Dave Chinner
2026-07-29 10:01 ` [PATCH 09/33] xfs: remove internal transaction allocation from xfs_reflink_fill_cow_hole Dave Chinner
2026-07-29 10:01 ` [PATCH 10/33] xfs: use zero-block transaction with xfs_trans_reserve_more_inode for COW holes Dave Chinner
2026-07-29 10:01 ` [PATCH 11/33] xfs: change *tp to **tpp in COW allocation call chain Dave Chinner
2026-07-29 10:01 ` [PATCH 12/33] xfs: convert xfs_reflink_fill_delalloc to use rolling transactions Dave Chinner
2026-07-29 10:01 ` [PATCH 13/33] xfs: return -EAGAIN from xfs_reflink_allocate_cow for all allocation cases Dave Chinner
2026-07-29 10:01 ` [PATCH 14/33] xfs: remove dead internal transaction allocation from xfs_reflink_fill_delalloc Dave Chinner
2026-07-29 10:01 ` [PATCH 15/33] xfs: plumb struct xfs_trans *tp into xfs_bmapi_convert_one_delalloc Dave Chinner
2026-07-29 10:02 ` [PATCH 16/33] xfs: use rolling transaction in xfs_bmapi_convert_delalloc Dave Chinner
2026-07-29 10:02 ` [PATCH 17/33] xfs: remove dead internal transaction path from xfs_bmapi_convert_one_delalloc Dave Chinner
2026-07-29 10:02 ` [PATCH 18/33] xfs: add block reservation renewal to xfs_defer_finish Dave Chinner
2026-07-29 10:02 ` [PATCH 19/33] xfs: factor out xfs_iomap_write_unwritten_one helper Dave Chinner
2026-07-29 10:02 ` [PATCH 20/33] xfs: convert xfs_iomap_write_unwritten to rolling transactions Dave Chinner
2026-07-29 10:02 ` [PATCH 21/33] xfs: plumb struct xfs_trans *tp into xfs_reflink_end_cow_extent Dave Chinner
2026-07-29 10:02 ` [PATCH 22/33] xfs: convert xfs_reflink_end_cow to rolling transactions Dave Chinner
2026-07-29 10:02 ` [PATCH 23/33] xfs: remove xfs_reflink_end_cow_extent wrapper and rename locked variant Dave Chinner
2026-07-29 10:02 ` [PATCH 24/33] xfs: convert xfs_zoned_end_io to rolling transactions Dave Chinner
2026-07-29 10:02 ` [PATCH 25/33] xfs: plumb struct xfs_trans *tp into xfs_iomap_write_direct Dave Chinner
2026-07-29 10:02 ` [PATCH 26/33] xfs: make xfs_iomap_write_direct fill in the iomap directly Dave Chinner
2026-07-29 10:02 ` [PATCH 27/33] xfs: plumb struct xfs_trans **tpp into xfs_direct_write_cow_iomap_begin Dave Chinner
2026-07-29 10:02 ` [PATCH 28/33] xfs: introduce struct xfs_direct_write_args for direct write call chain Dave Chinner
2026-07-29 10:02 ` [PATCH 29/33] xfs: convert xfs_direct_write_iomap_begin to use dwa struct throughout Dave Chinner
2026-07-29 10:02 ` [PATCH 30/33] xfs: restructure xfs_direct_write_iomap_begin with unified retry loop Dave Chinner
2026-07-29 10:02 ` Dave Chinner [this message]
2026-07-29 10:02 ` [PATCH 32/33] xfs: make pNFS block allocation atomic with inode update Dave Chinner
2026-07-29 10:02 ` [PATCH 33/33] xfs: remove dead internal transaction path from xfs_iomap_write_direct Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729100629.1943710-32-dgc@kernel.org \
    --to=dgc@kernel.org \
    --cc=cem@kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox