All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.