[PATCH V2 00/38] XFS: Atomic multi-extent operations via rolling transactions

[PATCH V2 00/38] XFS: Atomic multi-extent operations via rolling transactions

[Dave Chinner]

Hi folks,

This is the second version of the patchset intended to address
long-standing races in extent manipulation operations throughput the
IO path. This series addresses the underlying cause of the problem
Darrick recently fixed in commit 2f4acd0fcd86 ("xfs: resample the
data fork mapping after cycling ILOCK") - that extent manipulations
are not atomic w.r.t.  each other and so can race in unexpected and
unpredictable ways.

This is not a new issue - these code paths have been a source of
data corruption bugs for a long time. We have several outstanding
bug reports for intermittent test failures caused by warnings or
assert failures due to unexpected mismatches in extent state
(delalloc, data/cow fork inconsistencies, state not matching cached
imap state, etc) that have resisted attempts to reproduce reliably
and/or find a root cause.

Hence I decided that it would probably be a good idea to do the work
necessary to try to address the underlying problems to try to get
rid of this entire class of bug from the code.

The first problematic pattern this patchset addresses is this:

	- lock ILOCK
	- map extent
	- decide what to do
	- unlock ILOCK
	- allocate transaction
	- lock ILOCK
	- do what was decided
	- commit transaction
	- unlock ILOCK

The issue is that the decision on what to do and actually doing it
is occurring in separate ILOCK contexts - the decision and action
are not atomic, leading to incorrect actions being taken.

This is further exacerbated by the second problematic pattern -
extent manipulations being executed in piece-wise manner and hence
not being atomic with respect to the range over which the action is
being performed. And example of this is unwritten IO completion. It
performs a loop like:

	for the range {offset, len} {
		- allocate transaction
		- lock ILOCK
		- convert next extent in range
		- update i_disk_size
		- commit transaction
		- unlock ILOCK
		- move offset forwards for next extent in range
	}

Hence we can have unwritten extent conversion interact with incoming
writes mapping the same range using the first pattern, and they
lock-step through the extents in the range and the extent map within
the range changes with each ILOCK cycle on both sides. The inherent
raciness of these operations means it is difficult to reason about
them and the races have caused data corruption issues in the past.

There are similar (but more complex) interactions with COW
operations as conversion and fork updates are split across incoming
writes, IO submission and IO completion.

What this patch set is doing is making both patterns atomic with
respect to the ILOCK. Code that uses the first pattern is adjusted
to return all the way back to the start if a transaction is
required. We then allocate a transaction and run the entire
map-decision-modify operation under a single ILOCK context.

For the second pattern, these are converted to use a rolling
transaction so the ILOCK is held across the entire set of
modifications whilst still maintaining the "one extent modification
per transaction" structure of the operation. Whilst this was always
possible with some operations, the operations that required a
physical block reservation for every extent modification (e.g. for
BMBT block allocation) require renewing the block reservation and
hence can receive ENOSPC from those reservations.

Historically speaking, handling errors from within rolling
transactions has been problematic and usually results in a
filesystem shutdown as we can't cancel a dirty transaction cleanly.

Hence to turn these into rolling transactions we require the
addition of a new transaction block reservation regrant mechanism
that can safely return ENOSPC from within a rolling transaction
context. This means we can only regrant block reservations when the
transaction is clean.

When we roll a transaction, we have a new transaction that is clean
after commiting the old transaction, and so this provides a location
for regranting block reservations whilst being able to handle an
ENOSPC error safely. However, if we are in the middle of a defer-ops
chain, we cannot safely unwind even though we have a clean
transaction. i.e. the "atomic modification" of the extent
modification has to complete fully before we can regrant block
reservation space and handle ENOSPC.

We only want to do this block reservation regrant when committing
the final transaction that finishes deferops processing - the
original reservation should already be sufficient for the entire
deferops chain, and so by avoiding regrants in the middle of an
intent processing chain we avoid seeing ENOSPC errors that would
cause a shutdown. Hence we only process regrants at the top level
commit in xfs_defer_finish().

With this mechanism, we can convert the individual transactions in a
loop like unwritten conversion to a rolling transaction that handles
ENOSPC in exactly the same way that the individual transaction
method does. This is how the series makes these multi-extent
modifications atomic with respect to the ILOCK and hence other
extent manipulations.

The overall series addresses the most frequently hit instances of
both patterns.  There are other code paths that have similar
patterns that could be converted, but it's not immediately clear
that any of them matter in any material way. If there are such
instances, they can be addressed in future series.

The patchset is made up entirely of LLM predicted code. I review it
all and correct any errors, issues, concerns, etc immediately as
each new hunk of code is predicted by the LLM to prevent errors from
compounding and multiplying. Thus the code ends up the way I'd write
it manually, not the way an unchecked LLM would vibe code it.
Mistakes in the code are mine, not a result of an LLM being used to
write the code.

The series passed many iterations of the fstests auto group without
regressions using a v6.19 XFS userspace, and it has survived several
hundred iterations of the recoveryloop group without an unexpected
failure.

Feedback, comments and thoughts are welcome.

-Dave.

----

Version 2:

- Fixed zoned gc remap reservation - needs two extent add
  reservations because a remap involves two independent extent map
  modifications.
- Factored transaction block reservation code to minimise duplicate
  accounting code.
- reworked XFS_TRANS_RENEW_BLKRES implementation on top of this
  factoring
- New async blockgc flushing infrastructure to allow blockgc to be
  triggered in ENOSPC/EDQUOT situations where waiting is not
  possible (e.g. in transaction context).
- Fix ILOCK leaks on the internal error paths in various
transitionary state patches.

Version 1:
- https://lore.kernel.org/linux-xfs/20260729100629.1943710-1-dgc@kernel.org/

----

LLM generated summary of the series:

This series of 38 patches fixes several race conditions and bugs in the XFS
extent manipulation code, then reworks the transaction handling to make
multi-extent operations atomic with respect to the ILOCK.

Bug fixes (patches 1-4):

Four pre-existing bugs are fixed.

A dirty transaction cancellation in xfs_bmapi_convert_one_delalloc() where
xfs_iext_count_extend() could dirty a transaction before discovering there was
nothing to convert, leading to a filesystem shutdown on the -EAGAIN cancel path.

The same dirty transaction cancellation pattern in xfs_attr_set() is fixed by
moving the attr lookup ahead of xfs_iext_count_extend() so the transaction is
only dirtied once the operation is known to proceed.

An isize update bug in xfs_iomap_write_unwritten() where i_disk_size was set to
the end of the entire write range rather than the end of the actually converted
extent, potentially exposing zeroes after a crash.

A block reservation bug in xfs_zoned_end_io() where the reservation only covered
bmbt splits but not the deferred rmap and refcount btree operations, papered
over by the fragile XFS_TRANS_RES_FDBLKS workaround.

Transaction reservation and async blockgc infrastructure (patches 5-8):

The block reservation accounting in xfs_trans_reserve() is factored into
reusable xfs_trans_reserve_blocks()/xfs_trans_unreserve_blocks() helpers to
avoid duplicating the accounting code needed by the later regrant mechanism.
New asynchronous blockgc flushing infrastructure is added so that blockgc can be
kicked without waiting in contexts where blocking is not possible - in
particular from within a transaction context holding the ILOCK, where a
synchronous flush would deadlock against the blockgc workers.
xfs_trans_reserve_more_inode() gains a retry loop that kicks async block and
quota blockgc on ENOSPC/EDQUOT and retries after a short io_schedule_timeout()
rather than failing immediately.

COW direct write atomicity (patches 9-19):

The COW extent allocation path in xfs_direct_write_iomap_begin() is restructured
so that the extent lookup and COW allocation are performed atomically under a
single ILOCK hold. Previously, the ILOCK was cycled between the extent lookup
and the allocation, leaving a window where racing DIO writes could modify the
extent tree and cause stale state bugs. Transaction allocation is lifted to the
caller with a -EAGAIN retry mechanism, and xfs_reflink_fill_cow_hole() and
xfs_reflink_fill_delalloc() are converted to use rolling transactions via
xfs_defer_finish() to keep the ILOCK held across their entire operation.

Delalloc conversion atomicity (patches 20-22):

The xfs_bmapi_convert_delalloc() loop, which converts delalloc extents to real
allocations, is converted from per-iteration transaction allocation to a single
rolling transaction using xfs_defer_finish(). This makes multi-extent delalloc
conversion atomic with respect to other concurrent extent operations.

Block reservation renewal infrastructure (patch 23):

New XFS_TRANS_RENEW_BLKRES transaction flag and xfs_trans_regrant_blkres()
mechanism that automatically restores the block reservation to its original
level after xfs_defer_finish() processes deferred operations. This is needed for
rolling transaction loops where each iteration requires a block reservation for
btree splits.

Unwritten extent conversion atomicity (patches 24-25):

The xfs_iomap_write_unwritten() loop, used for buffered writeback and DIO
completion, is converted from per-iteration transaction allocation to a single
rolling transaction with XFS_TRANS_RENEW_BLKRES for automatic block reservation
renewal.

COW IO completion atomicity (patches 26-28):

The xfs_reflink_end_cow() loop is converted to a rolling transaction, making COW
extent remapping atomic. The xfs_reflink_end_cow_extent() wrapper is removed and
the locked variant renamed since there is no longer an unlocked path.

Zoned IO completion atomicity (patch 29):

The xfs_zoned_end_io() loop is converted to a rolling transaction with the
corrected XFS_RTEXTENTADD_SPACE_RES() block reservation.

Direct write iomap restructure (patches 30-38):

The xfs_direct_write_iomap_begin() function is restructured with a unified
do/while retry loop that handles both COW and non-COW allocation paths
atomically. A new xfs_direct_write_args struct replaces the long parameter lists
throughout the call chain. The pNFS block allocation path is reworked to perform
extent allocation and inode update in a single synchronous transaction. Dead
internal transaction paths are removed from xfs_iomap_write_direct() and
xfs_direct_write_cow_iomap_begin() now that all callers supply transactions.

----

Diffstat:

 fs/xfs/libxfs/xfs_attr.c        |  70 ++---
 fs/xfs/libxfs/xfs_bmap.c        |  80 +++---
 fs/xfs/libxfs/xfs_defer.c       |  38 ++-
 fs/xfs/libxfs/xfs_shared.h      |   3 +
 fs/xfs/libxfs/xfs_trans_space.h |  12 +
 fs/xfs/scrub/cow_repair.c       |   4 +-
 fs/xfs/scrub/quota_repair.c     |   2 +-
 fs/xfs/scrub/rtbitmap_repair.c  |   2 +-
 fs/xfs/scrub/tempfile.c         |   2 +-
 fs/xfs/xfs_icache.c             | 172 ++++++++++---
 fs/xfs/xfs_icache.h             |   2 +
 fs/xfs/xfs_iomap.c              | 548 +++++++++++++++++++++++-----------------
 fs/xfs/xfs_iomap.h              |  23 +-
 fs/xfs/xfs_pnfs.c               | 117 +++++----
 fs/xfs/xfs_reflink.c            | 313 +++++++++--------------
 fs/xfs/xfs_reflink.h            |   6 +-
 fs/xfs/xfs_trans.c              | 227 ++++++++++-------
 fs/xfs/xfs_trans.h              |   7 +-
 fs/xfs/xfs_zone_alloc.c         |  47 ++--
 19 files changed, 957 insertions(+), 718 deletions(-)

[PATCH 01/38] xfs: fix dirty transaction cancellation in xfs_bmapi_convert_one_delalloc

[Dave Chinner]

xfs_bmapi_convert_one_delalloc() calls xfs_iext_count_extend() before
checking whether there is actually a delalloc extent to convert. If
xfs_iext_count_extend() modifies the inode's extent count fields it
will dirty the transaction. If the subsequent extent lookup finds
nothing to convert (e.g. a racing COW completion moved the extent to
the data fork), the function returns -EAGAIN and the caller cancels
the transaction. Cancelling a dirty transaction triggers a filesystem
shutdown.

Fix this by moving the extent lookup and validation ahead of the
xfs_iext_count_extend() call. This ensures that if there is no extent
to convert, or the extent has already been converted by a racing
thread, the function returns before the transaction is dirtied and
can be safely cancelled.

Fixes: 4f86bb4b66c9 ("xfs: Conditionally upgrade existing inodes to use large extent counters")
Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index d64defeda645..47c4d5c52b95 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4380,11 +4380,11 @@ xfs_bmapi_convert_one_delalloc(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
-	error = xfs_iext_count_extend(tp, ip, whichfork,
-			XFS_IEXT_ADD_NOSPLIT_CNT);
-	if (error)
-		goto out_trans_cancel;
-
+	/*
+	 * Look up the extent before extending the extent count so that we
+	 * don't dirty the transaction if there is nothing to convert. A
+	 * dirty transaction cancellation on -EAGAIN would shutdown the fs.
+	 */
 	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &bma.icur, &bma.got) ||
 	    bma.got.br_startoff > offset_fsb) {
 		/*
@@ -4409,6 +4409,11 @@ xfs_bmapi_convert_one_delalloc(
 		goto out_trans_cancel;
 	}
 
+	error = xfs_iext_count_extend(tp, ip, whichfork,
+			XFS_IEXT_ADD_NOSPLIT_CNT);
+	if (error)
+		goto out_trans_cancel;
+
 	bma.tp = tp;
 	bma.ip = ip;
 	bma.wasdel = true;
-- 
2.55.0

[PATCH 02/38] xfs: fix dirty transaction cancellation in xfs_attr_set

[Dave Chinner]

xfs_attr_set() calls xfs_iext_count_extend() before xfs_attr_lookup().
If xfs_iext_count_extend() upgrades the inode to NREXT64 format it
will dirty the transaction. If the subsequent xfs_attr_lookup() finds
a result that is incompatible with the requested operation (e.g.
EEXIST for CREATE, ENOATTR for REMOVE/REPLACE), the function returns
the error and the caller cancels the transaction. Cancelling a dirty
transaction triggers a filesystem shutdown.

Fix this by moving the xfs_attr_lookup() call before
xfs_iext_count_extend() and validating the lookup result against the
requested operation before dirtying the transaction. The code is
restructured to separate validation from execution: first filter out
the error cases that should cancel cleanly, then extend the extent
count, then dispatch to the appropriate modification function based
on the operation type.

Fixes: 4f86bb4b66c9 ("xfs: Conditionally upgrade existing inodes to use large extent counters")
Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_attr.c | 70 ++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index b3f7b2c34ad7..bf0e867628e7 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1136,7 +1136,7 @@ xfs_attr_set(
 	struct xfs_inode	*dp = args->dp;
 	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_trans_res	tres;
-	int			error, local;
+	int			error, lookup_result, local;
 	int			rmt_blks = 0;
 	unsigned int		total = 0;
 
@@ -1185,48 +1185,48 @@ xfs_attr_set(
 	if (error)
 		return error;
 
-	if (op != XFS_ATTRUPDATE_REMOVE || xfs_inode_hasattr(dp)) {
-		error = xfs_iext_count_extend(args->trans, dp, XFS_ATTR_FORK,
-				XFS_IEXT_ATTR_MANIP_CNT(rmt_blks));
-		if (error)
-			goto out_trans_cancel;
-	}
-
-	error = xfs_attr_lookup(args);
-	switch (error) {
-	case -EEXIST:
-		if (op == XFS_ATTRUPDATE_REMOVE) {
-			/* if no value, we are performing a remove operation */
-			error = xfs_attr_removename(args);
-			if (error)
-				goto out_trans_cancel;
-			break;
-		}
-
-		/* Pure create fails if the attr already exists */
+	/*
+	 * Look up the attr before extending the extent count so that we
+	 * don't dirty the transaction if the op is going to fail with an
+	 * error. Cancelling a dirty transaction would shutdown the fs.
+	 */
+	lookup_result = xfs_attr_lookup(args);
+	if (lookup_result == -EEXIST) {
 		if (op == XFS_ATTRUPDATE_CREATE)
 			goto out_trans_cancel;
-
-		error = xfs_attr_replacename(args, rmt_blks);
-		if (error)
-			goto out_trans_cancel;
-		break;
-	case -ENOATTR:
-		/* Can't remove what isn't there. */
-		if (op == XFS_ATTRUPDATE_REMOVE)
+	} else if (lookup_result == -ENOATTR) {
+		if (op == XFS_ATTRUPDATE_REMOVE ||
+		    op == XFS_ATTRUPDATE_REPLACE)
 			goto out_trans_cancel;
+	} else {
+		error = lookup_result;
+		goto out_trans_cancel;
+	}
 
-		/* Pure replace fails if no existing attr to replace. */
-		if (op == XFS_ATTRUPDATE_REPLACE)
-			goto out_trans_cancel;
+	error = xfs_iext_count_extend(args->trans, dp, XFS_ATTR_FORK,
+			XFS_IEXT_ATTR_MANIP_CNT(rmt_blks));
+	if (error)
+		goto out_trans_cancel;
 
+	switch (op) {
+	case XFS_ATTRUPDATE_REMOVE:
+		error = xfs_attr_removename(args);
+		break;
+	case XFS_ATTRUPDATE_CREATE:
 		error = xfs_attr_setname(args, rmt_blks);
-		if (error)
-			goto out_trans_cancel;
 		break;
-	default:
-		goto out_trans_cancel;
+	case XFS_ATTRUPDATE_UPSERT:
+		if (lookup_result == -ENOATTR) {
+			error = xfs_attr_setname(args, rmt_blks);
+			break;
+		}
+		fallthrough;
+	case XFS_ATTRUPDATE_REPLACE:
+		error = xfs_attr_replacename(args, rmt_blks);
+		break;
 	}
+	if (error)
+		goto out_trans_cancel;
 
 	/*
 	 * If this is a synchronous mount, make sure that the
-- 
2.55.0

[PATCH 03/38] xfs: fix isize update in xfs_iomap_write_unwritten to track conversion progress

[Dave Chinner]

xfs_iomap_write_unwritten() updates i_disk_size using the end of
the entire unwritten range (offset_fsb + count_fsb) rather than the
end of the extent that was actually converted in each iteration
(imap.br_startoff + imap.br_blockcount).

If the conversion requires multiple iterations and a crash occurs
partway through, recovery would replay the first transaction which
set i_disk_size to the end of the full range. This exposes
unwritten extents to userspace reads as zeroes rather than the
data that was written, because those extents have not yet been
converted from unwritten to written state.

Fix this by computing i_size from the extent that was actually
converted (imap), so i_disk_size advances incrementally as each
extent is converted. On crash, recovery only exposes data in
extents that have been both written and converted.

Fixes: 84803fb78237 ("xfs: log file size updates as part of unwritten extent conversion")
Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 225c3de88d03..1437ea93563c 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -682,11 +682,15 @@ xfs_iomap_write_unwritten(
 			goto error_on_bmapi_transaction;
 
 		/*
-		 * Log the updated inode size as we go.  We have to be careful
-		 * to only log it up to the actual write offset if it is
-		 * halfway into a block.
+		 * Update the inode size to reflect the extent that was
+		 * converted in this iteration. We must not advance isize
+		 * beyond the extent we just converted, otherwise a crash
+		 * before the next conversion exposes unwritten extents
+		 * (zeroes) to userspace instead of the written data.
+		 * Clamp to the byte-level write end in case the converted
+		 * extent extends past the write boundary.
 		 */
-		i_size = XFS_FSB_TO_B(mp, offset_fsb + count_fsb);
+		i_size = XFS_FSB_TO_B(mp, imap.br_startoff + imap.br_blockcount);
 		if (i_size > offset + count)
 			i_size = offset + count;
 		if (update_isize && i_size > i_size_read(inode))
-- 
2.55.0

[PATCH 04/38] xfs: fix block reservation for zoned RT extent remapping

[Dave Chinner]

xfs_zoned_end_io() uses XFS_EXTENTADD_SPACE_RES() for its block
reservation, which only covers bmbt splits. However, the remap
operation in xfs_zoned_map_extent() generates deferred rmap and
refcount btree updates that also need blocks for btree splits
when they are processed.

The code used XFS_TRANS_RES_FDBLKS as a workaround to recycle
freed data blocks back into the transaction's block reservation.
This is fragile — it depends on freed blocks being large enough
to cover the metadata btree needs, and conflates data block
recycling with metadata reservation.

Fix this by introducing XFS_RTEXTENTADD_SPACE_RES() which computes
the correct reservation for any RT data extent modification: the
bmbt split cost plus the rt rmap btree split cost plus the rt
refcount btree cost. This covers all the deferred operations that
are generated when an RT extent is mapped or unmapped.

Drop XFS_TRANS_RES_FDBLKS from xfs_zoned_end_io() since the
reservation now correctly covers all btree costs.

Note: XFS_RTEXTENTADD_SPACE_RES() should eventually be folded
into XFS_EXTENTADD_SPACE_RES() so that all callers operating on
RT inodes automatically get the correct reservation. There are
approximately 11 sites that use XFS_EXTENTADD_SPACE_RES either
directly or via XFS_DIOSTRAT_SPACE_RES that have the same
under-reservation issue for RT inodes.

Fixes: 4e4d52075577 ("xfs: add the zoned space allocator")
Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_trans_space.h | 12 ++++++++++++
 fs/xfs/xfs_zone_alloc.c         |  9 ++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_trans_space.h b/fs/xfs/libxfs/xfs_trans_space.h
index d89b570aafcc..4cfc29ac764f 100644
--- a/fs/xfs/libxfs/xfs_trans_space.h
+++ b/fs/xfs/libxfs/xfs_trans_space.h
@@ -55,6 +55,18 @@
 	  XFS_MAX_CONTIG_EXTENTS_PER_BLOCK(mp)) * \
 	  XFS_EXTENTADD_SPACE_RES(mp,w))
 
+/*
+ * Blocks needed to add or remove a realtime data extent: the bmbt split plus
+ * rt rmap btree and rt refcount btree updates deferred from the bmbt operation.
+ *
+ * TODO: this should be folded into XFS_EXTENTADD_SPACE_RES() so that all
+ * callers that operate on RT inodes automatically get the correct reservation.
+ */
+#define XFS_RTEXTENTADD_SPACE_RES(mp)	\
+	(XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK) + \
+	 (xfs_has_rmapbt(mp) ? XFS_RTRMAPADD_SPACE_RES(mp) : 0) + \
+	 (xfs_has_reflink(mp) ? 2 * (mp)->m_rtrefc_maxlevels - 1 : 0))
+
 /* Blocks we might need to add "b" mappings & rmappings to a file. */
 #define XFS_SWAP_RMAP_SPACE_RES(mp,b,w)\
 	(XFS_NEXTENTADD_SPACE_RES((mp), (b), (w)) + \
diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 7d13fa7ab30a..61b483d24c88 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -330,8 +330,11 @@ xfs_zoned_end_io(
 		.br_startblock	= xfs_daddr_to_rtb(mp, daddr),
 		.br_state	= XFS_EXT_NORM,
 	};
-	unsigned int		resblks =
-		XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
+	/*
+	 * A remap performs two independent BMBT modifications - an unmap
+	 * of the old extent followed by a map of the new extent.
+	 */
+	unsigned int		resblks = 2 * XFS_RTEXTENTADD_SPACE_RES(mp);
 	struct xfs_trans	*tp;
 	int			error;
 
@@ -342,7 +345,7 @@ xfs_zoned_end_io(
 		new.br_blockcount = end_fsb - new.br_startoff;
 
 		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
-				XFS_TRANS_RESERVE | XFS_TRANS_RES_FDBLKS, &tp);
+				XFS_TRANS_RESERVE, &tp);
 		if (error)
 			return error;
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
-- 
2.55.0

[PATCH 05/38] xfs: factor xfs_trans_reserve_blocks() from xfs_trans_reserve()

[Dave Chinner]

Rename xfs_trans_reserve_more() to xfs_trans_reserve_blocks() and
refactor xfs_trans_reserve() to call it instead of open coding the
block reservation. Add the inverse xfs_trans_unreserve_blocks() for
callers that need to give back reservations. This provides a common
helper for both the initial transaction reservation and the
reserve-more-later pattern used by online repair and the iomap
paths.

Update xfs_trans_reserve_more_inode() to use the new helpers and
convert all remaining xfs_trans_reserve_more() callers in scrub.

Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/scrub/cow_repair.c      |   4 +-
 fs/xfs/scrub/quota_repair.c    |   2 +-
 fs/xfs/scrub/rtbitmap_repair.c |   2 +-
 fs/xfs/scrub/tempfile.c        |   2 +-
 fs/xfs/xfs_trans.c             | 136 ++++++++++++++-------------------
 fs/xfs/xfs_trans.h             |   4 +-
 6 files changed, 67 insertions(+), 83 deletions(-)

diff --git a/fs/xfs/scrub/cow_repair.c b/fs/xfs/scrub/cow_repair.c
index 8dd9c0266e21..ee2a362d9bb2 100644
--- a/fs/xfs/scrub/cow_repair.c
+++ b/fs/xfs/scrub/cow_repair.c
@@ -435,7 +435,7 @@ xrep_cow_alloc(
 	};
 	int			error;
 
-	error = xfs_trans_reserve_more(sc->tp, del->br_blockcount, 0);
+	error = xfs_trans_reserve_blocks(sc->tp, del->br_blockcount, 0);
 	if (error)
 		return error;
 
@@ -467,7 +467,7 @@ xrep_cow_alloc_rt(
 	xfs_extlen_t		len;
 	int			error;
 
-	error = xfs_trans_reserve_more(sc->tp, 0, maxrtx);
+	error = xfs_trans_reserve_blocks(sc->tp, 0, maxrtx);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
index 487bd4f68ebb..759a5d5b2379 100644
--- a/fs/xfs/scrub/quota_repair.c
+++ b/fs/xfs/scrub/quota_repair.c
@@ -66,7 +66,7 @@ xrep_quota_item_fill_bmap_hole(
 	xfs_trans_ijoin(sc->tp, sc->ip, 0);
 
 	/* Map a block into the file. */
-	error = xfs_trans_reserve_more(sc->tp, XFS_QM_DQALLOC_SPACE_RES(mp),
+	error = xfs_trans_reserve_blocks(sc->tp, XFS_QM_DQALLOC_SPACE_RES(mp),
 			0);
 	if (error)
 		return error;
diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index 442a17bf9720..61f33e558ba4 100644
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -546,7 +546,7 @@ xrep_rtbitmap(
 		if (delta > UINT_MAX)
 			return -EOPNOTSUPP;
 
-		error = xfs_trans_reserve_more(sc->tp, delta, 0);
+		error = xfs_trans_reserve_blocks(sc->tp, delta, 0);
 		if (error)
 			return error;
 
diff --git a/fs/xfs/scrub/tempfile.c b/fs/xfs/scrub/tempfile.c
index 98820003b929..6bffaf9bc1e8 100644
--- a/fs/xfs/scrub/tempfile.c
+++ b/fs/xfs/scrub/tempfile.c
@@ -818,7 +818,7 @@ xrep_tempexch_trans_reserve(
 	if (error)
 		return error;
 
-	error = xfs_trans_reserve_more(sc->tp, tx->req.resblks, 0);
+	error = xfs_trans_reserve_blocks(sc->tp, tx->req.resblks, 0);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 1b36cf12d4e3..609b0fe3d4d3 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -131,6 +131,53 @@ xfs_trans_dup(
 	return ntp;
 }
 
+/*
+ * Reserve disk blocks and RT extents for a transaction. On success the
+ * requested blocks are decremented from the global free space counters and
+ * added to the transaction's block reservation. On failure, no space is
+ * reserved and the transaction is not modified, and callers must be able to
+ * cancel the transaction without shutting down the filesystem.
+ */
+int
+xfs_trans_reserve_blocks(
+	struct xfs_trans	*tp,
+	unsigned int		blocks,
+	unsigned int		rtextents)
+{
+	bool			rsvd = tp->t_flags & XFS_TRANS_RESERVE;
+
+	if (blocks && xfs_dec_fdblocks(tp->t_mountp, blocks, rsvd))
+		return -ENOSPC;
+	if (rtextents && xfs_dec_frextents(tp->t_mountp, rtextents)) {
+		if (blocks)
+			xfs_add_fdblocks(tp->t_mountp, blocks);
+		return -ENOSPC;
+	}
+	tp->t_blk_res += blocks;
+	tp->t_rtx_res += rtextents;
+	return 0;
+}
+
+/*
+ * Give back block and RT extent reservations to the free space counters.
+ * This is the inverse of xfs_trans_reserve_blocks().
+ */
+void
+xfs_trans_unreserve_blocks(
+	struct xfs_trans	*tp,
+	unsigned int		blocks,
+	unsigned int		rtextents)
+{
+	if (blocks) {
+		xfs_add_fdblocks(tp->t_mountp, blocks);
+		tp->t_blk_res -= blocks;
+	}
+	if (rtextents) {
+		xfs_add_frextents(tp->t_mountp, rtextents);
+		tp->t_rtx_res -= rtextents;
+	}
+}
+
 /*
  * This is called to reserve free disk blocks and log space for the given
  * transaction before allocating any resources within the transaction.
@@ -149,22 +196,13 @@ xfs_trans_reserve(
 	uint			rtextents)
 {
 	struct xfs_mount	*mp = tp->t_mountp;
-	int			error = 0;
-	bool			rsvd = (tp->t_flags & XFS_TRANS_RESERVE) != 0;
+	int			error;
 
 	ASSERT(resp->tr_logres > 0);
 
-	/*
-	 * Attempt to reserve the needed disk blocks by decrementing the number
-	 * needed from the number available.  This will fail if the count would
-	 * go below zero.
-	 */
-	if (blocks > 0) {
-		error = xfs_dec_fdblocks(mp, blocks, rsvd);
-		if (error != 0)
-			return -ENOSPC;
-		tp->t_blk_res += blocks;
-	}
+	error = xfs_trans_reserve_blocks(tp, blocks, rtextents);
+	if (error)
+		return error;
 
 	/*
 	 * Reserve the log space needed for this transaction.
@@ -173,39 +211,15 @@ xfs_trans_reserve(
 		tp->t_flags |= XFS_TRANS_PERM_LOG_RES;
 	error = xfs_log_reserve(mp, resp->tr_logres, resp->tr_logcount,
 			&tp->t_ticket, (tp->t_flags & XFS_TRANS_PERM_LOG_RES));
-	if (error)
-		goto undo_blocks;
+	if (error) {
+		xfs_trans_unreserve_blocks(tp, blocks, rtextents);
+		tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
+		return error;
+	}
 
 	tp->t_log_res = resp->tr_logres;
 	tp->t_log_count = resp->tr_logcount;
-
-	/*
-	 * Attempt to reserve the needed realtime extents by decrementing the
-	 * number needed from the number available.  This will fail if the
-	 * count would go below zero.
-	 */
-	if (rtextents > 0) {
-		error = xfs_dec_frextents(mp, rtextents);
-		if (error) {
-			error = -ENOSPC;
-			goto undo_log;
-		}
-		tp->t_rtx_res += rtextents;
-	}
-
 	return 0;
-
-undo_log:
-	xfs_log_ticket_ungrant(mp->m_log, tp->t_ticket);
-	tp->t_ticket = NULL;
-	tp->t_log_res = 0;
-	tp->t_flags &= ~XFS_TRANS_PERM_LOG_RES;
-undo_blocks:
-	if (blocks > 0) {
-		xfs_add_fdblocks(mp, blocks);
-		tp->t_blk_res = 0;
-	}
-	return error;
 }
 
 static struct xfs_trans *
@@ -1115,38 +1129,9 @@ xfs_trans_alloc_inode(
 	return error;
 }
 
-/*
- * Try to reserve more blocks for a transaction.
- *
- * This is for callers that need to attach resources to a transaction, scan
- * those resources to determine the space reservation requirements, and then
- * modify the attached resources.  In other words, online repair.  This can
- * fail due to ENOSPC, so the caller must be able to cancel the transaction
- * without shutting down the fs.
- */
-int
-xfs_trans_reserve_more(
-	struct xfs_trans	*tp,
-	unsigned int		blocks,
-	unsigned int		rtextents)
-{
-	bool			rsvd = tp->t_flags & XFS_TRANS_RESERVE;
-
-	if (blocks && xfs_dec_fdblocks(tp->t_mountp, blocks, rsvd))
-		return -ENOSPC;
-	if (rtextents && xfs_dec_frextents(tp->t_mountp, rtextents)) {
-		if (blocks)
-			xfs_add_fdblocks(tp->t_mountp, blocks);
-		return -ENOSPC;
-	}
-	tp->t_blk_res += blocks;
-	tp->t_rtx_res += rtextents;
-	return 0;
-}
-
 /*
  * Try to reserve more blocks and file quota for a transaction.  Same
- * conditions of usage as xfs_trans_reserve_more.
+ * conditions of usage as xfs_trans_reserve_blocks.
  */
 int
 xfs_trans_reserve_more_inode(
@@ -1162,7 +1147,7 @@ xfs_trans_reserve_more_inode(
 
 	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
 
-	error = xfs_trans_reserve_more(tp, dblocks, rtx);
+	error = xfs_trans_reserve_blocks(tp, dblocks, rtx);
 	if (error)
 		return error;
 
@@ -1178,10 +1163,7 @@ xfs_trans_reserve_more_inode(
 		return 0;
 
 	/* Quota failed, give back the new reservation. */
-	xfs_add_fdblocks(mp, dblocks);
-	tp->t_blk_res -= dblocks;
-	xfs_add_frextents(mp, rtx);
-	tp->t_rtx_res -= rtx;
+	xfs_trans_unreserve_blocks(tp, dblocks, rtx);
 	return error;
 }
 
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index eb83c5dac032..de77b617bccf 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -167,7 +167,9 @@ typedef struct xfs_trans {
 int		xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
 			uint blocks, uint rtextents, uint flags,
 			struct xfs_trans **tpp);
-int		xfs_trans_reserve_more(struct xfs_trans *tp,
+int		xfs_trans_reserve_blocks(struct xfs_trans *tp,
+			unsigned int blocks, unsigned int rtextents);
+void		xfs_trans_unreserve_blocks(struct xfs_trans *tp,
 			unsigned int blocks, unsigned int rtextents);
 struct xfs_trans *xfs_trans_alloc_empty(struct xfs_mount *mp);
 void		xfs_trans_mod_sb(xfs_trans_t *, uint, int64_t);
-- 
2.55.0

[PATCH 06/38] xfs: factor xfs_blockgc_start_flush() from xfs_blockgc_flush_all()

[Dave Chinner]

Factor xfs_blockgc_start_flush() out of xfs_blockgc_flush_all() to
provide an async variant that kicks the per-AG background blockgc
workers without waiting for them to complete.

xfs_blockgc_flush_all() uses synchronous flush_delayed_work() calls
to wait for the blockgc workers and inodegc to finish. This cannot
be used from contexts that hold the ILOCK because the blockgc
workers need to take the ILOCK to scan inodes for reclaimable
speculative preallocations, and the inode we hold the ILOCK on pins
journal space that the workers need to run transactions.

Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_icache.c | 27 +++++++++++++++++++--------
 fs/xfs/xfs_icache.h |  1 +
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 9d8dd30bd927..bad09cb090cc 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1596,23 +1596,34 @@ xfs_blockgc_free_space(
 }
 
 /*
- * Reclaim all the free space that we can by scheduling the background blockgc
- * and inodegc workers immediately and waiting for them all to clear.
+ * Kick all the blockgc workers immediately so they start freeing speculative
+ * preallocations. This does not wait for the workers to complete, so callers
+ * that need to wait for space to become available should call
+ * xfs_blockgc_flush_all() instead.
  */
-int
-xfs_blockgc_flush_all(
+void
+xfs_blockgc_start_flush(
 	struct xfs_mount	*mp)
 {
 	struct xfs_perag	*pag = NULL;
 
 	trace_xfs_blockgc_flush_all(mp, __return_address);
 
-	/*
-	 * For each blockgc worker, move its queue time up to now.  If it wasn't
-	 * queued, it will not be requeued.  Then flush whatever is left.
-	 */
 	while ((pag = xfs_perag_grab_next_tag(mp, pag, XFS_ICI_BLOCKGC_TAG)))
 		mod_delayed_work(mp->m_blockgc_wq, &pag->pag_blockgc_work, 0);
+}
+
+/*
+ * Reclaim all the free space that we can by scheduling the background blockgc
+ * and inodegc workers immediately and waiting for them all to clear.
+ */
+int
+xfs_blockgc_flush_all(
+	struct xfs_mount	*mp)
+{
+	struct xfs_perag	*pag = NULL;
+
+	xfs_blockgc_start_flush(mp);
 
 	while ((pag = xfs_perag_grab_next_tag(mp, pag, XFS_ICI_BLOCKGC_TAG)))
 		flush_delayed_work(&pag->pag_blockgc_work);
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 905944dafbe5..e68d13125f7b 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -62,6 +62,7 @@ int xfs_blockgc_free_dquots(struct xfs_mount *mp, struct xfs_dquot *udqp,
 		unsigned int iwalk_flags);
 int xfs_blockgc_free_quota(struct xfs_inode *ip, unsigned int iwalk_flags);
 int xfs_blockgc_free_space(struct xfs_mount *mp, struct xfs_icwalk *icm);
+void xfs_blockgc_start_flush(struct xfs_mount *mp);
 int xfs_blockgc_flush_all(struct xfs_mount *mp);
 
 void xfs_inode_set_eofblocks_tag(struct xfs_inode *ip);
-- 
2.55.0

[PATCH 07/38] xfs: add async quota-targeted blockgc flush

[Dave Chinner]

Convert xfs_blockgc_free_space() to queue the icwalk scan to the
blockgc workqueue rather than running it inline. The existing
synchronous callers wait for completion via flush_work(). Add a
static xfs_blockgc_free_space_nowait() for contexts that need to
kick a scan without waiting.

Factor xfs_blockgc_dquot_filter() out of xfs_blockgc_free_dquots()
to build the icwalk quota filter separately from running the scan.
This allows both the existing synchronous xfs_blockgc_free_dquots()
and the new async xfs_blockgc_free_quota_nowait() to share the
filter construction.

xfs_blockgc_free_quota_nowait() is the async variant for contexts
that hold the ILOCK and cannot wait for the scan synchronously. The
blockgc workers need the ILOCK to scan inodes for reclaimable
speculative preallocations, and the locked inode pins journal space
that the workers need for transactions.

Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_icache.c | 145 +++++++++++++++++++++++++++++++++++---------
 fs/xfs/xfs_icache.h |   1 +
 2 files changed, 116 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index bad09cb090cc..256253bfeac1 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1575,24 +1575,75 @@ xfs_blockgc_worker(
 	xfs_blockgc_queue(pag);
 }
 
+struct xfs_blockgc_async_work {
+	struct work_struct	work;
+	struct xfs_mount	*mp;
+	struct xfs_icwalk	icw;
+	int			error;
+	bool			async;
+};
+
+static void
+xfs_blockgc_free_space_worker(
+	struct work_struct	*work)
+{
+	struct xfs_blockgc_async_work *baw = container_of(work,
+					struct xfs_blockgc_async_work, work);
+
+	baw->error = xfs_icwalk(baw->mp, XFS_ICWALK_BLOCKGC, &baw->icw);
+	if (!baw->error)
+		baw->error = xfs_inodegc_flush(baw->mp);
+
+	if (baw->async)
+		kfree(baw);
+}
+
 /*
  * Try to free space in the filesystem by purging inactive inodes, eofblocks
- * and cowblocks.
+ * and cowblocks. The scan is queued to the blockgc workqueue and the caller
+ * waits for it to complete.
  */
 int
 xfs_blockgc_free_space(
 	struct xfs_mount	*mp,
 	struct xfs_icwalk	*icw)
 {
-	int			error;
+	struct xfs_blockgc_async_work baw = {
+		.mp		= mp,
+		.icw		= *icw,
+		.async		= false,
+	};
 
 	trace_xfs_blockgc_free_space(mp, icw, _RET_IP_);
 
-	error = xfs_icwalk(mp, XFS_ICWALK_BLOCKGC, icw);
-	if (error)
-		return error;
+	INIT_WORK_ONSTACK(&baw.work, xfs_blockgc_free_space_worker);
+	queue_work(mp->m_blockgc_wq, &baw.work);
+	flush_work(&baw.work);
+	destroy_work_on_stack(&baw.work);
+	return baw.error;
+}
 
-	return xfs_inodegc_flush(mp);
+/*
+ * Queue a background space reclaim scan without waiting for it to complete.
+ * This is for callers that hold the ILOCK and cannot wait for the scan
+ * synchronously because the blockgc workers need the ILOCK to scan inodes
+ * and the locked inode pins journal space that the workers need for
+ * transactions.
+ */
+static void
+xfs_blockgc_free_space_nowait(
+	struct xfs_mount	*mp,
+	struct xfs_icwalk	*icw)
+{
+	struct xfs_blockgc_async_work *baw;
+
+	baw = kzalloc(sizeof(*baw), GFP_KERNEL | __GFP_NOFAIL);
+	baw->mp = mp;
+	baw->icw = *icw;
+	baw->async = true;
+
+	INIT_WORK(&baw->work, xfs_blockgc_free_space_worker);
+	queue_work(mp->m_blockgc_wq, &baw->work);
 }
 
 /*
@@ -1632,56 +1683,72 @@ xfs_blockgc_flush_all(
 }
 
 /*
- * Run cow/eofblocks scans on the supplied dquots.  We don't know exactly which
- * quota caused an allocation failure, so we make a best effort by including
- * each quota under low free space conditions (less than 1% free space) in the
- * scan.
+ * Build an icwalk filter to free speculative preallocations on inodes in
+ * quota groups that are under low free space conditions. We don't know
+ * exactly which quota caused an allocation failure, so we make a best effort
+ * by including each quota under low free space conditions (less than 1% free
+ * space) in the scan.
  *
- * Callers must not hold any inode's ILOCK.  If requesting a synchronous scan
- * (XFS_ICWALK_FLAG_SYNC), the caller also must not hold any inode's IOLOCK or
- * MMAPLOCK.
+ * Returns true if there are quota groups that need scanning, false otherwise.
  */
-int
-xfs_blockgc_free_dquots(
+static bool
+xfs_blockgc_dquot_filter(
 	struct xfs_mount	*mp,
 	struct xfs_dquot	*udqp,
 	struct xfs_dquot	*gdqp,
 	struct xfs_dquot	*pdqp,
-	unsigned int		iwalk_flags)
+	struct xfs_icwalk	*icw)
 {
-	struct xfs_icwalk	icw = {0};
 	bool			do_work = false;
 
 	if (!udqp && !gdqp && !pdqp)
-		return 0;
+		return false;
 
-	/*
-	 * Run a scan to free blocks using the union filter to cover all
-	 * applicable quotas in a single scan.
-	 */
-	icw.icw_flags = XFS_ICWALK_FLAG_UNION | iwalk_flags;
+	memset(icw, 0, sizeof(*icw));
+	icw->icw_flags = XFS_ICWALK_FLAG_UNION;
 
 	if (XFS_IS_UQUOTA_ENFORCED(mp) && udqp && xfs_dquot_lowsp(udqp)) {
-		icw.icw_uid = make_kuid(mp->m_super->s_user_ns, udqp->q_id);
-		icw.icw_flags |= XFS_ICWALK_FLAG_UID;
+		icw->icw_uid = make_kuid(mp->m_super->s_user_ns, udqp->q_id);
+		icw->icw_flags |= XFS_ICWALK_FLAG_UID;
 		do_work = true;
 	}
 
 	if (XFS_IS_UQUOTA_ENFORCED(mp) && gdqp && xfs_dquot_lowsp(gdqp)) {
-		icw.icw_gid = make_kgid(mp->m_super->s_user_ns, gdqp->q_id);
-		icw.icw_flags |= XFS_ICWALK_FLAG_GID;
+		icw->icw_gid = make_kgid(mp->m_super->s_user_ns, gdqp->q_id);
+		icw->icw_flags |= XFS_ICWALK_FLAG_GID;
 		do_work = true;
 	}
 
 	if (XFS_IS_PQUOTA_ENFORCED(mp) && pdqp && xfs_dquot_lowsp(pdqp)) {
-		icw.icw_prid = pdqp->q_id;
-		icw.icw_flags |= XFS_ICWALK_FLAG_PRID;
+		icw->icw_prid = pdqp->q_id;
+		icw->icw_flags |= XFS_ICWALK_FLAG_PRID;
 		do_work = true;
 	}
 
-	if (!do_work)
+	return do_work;
+}
+
+/*
+ * Run cow/eofblocks scans on the supplied dquots.
+ *
+ * Callers must not hold any inode's ILOCK.  If requesting a synchronous scan
+ * (XFS_ICWALK_FLAG_SYNC), the caller also must not hold any inode's IOLOCK or
+ * MMAPLOCK.
+ */
+int
+xfs_blockgc_free_dquots(
+	struct xfs_mount	*mp,
+	struct xfs_dquot	*udqp,
+	struct xfs_dquot	*gdqp,
+	struct xfs_dquot	*pdqp,
+	unsigned int		iwalk_flags)
+{
+	struct xfs_icwalk	icw;
+
+	if (!xfs_blockgc_dquot_filter(mp, udqp, gdqp, pdqp, &icw))
 		return 0;
 
+	icw.icw_flags |= iwalk_flags;
 	return xfs_blockgc_free_space(mp, &icw);
 }
 
@@ -1697,6 +1764,24 @@ xfs_blockgc_free_quota(
 			xfs_inode_dquot(ip, XFS_DQTYPE_PROJ), iwalk_flags);
 }
 
+/*
+ * Start an async background scan to free speculative preallocations on inodes
+ * in the quota groups attached to the given inode. This does not wait for the
+ * scan to complete and is safe to call with the ILOCK held.
+ */
+void
+xfs_blockgc_free_quota_nowait(
+	struct xfs_inode	*ip)
+{
+	struct xfs_icwalk	icw;
+
+	if (xfs_blockgc_dquot_filter(ip->i_mount,
+			xfs_inode_dquot(ip, XFS_DQTYPE_USER),
+			xfs_inode_dquot(ip, XFS_DQTYPE_GROUP),
+			xfs_inode_dquot(ip, XFS_DQTYPE_PROJ), &icw))
+		xfs_blockgc_free_space_nowait(ip->i_mount, &icw);
+}
+
 /* XFS Inode Cache Walking Code */
 
 /*
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index e68d13125f7b..f55cabb9fcf5 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -62,6 +62,7 @@ int xfs_blockgc_free_dquots(struct xfs_mount *mp, struct xfs_dquot *udqp,
 		unsigned int iwalk_flags);
 int xfs_blockgc_free_quota(struct xfs_inode *ip, unsigned int iwalk_flags);
 int xfs_blockgc_free_space(struct xfs_mount *mp, struct xfs_icwalk *icm);
+void xfs_blockgc_free_quota_nowait(struct xfs_inode *ip);
 void xfs_blockgc_start_flush(struct xfs_mount *mp);
 int xfs_blockgc_flush_all(struct xfs_mount *mp);
 
-- 
2.55.0

[PATCH 08/38] xfs: add async blockgc retry to xfs_trans_reserve_more_inode()

[Dave Chinner]

xfs_trans_reserve_more_inode() can fail with ENOSPC or EDQUOT when
called from contexts that hold the ILOCK. Unlike xfs_trans_alloc_inode()
which can drop the ILOCK, run synchronous blockgc, and retry, this
function cannot use synchronous reclaim because the ILOCK is held.

Synchronous blockgc would deadlock in two ways: first, the blockgc
workers need to take the ILOCK on each inode they scan, so they
would block on the inode we hold. Second, the locked inode pins
journal space that blockgc transactions need for their own commits.

Add a retry loop that on ENOSPC kicks xfs_blockgc_start_flush() to
reclaim global speculative preallocations, and on EDQUOT/ENOSPC
from quota kicks xfs_blockgc_free_quota_nowait() to reclaim quota-
targeted preallocations. After kicking, the loop sleeps for 100ms
via io_schedule_timeout() to allow the background workers to make
progress, then retries once. io_schedule_timeout() is used because
this can be called from the IO submission path where block plugs
may be held.

Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_trans.c | 63 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 609b0fe3d4d3..8e32da47501e 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1130,8 +1130,24 @@ xfs_trans_alloc_inode(
 }
 
 /*
- * Try to reserve more blocks and file quota for a transaction.  Same
- * conditions of usage as xfs_trans_reserve_blocks.
+ * Reserve blocks and quota for an inode that is already locked and joined to
+ * the transaction. If the reservation fails due to ENOSPC or EDQUOT, kick
+ * background blockgc reclaim and retry once after a short delay.
+ *
+ * We cannot use synchronous blockgc flushing here because we hold the ILOCK.
+ * Synchronous blockgc needs to take the ILOCK on each inode it scans, so it
+ * would deadlock on the inode we hold. Furthermore, the locked inode pins
+ * journal space that blockgc transactions need for their own commits, creating
+ * a second deadlock vector. Instead, we kick the background workers and sleep
+ * briefly so that if blockgc is stuck waiting on resources we hold, we still
+ * retry and return ENOSPC rather than deadlocking.
+ *
+ * We use io_schedule_timeout() for the delay because this can be called from
+ * the IO submission path where block plugs may be held. The 100ms delay is
+ * long enough for blockgc workers to lock an inode, run a transaction to free
+ * speculative preallocations and commit it, but short enough that we don't
+ * cause excessive IO latency if blockgc cannot make progress because it is
+ * blocked on resources we hold.
  */
 int
 xfs_trans_reserve_more_inode(
@@ -1143,27 +1159,44 @@ xfs_trans_reserve_more_inode(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	unsigned int		rtx = xfs_extlen_to_rtxlen(mp, rblocks);
+	int			retry = 0;
 	int			error;
 
 	xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
 
-	error = xfs_trans_reserve_blocks(tp, dblocks, rtx);
-	if (error)
-		return error;
-
-	if (!XFS_IS_QUOTA_ON(mp) || xfs_is_quota_inode(&mp->m_sb, I_INO(ip)))
-		return 0;
-
 	if (tp->t_flags & XFS_TRANS_RESERVE)
 		force_quota = true;
 
-	error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks, rblocks,
-			force_quota);
-	if (!error)
-		return 0;
+	do {
+		if (retry)
+			io_schedule_timeout(msecs_to_jiffies(100));
+
+		error = xfs_trans_reserve_blocks(tp, dblocks, rtx);
+		if (error == -ENOSPC) {
+			xfs_blockgc_start_flush(mp);
+			continue;
+		}
+		if (error)
+			return error;
+
+		if (!XFS_IS_QUOTA_ON(mp) ||
+		    xfs_is_quota_inode(&mp->m_sb, I_INO(ip)))
+			return 0;
+
+		error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks,
+				rblocks, force_quota);
+		if (!error)
+			return 0;
+
+		xfs_trans_unreserve_blocks(tp, dblocks, rtx);
+
+		if (error == -EDQUOT || error == -ENOSPC) {
+			xfs_blockgc_free_quota_nowait(ip);
+			continue;
+		}
+		return error;
+	} while (retry++ == 0);
 
-	/* Quota failed, give back the new reservation. */
-	xfs_trans_unreserve_blocks(tp, dblocks, rtx);
 	return error;
 }
 
-- 
2.55.0

[PATCH 09/38] xfs: factor out COW iomap handling from xfs_direct_write_iomap_begin()

[Dave Chinner]

Extract the COW extent allocation and iomap setup logic from
xfs_direct_write_iomap_begin() into a new helper function
xfs_direct_write_cow_iomap_begin().

When the inode is a COW inode, the new function handles the entire
COW path: acquiring the ILOCK exclusively, reading the data fork
extent mapping, checking if COW is needed, allocating COW extents
via xfs_reflink_allocate_cow(), and setting up the iomap/srcmap for
the COW write.

The *nimaps output parameter tells the caller what to do next:
  - *nimaps == 0: COW was fully handled, iomap/srcmap are filled in,
    and the ILOCK has been released. The caller returns immediately.
  - *nimaps > 0: The extent is not shared. The imap is valid and the
    ILOCK is still held, so the caller continues with the normal
    allocation or overwrite IO path.

When the inode is not a COW inode, xfs_direct_write_iomap_begin()
handles the locking and extent lookup itself as before, taking
only a shared ILOCK.

This is a pure refactoring with no functional change, done to prepare
for reworking the COW allocation to handle transaction allocation and
retry logic at the xfs_direct_write_cow_iomap_begin() level.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 1437ea93563c..df9fc7c6a4b9 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -848,6 +848,111 @@ xfs_bmap_hw_atomic_write_possible(
 	return len <= xfs_inode_buftarg(ip)->bt_awu_max;
 }
 
+/*
+ * Handle COW extent allocation and iomap setup for direct writes to reflinked
+ * files.
+ *
+ * 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.
+ */
+static int
+xfs_direct_write_cow_iomap_begin(
+	struct xfs_inode	*ip,
+	loff_t			offset,
+	loff_t			length,
+	unsigned		flags,
+	struct iomap		*iomap,
+	struct iomap		*srcmap,
+	struct xfs_bmbt_irec	*imap,
+	int			*nimaps,
+	unsigned int		*lockmode,
+	u16			iomap_flags)
+{
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_bmbt_irec	cmap;
+	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
+	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, length);
+	bool			shared = false;
+	int			error;
+	u64			seq;
+
+	*lockmode = XFS_ILOCK_EXCL;
+
+relock:
+	error = xfs_ilock_for_iomap(ip, flags, lockmode);
+	if (error)
+		return error;
+
+	/*
+	 * The reflink iflag could have changed since the earlier unlocked
+	 * check, check if it again and relock if needed.
+	 */
+	if (xfs_is_cow_inode(ip) && *lockmode == XFS_ILOCK_SHARED) {
+		xfs_iunlock(ip, *lockmode);
+		*lockmode = XFS_ILOCK_EXCL;
+		goto relock;
+	}
+
+	*nimaps = 1;
+	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, imap,
+			       nimaps, 0);
+	if (error)
+		goto out_unlock;
+
+	if (!imap_needs_cow(ip, flags, imap, *nimaps))
+		return 0;
+
+	error = -EAGAIN;
+	if (flags & IOMAP_NOWAIT)
+		goto out_unlock;
+
+	/* may drop and re-acquire the ilock */
+	error = xfs_reflink_allocate_cow(ip, imap, &cmap, &shared,
+			lockmode,
+			(flags & IOMAP_DIRECT) || IS_DAX(VFS_I(ip)));
+	if (error)
+		goto out_unlock;
+
+	if (!shared)
+		return 0;
+
+	if ((flags & IOMAP_ATOMIC) &&
+	    !xfs_bmap_hw_atomic_write_possible(ip, &cmap,
+			offset_fsb, end_fsb)) {
+		error = -ENOPROTOOPT;
+		goto out_unlock;
+	}
+
+	/*
+	 * COW extent found and allocated. Set up iomap/srcmap and return
+	 * with *nimaps = 0 to tell the caller the COW path is complete.
+	 */
+	*nimaps = 0;
+	length = XFS_FSB_TO_B(mp, cmap.br_startoff + cmap.br_blockcount);
+	trace_xfs_iomap_found(ip, offset, length - offset, XFS_COW_FORK,
+			&cmap);
+	if (imap->br_startblock != HOLESTARTBLOCK) {
+		seq = xfs_iomap_inode_sequence(ip, 0);
+		error = xfs_bmbt_to_iomap(ip, srcmap, imap, flags, 0, seq);
+		if (error)
+			goto out_unlock;
+	}
+	seq = xfs_iomap_inode_sequence(ip, IOMAP_F_SHARED);
+	xfs_iunlock(ip, *lockmode);
+	return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
+
+out_unlock:
+	if (*lockmode)
+		xfs_iunlock(ip, *lockmode);
+	return error;
+}
+
 static int
 xfs_direct_write_iomap_begin(
 	struct inode		*inode,
@@ -859,12 +964,11 @@ xfs_direct_write_iomap_begin(
 {
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_bmbt_irec	imap, cmap;
+	struct xfs_bmbt_irec	imap;
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
 	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, length);
 	xfs_fileoff_t		orig_end_fsb = end_fsb;
 	int			nimaps = 1, error = 0;
-	bool			shared = false;
 	u16			iomap_flags = 0;
 	bool			needs_alloc;
 	unsigned int		lockmode;
@@ -887,57 +991,28 @@ xfs_direct_write_iomap_begin(
 	if (flags & IOMAP_ATOMIC)
 		iomap_flags |= IOMAP_F_ATOMIC_BIO;
 
-	/*
-	 * COW writes may allocate delalloc space or convert unwritten COW
-	 * extents, so we need to make sure to take the lock exclusively here.
-	 */
-	if (xfs_is_cow_inode(ip))
-		lockmode = XFS_ILOCK_EXCL;
-	else
-		lockmode = XFS_ILOCK_SHARED;
-
-relock:
-	error = xfs_ilock_for_iomap(ip, flags, &lockmode);
-	if (error)
-		return error;
-
-	/*
-	 * The reflink iflag could have changed since the earlier unlocked
-	 * check, check if it again and relock if needed.
-	 */
-	if (xfs_is_cow_inode(ip) && lockmode == XFS_ILOCK_SHARED) {
-		xfs_iunlock(ip, lockmode);
-		lockmode = XFS_ILOCK_EXCL;
-		goto relock;
-	}
+	if (xfs_is_cow_inode(ip)) {
+		error = xfs_direct_write_cow_iomap_begin(ip, offset, length,
+				flags, iomap, srcmap, &imap, &nimaps,
+				&lockmode, iomap_flags);
+		if (error)
+			return error;
+		if (!nimaps)
+			return 0;
 
-	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
-			       &nimaps, 0);
-	if (error)
-		goto out_unlock;
+		end_fsb = imap.br_startoff + imap.br_blockcount;
+		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
+	} else {
+		lockmode = XFS_ILOCK_SHARED;
 
-	if (imap_needs_cow(ip, flags, &imap, nimaps)) {
-		error = -EAGAIN;
-		if (flags & IOMAP_NOWAIT)
-			goto out_unlock;
+		error = xfs_ilock_for_iomap(ip, flags, &lockmode);
+		if (error)
+			return error;
 
-		/* may drop and re-acquire the ilock */
-		error = xfs_reflink_allocate_cow(ip, &imap, &cmap, &shared,
-				&lockmode,
-				(flags & IOMAP_DIRECT) || IS_DAX(inode));
+		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
+				&imap, &nimaps, 0);
 		if (error)
 			goto out_unlock;
-		if (shared) {
-			if ((flags & IOMAP_ATOMIC) &&
-			    !xfs_bmap_hw_atomic_write_possible(ip, &cmap,
-					offset_fsb, end_fsb)) {
-				error = -ENOPROTOOPT;
-				goto out_unlock;
-			}
-			goto out_found_cow;
-		}
-		end_fsb = imap.br_startoff + imap.br_blockcount;
-		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
 	}
 
 	needs_alloc = imap_needs_alloc(inode, flags, &imap, nimaps);
@@ -1022,19 +1097,6 @@ xfs_direct_write_iomap_begin(
 	return xfs_bmbt_to_iomap(ip, iomap, &imap, flags,
 				 iomap_flags | IOMAP_F_NEW, seq);
 
-out_found_cow:
-	length = XFS_FSB_TO_B(mp, cmap.br_startoff + cmap.br_blockcount);
-	trace_xfs_iomap_found(ip, offset, length - offset, XFS_COW_FORK, &cmap);
-	if (imap.br_startblock != HOLESTARTBLOCK) {
-		seq = xfs_iomap_inode_sequence(ip, 0);
-		error = xfs_bmbt_to_iomap(ip, srcmap, &imap, flags, 0, seq);
-		if (error)
-			goto out_unlock;
-	}
-	seq = xfs_iomap_inode_sequence(ip, IOMAP_F_SHARED);
-	xfs_iunlock(ip, lockmode);
-	return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
-
 out_unlock:
 	if (lockmode)
 		xfs_iunlock(ip, lockmode);
-- 
2.55.0

[PATCH 10/38] xfs: plumb xfs_trans through xfs_reflink_allocate_cow and fill_cow_hole

[Dave Chinner]

Add a struct xfs_trans pointer as the first parameter to
xfs_reflink_allocate_cow() and xfs_reflink_fill_cow_hole().
All callers currently pass NULL.

This is preparation for having the caller pre-allocate a transaction
and pass it down to avoid needing to cycle the ILOCK inside
fill_cow_hole().

The local transaction variable in fill_cow_hole() is renamed to
local_tp to avoid shadowing the new parameter.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c   |  2 +-
 fs/xfs/xfs_reflink.c | 17 ++++++++++-------
 fs/xfs/xfs_reflink.h |  6 +++---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index df9fc7c6a4b9..e4e64412b431 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -913,7 +913,7 @@ xfs_direct_write_cow_iomap_begin(
 		goto out_unlock;
 
 	/* may drop and re-acquire the ilock */
-	error = xfs_reflink_allocate_cow(ip, imap, &cmap, &shared,
+	error = xfs_reflink_allocate_cow(NULL, ip, imap, &cmap, &shared,
 			lockmode,
 			(flags & IOMAP_DIRECT) || IS_DAX(VFS_I(ip)));
 	if (error)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 480136136635..1fa899048b24 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -430,6 +430,7 @@ xfs_reflink_convert_unwritten(
 
 static int
 xfs_reflink_fill_cow_hole(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
@@ -438,7 +439,7 @@ xfs_reflink_fill_cow_hole(
 	bool			convert_now)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*tp;
+	struct xfs_trans	*local_tp;
 	xfs_filblks_t		resaligned;
 	unsigned int		seq_before = READ_ONCE(ip->i_df.if_seq);
 	unsigned int		dblocks = 0, rblocks = 0;
@@ -460,7 +461,7 @@ xfs_reflink_fill_cow_hole(
 	*lockmode = 0;
 
 	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
-			rblocks, false, &tp);
+			rblocks, false, &local_tp);
 	if (error)
 		return error;
 
@@ -487,20 +488,21 @@ xfs_reflink_fill_cow_hole(
 		goto out_trans_cancel;
 
 	if (found) {
-		xfs_trans_cancel(tp);
+		xfs_trans_cancel(local_tp);
 		goto convert;
 	}
 
 	/* Allocate the entire reservation as unwritten blocks. */
 	nimaps = 1;
-	error = xfs_bmapi_write(tp, ip, imap->br_startoff, imap->br_blockcount,
+	error = xfs_bmapi_write(local_tp, ip, imap->br_startoff,
+			imap->br_blockcount,
 			XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0, cmap,
 			&nimaps);
 	if (error)
 		goto out_trans_cancel;
 
 	xfs_inode_set_cowblocks_tag(ip);
-	error = xfs_trans_commit(tp);
+	error = xfs_trans_commit(local_tp);
 	if (error)
 		return error;
 
@@ -508,7 +510,7 @@ xfs_reflink_fill_cow_hole(
 	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
 
 out_trans_cancel:
-	xfs_trans_cancel(tp);
+	xfs_trans_cancel(local_tp);
 	return error;
 }
 
@@ -597,6 +599,7 @@ xfs_reflink_fill_delalloc(
 /* Allocate all CoW reservations covering a range of blocks in a file. */
 int
 xfs_reflink_allocate_cow(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
@@ -627,7 +630,7 @@ xfs_reflink_allocate_cow(
 	 * Allocate a real extent in the CoW fork.
 	 */
 	if (cmap->br_startoff > imap->br_startoff)
-		return xfs_reflink_fill_cow_hole(ip, imap, cmap, shared,
+		return xfs_reflink_fill_cow_hole(NULL, ip, imap, cmap, shared,
 				lockmode, convert_now);
 
 	/*
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 9d1ed9bb0bee..6a984363edbc 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -30,9 +30,9 @@ int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
 int xfs_bmap_trim_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
 		bool *shared);
 
-int xfs_reflink_allocate_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
-		struct xfs_bmbt_irec *cmap, bool *shared, uint *lockmode,
-		bool convert_now);
+int xfs_reflink_allocate_cow(struct xfs_trans *tp, struct xfs_inode *ip,
+		struct xfs_bmbt_irec *imap, struct xfs_bmbt_irec *cmap,
+		bool *shared, uint *lockmode, bool convert_now);
 extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset,
 		xfs_off_t count);
 int xfs_reflink_convert_cow_locked(struct xfs_inode *ip,
-- 
2.55.0

[PATCH 11/38] xfs: teach xfs_reflink_fill_cow_hole() to use a caller-supplied transaction

[Dave Chinner]

When xfs_reflink_fill_cow_hole() is called with a non-NULL transaction
pointer, use it directly for the COW extent allocation instead of
dropping the ILOCK and allocating a new transaction internally.

When a caller-supplied transaction is used:
- The ILOCK drop/reacquire cycle is skipped entirely
- The caller is responsible for committing or cancelling the transaction
- xfs_find_trim_cow_extent() is still called to re-validate the COW
  fork state

When tp is NULL, the existing behaviour is preserved: the function
drops the ILOCK, allocates a transaction (which re-acquires the ILOCK),
and handles commit/cancel internally.

No callers pass a non-NULL transaction yet, so there is no functional
change.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 1fa899048b24..1ad0569ecdd1 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -439,7 +439,7 @@ xfs_reflink_fill_cow_hole(
 	bool			convert_now)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*local_tp;
+	struct xfs_trans	*local_tp = NULL;
 	xfs_filblks_t		resaligned;
 	unsigned int		seq_before = READ_ONCE(ip->i_df.if_seq);
 	unsigned int		dblocks = 0, rblocks = 0;
@@ -447,6 +447,16 @@ xfs_reflink_fill_cow_hole(
 	int			error;
 	bool			found;
 
+	/*
+	 * If the caller supplied a transaction, use it directly. The caller
+	 * is responsible for commit/cancel and holds the ILOCK.
+	 *
+	 * Otherwise, we need to drop the ILOCK and allocate a transaction
+	 * ourselves, which will re-acquire the ILOCK.
+	 */
+	if (tp)
+		goto allocate;
+
 	resaligned = xfs_aligned_fsb_count(imap->br_startoff,
 		imap->br_blockcount, xfs_get_cowextsz_hint(ip));
 	if (XFS_IS_REALTIME_INODE(ip)) {
@@ -466,6 +476,7 @@ xfs_reflink_fill_cow_hole(
 		return error;
 
 	*lockmode = XFS_ILOCK_EXCL;
+	tp = local_tp;
 
 	/*
 	 * The data fork mapping may have changed while we dropped the ILOCK
@@ -483,18 +494,20 @@ xfs_reflink_fill_cow_hole(
 			goto out_trans_cancel;
 	}
 
+allocate:
 	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
 	if (error || !*shared)
 		goto out_trans_cancel;
 
 	if (found) {
-		xfs_trans_cancel(local_tp);
+		if (local_tp)
+			xfs_trans_cancel(local_tp);
 		goto convert;
 	}
 
 	/* Allocate the entire reservation as unwritten blocks. */
 	nimaps = 1;
-	error = xfs_bmapi_write(local_tp, ip, imap->br_startoff,
+	error = xfs_bmapi_write(tp, ip, imap->br_startoff,
 			imap->br_blockcount,
 			XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0, cmap,
 			&nimaps);
@@ -502,15 +515,18 @@ xfs_reflink_fill_cow_hole(
 		goto out_trans_cancel;
 
 	xfs_inode_set_cowblocks_tag(ip);
-	error = xfs_trans_commit(local_tp);
-	if (error)
-		return error;
+	if (local_tp) {
+		error = xfs_trans_commit(local_tp);
+		if (error)
+			return error;
+	}
 
 convert:
 	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
 
 out_trans_cancel:
-	xfs_trans_cancel(local_tp);
+	if (local_tp)
+		xfs_trans_cancel(local_tp);
 	return error;
 }
 
-- 
2.55.0

[PATCH 12/38] xfs: add transaction retry infrastructure to xfs_direct_write_cow_iomap_begin

[Dave Chinner]

Add the infrastructure for handling COW extent allocation retries with
a pre-allocated transaction in xfs_direct_write_cow_iomap_begin().

When xfs_reflink_allocate_cow() returns -EAGAIN (indicating it needs
a transaction to proceed), the function drops the ILOCK, allocates a
zero-block transaction via xfs_trans_alloc_inode() (which reacquires
the ILOCK), and jumps to retry: to re-read the imap via
xfs_bmapi_read() since the extent tree may have changed while the
ILOCK was not held.

The initial xfs_ilock_for_iomap() call is done once before the retry
label since we always enter with ILOCK_EXCL and xfs_trans_alloc_inode()
re-acquires ILOCK_EXCL on the retry path.

The old relock label and shared-to-exclusive lock upgrade logic is
removed as it is unnecessary - this function always takes ILOCK_EXCL.

Nothing returns -EAGAIN from xfs_reflink_allocate_cow() yet, so there
is no functional change.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index e4e64412b431..017355372a44 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -876,6 +876,7 @@ xfs_direct_write_cow_iomap_begin(
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_bmbt_irec	cmap;
+	struct xfs_trans	*tp = NULL;
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
 	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, length);
 	bool			shared = false;
@@ -884,41 +885,78 @@ xfs_direct_write_cow_iomap_begin(
 
 	*lockmode = XFS_ILOCK_EXCL;
 
-relock:
 	error = xfs_ilock_for_iomap(ip, flags, lockmode);
 	if (error)
 		return error;
 
-	/*
-	 * The reflink iflag could have changed since the earlier unlocked
-	 * check, check if it again and relock if needed.
-	 */
-	if (xfs_is_cow_inode(ip) && *lockmode == XFS_ILOCK_SHARED) {
-		xfs_iunlock(ip, *lockmode);
-		*lockmode = XFS_ILOCK_EXCL;
-		goto relock;
-	}
-
+retry:
 	*nimaps = 1;
 	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, imap,
 			       nimaps, 0);
 	if (error)
 		goto out_unlock;
 
-	if (!imap_needs_cow(ip, flags, imap, *nimaps))
+	if (!imap_needs_cow(ip, flags, imap, *nimaps)) {
+		/*
+		 * Extent is not shared - return the imap and ILOCK to the
+		 * caller for normal IO path processing.
+		 */
+		if (tp) {
+			xfs_trans_cancel(tp);
+			tp = NULL;
+		}
 		return 0;
+	}
 
 	error = -EAGAIN;
 	if (flags & IOMAP_NOWAIT)
 		goto out_unlock;
 
 	/* may drop and re-acquire the ilock */
-	error = xfs_reflink_allocate_cow(NULL, ip, imap, &cmap, &shared,
+	error = xfs_reflink_allocate_cow(tp, ip, imap, &cmap, &shared,
 			lockmode,
 			(flags & IOMAP_DIRECT) || IS_DAX(VFS_I(ip)));
+	if (error == -EAGAIN) {
+		/*
+		 * COW allocation needs a transaction. Drop the ILOCK and
+		 * allocate a transaction, which will re-acquire the ILOCK.
+		 * Then retry the imap lookup since the extent tree may have
+		 * changed while the ILOCK was not held.
+		 */
+		xfs_filblks_t		resaligned;
+		unsigned int		dblocks, rblocks;
+
+		ASSERT(!tp);
+
+		xfs_iunlock(ip, *lockmode);
+
+		resaligned = xfs_aligned_fsb_count(offset_fsb,
+				end_fsb - offset_fsb, xfs_get_cowextsz_hint(ip));
+		if (XFS_IS_REALTIME_INODE(ip)) {
+			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
+			rblocks = resaligned;
+		} else {
+			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
+			rblocks = 0;
+		}
+
+		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
+				dblocks, rblocks, false, &tp);
+		if (error)
+			return error;
+
+		goto retry;
+	}
 	if (error)
 		goto out_unlock;
 
+	if (tp) {
+		error = xfs_trans_commit(tp);
+		tp = NULL;
+		if (error)
+			goto out_unlock;
+	}
+
 	if (!shared)
 		return 0;
 
@@ -948,6 +986,8 @@ xfs_direct_write_cow_iomap_begin(
 	return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
 
 out_unlock:
+	if (tp)
+		xfs_trans_cancel(tp);
 	if (*lockmode)
 		xfs_iunlock(ip, *lockmode);
 	return error;
-- 
2.55.0

[PATCH 13/38] xfs: return -EAGAIN from xfs_reflink_allocate_cow for COW hole without transaction

[Dave Chinner]

When xfs_reflink_allocate_cow() encounters a COW fork hole that needs
a real extent allocated and no transaction has been provided, return
-EAGAIN to tell the caller to allocate a transaction and retry.

When a transaction is provided, pass it through to
xfs_reflink_fill_cow_hole() which will use it directly for the COW
extent allocation without needing to cycle the ILOCK.

This activates the retry loop added in the previous patch: on the
first call (tp == NULL), allocate_cow returns -EAGAIN, the caller
drops the ILOCK, allocates a transaction, and retries. On the
second call (tp != NULL), the transaction is passed down to
fill_cow_hole() and the COW allocation completes atomically
under the ILOCK without cycling it.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 1ad0569ecdd1..56ad76b0f6b9 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -643,16 +643,22 @@ xfs_reflink_allocate_cow(
 
 	/*
 	 * CoW fork does not have an extent and data extent is shared.
-	 * Allocate a real extent in the CoW fork.
+	 * Allocate a real extent in the CoW fork. If the caller has not
+	 * provided a transaction for the allocation, return -EAGAIN to
+	 * tell the caller to allocate a transaction and retry.
 	 */
-	if (cmap->br_startoff > imap->br_startoff)
-		return xfs_reflink_fill_cow_hole(NULL, ip, imap, cmap, shared,
+	if (cmap->br_startoff > imap->br_startoff) {
+		if (!tp)
+			return -EAGAIN;
+		return xfs_reflink_fill_cow_hole(tp, ip, imap, cmap, shared,
 				lockmode, convert_now);
+	}
 
 	/*
 	 * CoW fork has a delalloc reservation. Replace it with a real extent.
 	 * There may or may not be a data fork mapping.
 	 */
+	ASSERT(!tp);
 	if (isnullstartblock(cmap->br_startblock) ||
 	    cmap->br_startblock == DELAYSTARTBLOCK)
 		return xfs_reflink_fill_delalloc(ip, imap, cmap, shared,
-- 
2.55.0

[PATCH 14/38] xfs: remove internal transaction allocation from xfs_reflink_fill_cow_hole

[Dave Chinner]

Now that xfs_reflink_fill_cow_hole() is always called with a valid
caller-supplied transaction, remove the dead internal transaction
allocation path.

This removes the ILOCK drop/reacquire cycle that was previously
needed to allocate a transaction, as the caller now handles
transaction allocation before calling into this function. The
COW extent allocation via xfs_bmapi_write() is now performed
entirely under the caller's ILOCK hold, eliminating the window
where the extent tree could change due to the lock being cycled.

The lockmode parameter is also removed as the function no longer
modifies the lock state.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 75 +++-----------------------------------------
 1 file changed, 5 insertions(+), 70 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 56ad76b0f6b9..a3b4343fd882 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -435,75 +435,20 @@ xfs_reflink_fill_cow_hole(
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
 	bool			*shared,
-	uint			*lockmode,
 	bool			convert_now)
 {
-	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*local_tp = NULL;
-	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;
 	bool			found;
 
-	/*
-	 * If the caller supplied a transaction, use it directly. The caller
-	 * is responsible for commit/cancel and holds the ILOCK.
-	 *
-	 * Otherwise, we need to drop the ILOCK and allocate a transaction
-	 * ourselves, which will re-acquire the ILOCK.
-	 */
-	if (tp)
-		goto allocate;
-
-	resaligned = xfs_aligned_fsb_count(imap->br_startoff,
-		imap->br_blockcount, xfs_get_cowextsz_hint(ip));
-	if (XFS_IS_REALTIME_INODE(ip)) {
-		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
-		rblocks = resaligned;
-	} else {
-		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
-		rblocks = 0;
-	}
-
-	xfs_iunlock(ip, *lockmode);
-	*lockmode = 0;
-
-	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
-			rblocks, false, &local_tp);
-	if (error)
-		return error;
-
-	*lockmode = XFS_ILOCK_EXCL;
-	tp = local_tp;
-
-	/*
-	 * 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;
-	}
+	ASSERT(tp);
 
-allocate:
 	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
 	if (error || !*shared)
-		goto out_trans_cancel;
+		return error;
 
-	if (found) {
-		if (local_tp)
-			xfs_trans_cancel(local_tp);
+	if (found)
 		goto convert;
-	}
 
 	/* Allocate the entire reservation as unwritten blocks. */
 	nimaps = 1;
@@ -512,22 +457,12 @@ xfs_reflink_fill_cow_hole(
 			XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0, cmap,
 			&nimaps);
 	if (error)
-		goto out_trans_cancel;
+		return error;
 
 	xfs_inode_set_cowblocks_tag(ip);
-	if (local_tp) {
-		error = xfs_trans_commit(local_tp);
-		if (error)
-			return error;
-	}
 
 convert:
 	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
-
-out_trans_cancel:
-	if (local_tp)
-		xfs_trans_cancel(local_tp);
-	return error;
 }
 
 static int
@@ -651,7 +586,7 @@ xfs_reflink_allocate_cow(
 		if (!tp)
 			return -EAGAIN;
 		return xfs_reflink_fill_cow_hole(tp, ip, imap, cmap, shared,
-				lockmode, convert_now);
+				convert_now);
 	}
 
 	/*
-- 
2.55.0

[PATCH 15/38] xfs: use zero-block transaction with xfs_trans_reserve_more_inode for COW holes

[Dave Chinner]

Change the COW transaction allocation strategy to use a zero-block
reservation at the top level and defer block reservation to the
callee that knows the actual extent state.

When xfs_reflink_allocate_cow() returns -EAGAIN, the ILOCK must be
dropped to allocate a transaction. Because the extent tree can change
while the ILOCK is not held, we cannot determine what extent type
will be found once we've regained the ILOCK. 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.

In xfs_reflink_fill_cow_hole(), add an xfs_trans_reserve_more_inode()
call before xfs_bmapi_write() to reserve the blocks needed for the
COW extent allocation. The reservation is computed from the current
imap which is stable under the ILOCK. An ASSERT verifies the
transaction has not been dirtied, confirming it is safe to cancel
on ENOSPC.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c   | 28 +++++++++++-----------------
 fs/xfs/xfs_reflink.c | 25 +++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 017355372a44..777048e6a2ca 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -919,29 +919,23 @@ xfs_direct_write_cow_iomap_begin(
 	if (error == -EAGAIN) {
 		/*
 		 * COW allocation needs a transaction. Drop the ILOCK and
-		 * allocate a transaction, which will re-acquire the ILOCK.
-		 * Then retry the imap lookup since the extent tree may have
-		 * changed while the ILOCK was not held.
+		 * 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_filblks_t		resaligned;
-		unsigned int		dblocks, rblocks;
-
 		ASSERT(!tp);
 
 		xfs_iunlock(ip, *lockmode);
 
-		resaligned = xfs_aligned_fsb_count(offset_fsb,
-				end_fsb - offset_fsb, xfs_get_cowextsz_hint(ip));
-		if (XFS_IS_REALTIME_INODE(ip)) {
-			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
-			rblocks = resaligned;
-		} else {
-			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
-			rblocks = 0;
-		}
-
 		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
-				dblocks, rblocks, false, &tp);
+				0, 0, false, &tp);
 		if (error)
 			return error;
 
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index a3b4343fd882..373ce9fea2a8 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -437,6 +437,9 @@ xfs_reflink_fill_cow_hole(
 	bool			*shared,
 	bool			convert_now)
 {
+	struct xfs_mount	*mp = ip->i_mount;
+	xfs_filblks_t		resaligned;
+	unsigned int		dblocks = 0, rblocks = 0;
 	int			nimaps;
 	int			error;
 	bool			found;
@@ -450,6 +453,28 @@ xfs_reflink_fill_cow_hole(
 	if (found)
 		goto convert;
 
+	/*
+	 * Reserve blocks for the COW extent allocation. The transaction was
+	 * allocated with a zero-block reservation because the caller could
+	 * not determine the block reservation required until the extent
+	 * state was known under the ILOCK. The transaction has not been
+	 * dirtied yet, so on ENOSPC it can safely be cancelled by the caller.
+	 */
+	ASSERT(!(tp->t_flags & XFS_TRANS_DIRTY));
+
+	resaligned = xfs_aligned_fsb_count(imap->br_startoff,
+			imap->br_blockcount, xfs_get_cowextsz_hint(ip));
+	if (XFS_IS_REALTIME_INODE(ip)) {
+		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
+		rblocks = resaligned;
+	} else {
+		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
+	}
+
+	error = xfs_trans_reserve_more_inode(tp, ip, dblocks, rblocks, false);
+	if (error)
+		return error;
+
 	/* Allocate the entire reservation as unwritten blocks. */
 	nimaps = 1;
 	error = xfs_bmapi_write(tp, ip, imap->br_startoff,
-- 
2.55.0

[PATCH 16/38] xfs: change *tp to **tpp in COW allocation call chain

[Dave Chinner]

Change the transaction parameter from struct xfs_trans *tp to
struct xfs_trans **tpp in xfs_reflink_fill_cow_hole(),
xfs_reflink_allocate_cow(), and the header declaration. Update
the caller in xfs_direct_write_cow_iomap_begin() to pass &tp.

This is preparation for xfs_reflink_fill_delalloc() using
xfs_trans_roll_inode() to implement rolling transactions, which
modifies the transaction pointer and requires **tpp semantics.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c   |  2 +-
 fs/xfs/xfs_reflink.c | 19 ++++++++++---------
 fs/xfs/xfs_reflink.h |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 777048e6a2ca..2f18a8f62e39 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -913,7 +913,7 @@ xfs_direct_write_cow_iomap_begin(
 		goto out_unlock;
 
 	/* may drop and re-acquire the ilock */
-	error = xfs_reflink_allocate_cow(tp, ip, imap, &cmap, &shared,
+	error = xfs_reflink_allocate_cow(&tp, ip, imap, &cmap, &shared,
 			lockmode,
 			(flags & IOMAP_DIRECT) || IS_DAX(VFS_I(ip)));
 	if (error == -EAGAIN) {
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 373ce9fea2a8..6ca99ddf9976 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -430,7 +430,7 @@ xfs_reflink_convert_unwritten(
 
 static int
 xfs_reflink_fill_cow_hole(
-	struct xfs_trans	*tp,
+	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip,
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
@@ -444,7 +444,7 @@ xfs_reflink_fill_cow_hole(
 	int			error;
 	bool			found;
 
-	ASSERT(tp);
+	ASSERT(*tpp);
 
 	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
 	if (error || !*shared)
@@ -460,7 +460,7 @@ xfs_reflink_fill_cow_hole(
 	 * state was known under the ILOCK. The transaction has not been
 	 * dirtied yet, so on ENOSPC it can safely be cancelled by the caller.
 	 */
-	ASSERT(!(tp->t_flags & XFS_TRANS_DIRTY));
+	ASSERT(!((*tpp)->t_flags & XFS_TRANS_DIRTY));
 
 	resaligned = xfs_aligned_fsb_count(imap->br_startoff,
 			imap->br_blockcount, xfs_get_cowextsz_hint(ip));
@@ -471,13 +471,14 @@ xfs_reflink_fill_cow_hole(
 		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
 	}
 
-	error = xfs_trans_reserve_more_inode(tp, ip, dblocks, rblocks, false);
+	error = xfs_trans_reserve_more_inode(*tpp, ip, dblocks, rblocks,
+			false);
 	if (error)
 		return error;
 
 	/* Allocate the entire reservation as unwritten blocks. */
 	nimaps = 1;
-	error = xfs_bmapi_write(tp, ip, imap->br_startoff,
+	error = xfs_bmapi_write(*tpp, ip, imap->br_startoff,
 			imap->br_blockcount,
 			XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0, cmap,
 			&nimaps);
@@ -575,7 +576,7 @@ xfs_reflink_fill_delalloc(
 /* Allocate all CoW reservations covering a range of blocks in a file. */
 int
 xfs_reflink_allocate_cow(
-	struct xfs_trans	*tp,
+	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip,
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
@@ -608,9 +609,9 @@ xfs_reflink_allocate_cow(
 	 * tell the caller to allocate a transaction and retry.
 	 */
 	if (cmap->br_startoff > imap->br_startoff) {
-		if (!tp)
+		if (!*tpp)
 			return -EAGAIN;
-		return xfs_reflink_fill_cow_hole(tp, ip, imap, cmap, shared,
+		return xfs_reflink_fill_cow_hole(tpp, ip, imap, cmap, shared,
 				convert_now);
 	}
 
@@ -618,7 +619,7 @@ xfs_reflink_allocate_cow(
 	 * CoW fork has a delalloc reservation. Replace it with a real extent.
 	 * There may or may not be a data fork mapping.
 	 */
-	ASSERT(!tp);
+	ASSERT(!*tpp);
 	if (isnullstartblock(cmap->br_startblock) ||
 	    cmap->br_startblock == DELAYSTARTBLOCK)
 		return xfs_reflink_fill_delalloc(ip, imap, cmap, shared,
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 6a984363edbc..3da8374829c3 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -30,7 +30,7 @@ int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
 int xfs_bmap_trim_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
 		bool *shared);
 
-int xfs_reflink_allocate_cow(struct xfs_trans *tp, struct xfs_inode *ip,
+int xfs_reflink_allocate_cow(struct xfs_trans **tpp, struct xfs_inode *ip,
 		struct xfs_bmbt_irec *imap, struct xfs_bmbt_irec *cmap,
 		bool *shared, uint *lockmode, bool convert_now);
 extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset,
-- 
2.55.0

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

[Dave Chinner]

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 <dgc@kernel.org>
---
 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

[PATCH 18/38] xfs: return -EAGAIN from xfs_reflink_allocate_cow for all allocation cases

[Dave Chinner]

Consolidate the -EAGAIN transaction check for both COW hole and
delalloc allocation paths. Both cases require a transaction to
proceed, so lift the *tpp == NULL check above both branches and
return -EAGAIN from a single point.

This activates the retry loop for the delalloc case, which was
previously handled by cycling the ILOCK internally in
xfs_reflink_fill_delalloc().

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 3e8eb16ce076..cc3c1caee2d8 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -616,23 +616,20 @@ xfs_reflink_allocate_cow(
 				convert_now);
 
 	/*
-	 * CoW fork does not have an extent and data extent is shared.
-	 * Allocate a real extent in the CoW fork. If the caller has not
-	 * provided a transaction for the allocation, return -EAGAIN to
-	 * tell the caller to allocate a transaction and retry.
+	 * Allocation is now required, so we need a transaction context from
+	 * the caller if it hasn't already supplied one.
 	 */
-	if (cmap->br_startoff > imap->br_startoff) {
-		if (!*tpp)
-			return -EAGAIN;
+	if (!*tpp)
+		return -EAGAIN;
+
+	if (cmap->br_startoff > imap->br_startoff)
 		return xfs_reflink_fill_cow_hole(tpp, ip, imap, cmap, shared,
 				convert_now);
-	}
 
 	/*
 	 * CoW fork has a delalloc reservation. Replace it with a real extent.
 	 * There may or may not be a data fork mapping.
 	 */
-	ASSERT(!*tpp);
 	if (isnullstartblock(cmap->br_startblock) ||
 	    cmap->br_startblock == DELAYSTARTBLOCK)
 		return xfs_reflink_fill_delalloc(tpp, ip, imap, cmap, shared,
-- 
2.55.0

[PATCH 19/38] xfs: remove dead internal transaction allocation from xfs_reflink_fill_delalloc

[Dave Chinner]

Now that xfs_reflink_fill_delalloc() is always called with a valid
caller-supplied transaction via the -EAGAIN retry mechanism, remove
the dead internal transaction allocation path.

This removes the ILOCK drop/reacquire cycle that was previously
needed to allocate a transaction. The delalloc conversion loop runs
entirely under the caller's ILOCK hold using rolling transactions,
eliminating the window where racing DIO writes could cause stale
extent state.

The lockmode parameter is also removed as the function no longer
modifies the lock state.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 59 ++++++--------------------------------------
 1 file changed, 8 insertions(+), 51 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index cc3c1caee2d8..e40f7afc2c7b 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -498,51 +498,20 @@ xfs_reflink_fill_delalloc(
 	struct xfs_bmbt_irec	*imap,
 	struct xfs_bmbt_irec	*cmap,
 	bool			*shared,
-	uint			*lockmode,
 	bool			convert_now)
 {
-	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*tp = *tpp;
 	int			nimaps;
 	int			error;
 	bool			found;
 
-	if (!tp) {
-		unsigned int	seq_before = READ_ONCE(ip->i_df.if_seq);
-
-		xfs_iunlock(ip, *lockmode);
-		*lockmode = 0;
-
-		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, 0, 0,
-				false, &tp);
-		if (error)
-			return error;
-
-		*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;
-		}
-	}
+	ASSERT(tp);
 
 	do {
 		error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
 				&found);
 		if (error || !*shared)
-			goto out_trans_cancel;
+			goto out_error;
 
 		if (found)
 			break;
@@ -559,30 +528,18 @@ xfs_reflink_fill_delalloc(
 				XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0,
 				cmap, &nimaps);
 		if (error)
-			goto out_trans_cancel;
+			goto out_error;
 
 		xfs_inode_set_cowblocks_tag(ip);
 
 		error = xfs_defer_finish(&tp);
 		if (error)
-			goto out_trans_cancel;
+			goto out_error;
 	} while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
 
-	if (*tpp) {
-		*tpp = tp;
-	} else {
-		error = xfs_trans_commit(tp);
-		if (error)
-			return error;
-	}
-
-	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
-
-out_trans_cancel:
-	if (*tpp)
-		*tpp = tp;
-	else
-		xfs_trans_cancel(tp);
+	error = xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
+out_error:
+	*tpp = tp;
 	return error;
 }
 
@@ -633,7 +590,7 @@ xfs_reflink_allocate_cow(
 	if (isnullstartblock(cmap->br_startblock) ||
 	    cmap->br_startblock == DELAYSTARTBLOCK)
 		return xfs_reflink_fill_delalloc(tpp, ip, imap, cmap, shared,
-				lockmode, convert_now);
+				convert_now);
 
 	/* Shouldn't get here. */
 	ASSERT(0);
-- 
2.55.0

[PATCH 20/38] xfs: plumb struct xfs_trans *tp into xfs_bmapi_convert_one_delalloc

[Dave Chinner]

Add a struct xfs_trans *tp parameter to xfs_bmapi_convert_one_delalloc().
When a non-NULL transaction is provided, use it directly instead of
allocating one internally, and skip the ILOCK acquisition and
xfs_trans_ijoin() since the caller already holds the ILOCK with the
inode joined to the transaction.

On success with a caller-supplied transaction, the function returns
without committing or unlocking. On error, it returns the error
without cancelling or unlocking.

When tp is NULL, the existing behaviour is preserved.

All callers currently pass NULL, so there is no functional change.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 43 ++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 47c4d5c52b95..7f3a3a02a035 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4351,6 +4351,7 @@ xfs_bmapi_write(
  */
 static int
 xfs_bmapi_convert_one_delalloc(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	int			whichfork,
 	xfs_off_t		offset,
@@ -4362,23 +4363,27 @@ xfs_bmapi_convert_one_delalloc(
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
 	struct xfs_bmalloca	bma = { NULL };
 	uint16_t		flags = 0;
-	struct xfs_trans	*tp;
+	struct xfs_trans	*local_tp = NULL;
 	int			error;
 
 	if (whichfork == XFS_COW_FORK)
 		flags |= IOMAP_F_SHARED;
 
-	/*
-	 * Space for the extent and indirect blocks was reserved when the
-	 * delalloc extent was created so there's no need to do so here.
-	 */
-	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0,
-				XFS_TRANS_RESERVE, &tp);
-	if (error)
-		return error;
+	if (!tp) {
+		/*
+		 * Space for the extent and indirect blocks was reserved when
+		 * the delalloc extent was created so there's no need to do so
+		 * here.
+		 */
+		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0,
+					XFS_TRANS_RESERVE, &local_tp);
+		if (error)
+			return error;
 
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	xfs_trans_ijoin(tp, ip, 0);
+		xfs_ilock(ip, XFS_ILOCK_EXCL);
+		xfs_trans_ijoin(local_tp, ip, 0);
+		tp = local_tp;
+	}
 
 	/*
 	 * Look up the extent before extending the extent count so that we
@@ -4470,15 +4475,19 @@ xfs_bmapi_convert_one_delalloc(
 		goto out_finish;
 
 	xfs_bmapi_finish(&bma, whichfork, 0);
-	error = xfs_trans_commit(tp);
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	if (local_tp) {
+		error = xfs_trans_commit(local_tp);
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	}
 	return error;
 
 out_finish:
 	xfs_bmapi_finish(&bma, whichfork, error);
 out_trans_cancel:
-	xfs_trans_cancel(tp);
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	if (local_tp) {
+		xfs_trans_cancel(local_tp);
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	}
 	return error;
 }
 
@@ -4503,8 +4512,8 @@ xfs_bmapi_convert_delalloc(
 	 * delalloc extent if free space is sufficiently fragmented.
 	 */
 	do {
-		error = xfs_bmapi_convert_one_delalloc(ip, whichfork, offset,
-					iomap, seq);
+		error = xfs_bmapi_convert_one_delalloc(NULL, ip, whichfork,
+					offset, iomap, seq);
 		if (error)
 			return error;
 	} while (iomap->offset + iomap->length <= offset);
-- 
2.55.0

[PATCH 21/38] xfs: use rolling transaction in xfs_bmapi_convert_delalloc

[Dave Chinner]

Move the transaction allocation, ILOCK acquisition and inode join
up from xfs_bmapi_convert_one_delalloc() into
xfs_bmapi_convert_delalloc(). Pass the transaction down to each
call to xfs_bmapi_convert_one_delalloc(), and use xfs_defer_finish()
after each iteration to process deferred operations and roll the
transaction while keeping the ILOCK held.

The rolling transaction keeps the ILOCK held across the entire
conversion loop, making this multi-extent conversion operation
atomic with respect to other concurrent extent operations.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7f3a3a02a035..4eaf67b51c8d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4503,8 +4503,26 @@ xfs_bmapi_convert_delalloc(
 	struct iomap		*iomap,
 	unsigned int		*seq)
 {
+	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_trans	*tp;
 	int			error;
 
+	/*
+	 * Allocate the transaction, take the ILOCK and join the inode to it.
+	 * Space for the extent and indirect blocks was reserved when the
+	 * delalloc extent was created so there's no need to reserve blocks.
+	 * The rolling transaction keeps the ILOCK held across the entire
+	 * conversion loop, making this multi-extent conversion operation
+	 * atomic with respect to other concurrent extent operations.
+	 */
+	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0,
+				XFS_TRANS_RESERVE, &tp);
+	if (error)
+		return error;
+
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	xfs_trans_ijoin(tp, ip, 0);
+
 	/*
 	 * Attempt to allocate whatever delalloc extent currently backs offset
 	 * and put the result into iomap.  Allocate in a loop because it may
@@ -4512,13 +4530,24 @@ xfs_bmapi_convert_delalloc(
 	 * delalloc extent if free space is sufficiently fragmented.
 	 */
 	do {
-		error = xfs_bmapi_convert_one_delalloc(NULL, ip, whichfork,
+		error = xfs_bmapi_convert_one_delalloc(tp, ip, whichfork,
 					offset, iomap, seq);
 		if (error)
-			return error;
+			goto out_trans_cancel;
+
+		error = xfs_defer_finish(&tp);
+		if (error)
+			goto out_trans_cancel;
 	} while (iomap->offset + iomap->length <= offset);
 
-	return 0;
+	error = xfs_trans_commit(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	return error;
+
+out_trans_cancel:
+	xfs_trans_cancel(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	return error;
 }
 
 int
-- 
2.55.0

[PATCH 22/38] xfs: remove dead internal transaction path from xfs_bmapi_convert_one_delalloc

[Dave Chinner]

Now that xfs_bmapi_convert_one_delalloc() is always called with a
valid caller-supplied transaction from xfs_bmapi_convert_delalloc(),
remove the dead internal transaction allocation path.

The function is now a pure helper that converts one delalloc extent
using the caller's transaction and returns errors directly without
managing the transaction or lock lifecycle.

The success and error finish paths are collapsed into a single
out_finish label since both call xfs_bmapi_finish() and return error.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_bmap.c | 41 +++++-----------------------------------
 1 file changed, 5 insertions(+), 36 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4eaf67b51c8d..181333d3dee6 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4363,28 +4363,13 @@ xfs_bmapi_convert_one_delalloc(
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
 	struct xfs_bmalloca	bma = { NULL };
 	uint16_t		flags = 0;
-	struct xfs_trans	*local_tp = NULL;
 	int			error;
 
+	ASSERT(tp);
+
 	if (whichfork == XFS_COW_FORK)
 		flags |= IOMAP_F_SHARED;
 
-	if (!tp) {
-		/*
-		 * Space for the extent and indirect blocks was reserved when
-		 * the delalloc extent was created so there's no need to do so
-		 * here.
-		 */
-		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0,
-					XFS_TRANS_RESERVE, &local_tp);
-		if (error)
-			return error;
-
-		xfs_ilock(ip, XFS_ILOCK_EXCL);
-		xfs_trans_ijoin(local_tp, ip, 0);
-		tp = local_tp;
-	}
-
 	/*
 	 * Look up the extent before extending the extent count so that we
 	 * don't dirty the transaction if there is nothing to convert. A
@@ -4398,8 +4383,7 @@ xfs_bmapi_convert_one_delalloc(
 		 * might have moved the extent to the data fork in the meantime.
 		 */
 		WARN_ON_ONCE(whichfork != XFS_COW_FORK);
-		error = -EAGAIN;
-		goto out_trans_cancel;
+		return -EAGAIN;
 	}
 
 	/*
@@ -4411,13 +4395,13 @@ xfs_bmapi_convert_one_delalloc(
 				xfs_iomap_inode_sequence(ip, flags));
 		if (seq)
 			*seq = READ_ONCE(ifp->if_seq);
-		goto out_trans_cancel;
+		return 0;
 	}
 
 	error = xfs_iext_count_extend(tp, ip, whichfork,
 			XFS_IEXT_ADD_NOSPLIT_CNT);
 	if (error)
-		goto out_trans_cancel;
+		return error;
 
 	bma.tp = tp;
 	bma.ip = ip;
@@ -4471,23 +4455,8 @@ xfs_bmapi_convert_one_delalloc(
 
 	error = xfs_bmap_btree_to_extents(tp, ip, bma.cur, &bma.logflags,
 			whichfork);
-	if (error)
-		goto out_finish;
-
-	xfs_bmapi_finish(&bma, whichfork, 0);
-	if (local_tp) {
-		error = xfs_trans_commit(local_tp);
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	}
-	return error;
-
 out_finish:
 	xfs_bmapi_finish(&bma, whichfork, error);
-out_trans_cancel:
-	if (local_tp) {
-		xfs_trans_cancel(local_tp);
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	}
 	return error;
 }
 
-- 
2.55.0

[PATCH 23/38] xfs: add block reservation renewal to xfs_defer_finish

[Dave Chinner]

Add infrastructure to automatically renew block and RT extent
reservations after deferred operations have been processed by
xfs_defer_finish().

Unlike log reservations which are renewed by xfs_log_regrant() on
every roll, block reservations are carried forward by
xfs_trans_dup() with consumed blocks subtracted. Over multiple
rolls, the reservation can be depleted even though each iteration
of the caller needs the same reservation.

Add a new XFS_TRANS_RENEW_BLKRES flag. When set, the original
block reservation is recorded in t_blk_res_orig (and t_rtx_res_orig
for RT extents) at reservation time and propagated through
xfs_trans_dup(). After xfs_defer_finish() completes all deferred
operations and performs the final roll, xfs_trans_regrant_blkres()
reserves the deficit between t_blk_res and t_blk_res_orig from the
free space pool, restoring the original reservation for the next
iteration.

The _orig values are set in xfs_trans_reserve() and
xfs_trans_reserve_more_inode() so that they are correctly captured
regardless of how the block reservation was established.

The regrant is not performed during the internal rolls in
xfs_defer_finish_noroll() because the original reservation already
contains all the space needed for the deferred op chain. The regrant
after the final roll operates on a clean transaction, so the caller
can safely cancel on ENOSPC without causing a filesystem shutdown.

xfs_defer_finish() is changed to unconditionally roll the transaction
after deferred op processing so that the regrant always occurs.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/libxfs/xfs_defer.c  | 38 +++++++++++++++++++++++++-----------
 fs/xfs/libxfs/xfs_shared.h |  3 +++
 fs/xfs/xfs_trans.c         | 40 +++++++++++++++++++++++++++++++++++---
 fs/xfs/xfs_trans.h         |  3 +++
 4 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 89501e8bd2f8..f1807dadafb2 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -725,6 +725,24 @@ xfs_defer_finish_noroll(
 	return error;
 }
 
+/*
+ * Finish all deferred ops and roll the transaction. The transaction is
+ * always rolled unconditionally so that the block reservation can be
+ * regranted after all deferred ops have completed.
+ *
+ * The block reservation regrant is not performed during the internal
+ * transaction rolls in xfs_defer_finish_noroll() because the original
+ * reservation already contains all the space needed for the deferred op
+ * chain. Regranting during internal rolls would risk unnecessary ENOSPC
+ * errors in the middle of a deferred op chain that cannot be safely
+ * aborted.
+ *
+ * The regrant is done after the final roll when the transaction is clean,
+ * replenishing whatever blocks were consumed by both the caller's
+ * modifications and the deferred operations. If the regrant fails with
+ * ENOSPC, the caller can safely cancel the clean transaction without
+ * causing a filesystem shutdown.
+ */
 int
 xfs_defer_finish(
 	struct xfs_trans	**tp)
@@ -734,22 +752,20 @@ xfs_defer_finish(
 #endif
 	int			error;
 
-	/*
-	 * Finish and roll the transaction once more to avoid returning to the
-	 * caller with a dirty transaction.
-	 */
 	error = xfs_defer_finish_noroll(tp);
 	if (error)
 		return error;
-	if ((*tp)->t_flags & XFS_TRANS_DIRTY) {
-		error = xfs_defer_trans_roll(tp);
-		if (error) {
-			xfs_force_shutdown((*tp)->t_mountp,
-					   SHUTDOWN_CORRUPT_INCORE);
-			return error;
-		}
+
+	error = xfs_defer_trans_roll(tp);
+	if (error) {
+		xfs_force_shutdown((*tp)->t_mountp, SHUTDOWN_CORRUPT_INCORE);
+		return error;
 	}
 
+	error = xfs_trans_regrant_blkres(*tp);
+	if (error)
+		return error;
+
 	/* Reset LOWMODE now that we've finished all the dfops. */
 #ifdef DEBUG
 	list_for_each_entry(dfp, &(*tp)->t_dfops, dfp_list)
diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
index b1e0d9bc1f7d..e3909d46f3cd 100644
--- a/fs/xfs/libxfs/xfs_shared.h
+++ b/fs/xfs/libxfs/xfs_shared.h
@@ -164,6 +164,9 @@ void	xfs_log_get_max_trans_res(struct xfs_mount *mp,
 /* Transaction has locked the rtbitmap and rtsum inodes */
 #define XFS_TRANS_RTBITMAP_LOCKED	(1u << 9)
 
+/* Renew block reservation on transaction roll */
+#define XFS_TRANS_RENEW_BLKRES		(1u << 10)
+
 /*
  * Field values for xfs_trans_mod_sb.
  */
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 8e32da47501e..6a274e3aed36 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -112,16 +112,19 @@ xfs_trans_dup(
 	ntp->t_flags = XFS_TRANS_PERM_LOG_RES |
 		       (tp->t_flags & XFS_TRANS_RESERVE) |
 		       (tp->t_flags & XFS_TRANS_NO_WRITECOUNT) |
-		       (tp->t_flags & XFS_TRANS_RES_FDBLKS);
+		       (tp->t_flags & XFS_TRANS_RES_FDBLKS) |
+		       (tp->t_flags & XFS_TRANS_RENEW_BLKRES);
 	/* We gave our writer reference to the new transaction */
 	tp->t_flags |= XFS_TRANS_NO_WRITECOUNT;
 	ntp->t_ticket = xfs_log_ticket_get(tp->t_ticket);
 
 	ASSERT(tp->t_blk_res >= tp->t_blk_res_used);
 	ntp->t_blk_res = tp->t_blk_res - tp->t_blk_res_used;
+	ntp->t_blk_res_orig = tp->t_blk_res_orig;
 	tp->t_blk_res = tp->t_blk_res_used;
 
 	ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
+	ntp->t_rtx_res_orig = tp->t_rtx_res_orig;
 	tp->t_rtx_res = tp->t_rtx_res_used;
 
 	/* move deferred ops over to the new tp */
@@ -203,6 +206,8 @@ xfs_trans_reserve(
 	error = xfs_trans_reserve_blocks(tp, blocks, rtextents);
 	if (error)
 		return error;
+	tp->t_blk_res_orig = tp->t_blk_res;
+	tp->t_rtx_res_orig = tp->t_rtx_res;
 
 	/*
 	 * Reserve the log space needed for this transaction.
@@ -1009,6 +1014,31 @@ xfs_trans_cancel(
 	xfs_trans_free(tp);
 }
 
+/*
+ * Renew the block and RT extent reservations from the free space pool.
+ * The consumed counts were carried forward by xfs_trans_dup() so we know
+ * exactly how many blocks need to be reserved to restore the original
+ * reservation.
+ */
+int
+xfs_trans_regrant_blkres(
+	struct xfs_trans	*tp)
+{
+	unsigned int		blk_deficit;
+	unsigned int		rtx_deficit;
+
+	if (!(tp->t_flags & XFS_TRANS_RENEW_BLKRES))
+		return 0;
+
+	blk_deficit = tp->t_blk_res_orig - tp->t_blk_res;
+	rtx_deficit = tp->t_rtx_res_orig - tp->t_rtx_res;
+
+	if (!blk_deficit && !rtx_deficit)
+		return 0;
+
+	return xfs_trans_reserve_blocks(tp, blk_deficit, rtx_deficit);
+}
+
 /*
  * Roll from one trans in the sequence of PERMANENT transactions to the next:
  * permanent transactions are only flushed out when committed with
@@ -1181,12 +1211,12 @@ xfs_trans_reserve_more_inode(
 
 		if (!XFS_IS_QUOTA_ON(mp) ||
 		    xfs_is_quota_inode(&mp->m_sb, I_INO(ip)))
-			return 0;
+			break;
 
 		error = xfs_trans_reserve_quota_nblks(tp, ip, dblocks,
 				rblocks, force_quota);
 		if (!error)
-			return 0;
+			break;
 
 		xfs_trans_unreserve_blocks(tp, dblocks, rtx);
 
@@ -1197,6 +1227,10 @@ xfs_trans_reserve_more_inode(
 		return error;
 	} while (retry++ == 0);
 
+	if (!error) {
+		tp->t_blk_res_orig = tp->t_blk_res;
+		tp->t_rtx_res_orig = tp->t_rtx_res;
+	}
 	return error;
 }
 
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index de77b617bccf..cf527d57d40b 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -127,8 +127,10 @@ typedef struct xfs_trans {
 	unsigned int		t_log_count;	/* count for perm log res */
 	unsigned int		t_blk_res;	/* # of blocks resvd */
 	unsigned int		t_blk_res_used;	/* # of resvd blocks used */
+	unsigned int		t_blk_res_orig;	/* used with XFS_TRANS_RENEW_BLKRES */
 	unsigned int		t_rtx_res;	/* # of rt extents resvd */
 	unsigned int		t_rtx_res_used;	/* # of resvd rt extents used */
+	unsigned int		t_rtx_res_orig;	/* used with XFS_TRANS_RENEW_BLKRES */
 	unsigned int		t_flags;	/* misc flags */
 	xfs_agnumber_t		t_highest_agno;	/* highest AGF locked */
 	struct xlog_ticket	*t_ticket;	/* log mgr ticket */
@@ -239,6 +241,7 @@ void		xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
 int		xfs_trans_commit(struct xfs_trans *);
 int		xfs_trans_roll(struct xfs_trans **);
 int		xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
+int		xfs_trans_regrant_blkres(struct xfs_trans *);
 void		xfs_trans_cancel(xfs_trans_t *);
 int		xfs_trans_ail_init(struct xfs_mount *);
 void		xfs_trans_ail_destroy(struct xfs_mount *);
-- 
2.55.0

[PATCH 24/38] xfs: factor out xfs_iomap_write_unwritten_one helper

[Dave Chinner]

Extract the per-iteration unwritten extent conversion work from
xfs_iomap_write_unwritten() into a new helper function
xfs_iomap_write_unwritten_one().

The helper takes a caller-supplied transaction and performs the
extent count extension, unwritten-to-written conversion via
xfs_bmapi_write(), and inode size update for a single extent.

The startblock validation check remains in the caller after the
transaction commit, as it is a corruption detection check that
should warn and return an error without causing a filesystem
shutdown from cancelling a dirty transaction.

This is a pure refactoring with no functional change, done to
prepare for converting the loop to use rolling transactions.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 2f18a8f62e39..16feac3ad3bd 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -609,6 +609,59 @@ xfs_iomap_prealloc_size(
 	return alloc_blocks;
 }
 
+/*
+ * Convert a single unwritten extent to a real extent using the supplied
+ * transaction. Returns the converted extent via @imap.
+ */
+static int
+xfs_iomap_write_unwritten_one(
+	struct xfs_trans	*tp,
+	struct xfs_inode	*ip,
+	xfs_fileoff_t		offset_fsb,
+	xfs_filblks_t		count_fsb,
+	xfs_off_t		end,
+	bool			update_isize,
+	uint			resblks,
+	struct xfs_bmbt_irec	*imap)
+{
+	xfs_fsize_t		i_size;
+	int			nimaps;
+	int			error;
+
+	error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK,
+			XFS_IEXT_WRITE_UNWRITTEN_CNT);
+	if (error)
+		return error;
+
+	nimaps = 1;
+	error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb,
+				XFS_BMAPI_CONVERT, resblks, imap, &nimaps);
+	if (error)
+		return error;
+
+	/*
+	 * Update the inode size to reflect the extent that was converted in
+	 * this iteration. We must not advance isize beyond the extent we just
+	 * converted, otherwise a crash before the next conversion exposes
+	 * unwritten extents (zeroes) to userspace instead of the written data.
+	 * Clamp to the byte-level write end in case the converted extent
+	 * extends past the write boundary.
+	 */
+	i_size = XFS_FSB_TO_B(ip->i_mount,
+			imap->br_startoff + imap->br_blockcount);
+	if (i_size > end)
+		i_size = end;
+	if (update_isize && i_size > i_size_read(VFS_I(ip)))
+		i_size_write(VFS_I(ip), i_size);
+	i_size = xfs_new_eof(ip, i_size);
+	if (i_size) {
+		ip->i_disk_size = i_size;
+		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+	}
+
+	return 0;
+}
+
 int
 xfs_iomap_write_unwritten(
 	xfs_inode_t	*ip,
@@ -617,21 +670,19 @@ xfs_iomap_write_unwritten(
 	bool		update_isize)
 {
 	xfs_mount_t	*mp = ip->i_mount;
+	xfs_off_t	end = offset + count;
 	xfs_fileoff_t	offset_fsb;
 	xfs_filblks_t	count_fsb;
 	xfs_filblks_t	numblks_fsb;
-	int		nimaps;
 	xfs_trans_t	*tp;
 	xfs_bmbt_irec_t imap;
-	struct inode	*inode = VFS_I(ip);
-	xfs_fsize_t	i_size;
 	uint		resblks;
 	int		error;
 
 	trace_xfs_unwritten_convert(ip, offset, count);
 
 	offset_fsb = XFS_B_TO_FSBT(mp, offset);
-	count_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + count);
+	count_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)end);
 	count_fsb = (xfs_filblks_t)(count_fsb - offset_fsb);
 
 	/*
@@ -666,39 +717,12 @@ xfs_iomap_write_unwritten(
 		if (error)
 			return error;
 
-		error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK,
-				XFS_IEXT_WRITE_UNWRITTEN_CNT);
-		if (error)
-			goto error_on_bmapi_transaction;
-
-		/*
-		 * Modify the unwritten extent state of the buffer.
-		 */
-		nimaps = 1;
-		error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb,
-					XFS_BMAPI_CONVERT, resblks, &imap,
-					&nimaps);
-		if (error)
-			goto error_on_bmapi_transaction;
-
-		/*
-		 * Update the inode size to reflect the extent that was
-		 * converted in this iteration. We must not advance isize
-		 * beyond the extent we just converted, otherwise a crash
-		 * before the next conversion exposes unwritten extents
-		 * (zeroes) to userspace instead of the written data.
-		 * Clamp to the byte-level write end in case the converted
-		 * extent extends past the write boundary.
-		 */
-		i_size = XFS_FSB_TO_B(mp, imap.br_startoff + imap.br_blockcount);
-		if (i_size > offset + count)
-			i_size = offset + count;
-		if (update_isize && i_size > i_size_read(inode))
-			i_size_write(inode, i_size);
-		i_size = xfs_new_eof(ip, i_size);
-		if (i_size) {
-			ip->i_disk_size = i_size;
-			xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+		error = xfs_iomap_write_unwritten_one(tp, ip, offset_fsb,
+				count_fsb, end, update_isize, resblks, &imap);
+		if (error) {
+			xfs_trans_cancel(tp);
+			xfs_iunlock(ip, XFS_ILOCK_EXCL);
+			return error;
 		}
 
 		error = xfs_trans_commit(tp);
@@ -724,11 +748,6 @@ xfs_iomap_write_unwritten(
 	} while (count_fsb > 0);
 
 	return 0;
-
-error_on_bmapi_transaction:
-	xfs_trans_cancel(tp);
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	return error;
 }
 
 static inline bool
-- 
2.55.0

[PATCH 25/38] xfs: convert xfs_iomap_write_unwritten to rolling transactions

[Dave Chinner]

Convert xfs_iomap_write_unwritten() from per-iteration transaction
allocation to a single rolling transaction that keeps the ILOCK
held across the entire unwritten extent conversion loop.

Previously each iteration allocated a new transaction, acquired the
ILOCK, converted one extent, committed, and released the ILOCK.
This left a window where the extent tree could change between
iterations.

The rolling transaction keeps the ILOCK held throughout, making the
multi-extent conversion atomic with respect to other concurrent
extent operations. XFS_TRANS_RENEW_BLKRES is set so that the btree
split block reservation is automatically renewed after each
xfs_defer_finish() call.

The startblock corruption check is performed after xfs_defer_finish()
commits the dirty transaction, so that corruption detection warns and
returns an error without causing a filesystem shutdown from cancelling
a dirty transaction.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 16feac3ad3bd..e43ce4811283 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -19,6 +19,7 @@
 #include "xfs_errortag.h"
 #include "xfs_error.h"
 #include "xfs_trans.h"
+#include "xfs_defer.h"
 #include "xfs_trans_space.h"
 #include "xfs_inode_item.h"
 #include "xfs_iomap.h"
@@ -702,44 +703,49 @@ xfs_iomap_write_unwritten(
 	if (error)
 		return error;
 
-	do {
-		/*
-		 * Set up a transaction to convert the range of extents
-		 * from unwritten to real. Do allocations in a loop until
-		 * we have covered the range passed in.
-		 *
-		 * Note that we can't risk to recursing back into the filesystem
-		 * here as we might be asked to write out the same inode that we
-		 * complete here and might deadlock on the iolock.
-		 */
-		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, resblks,
-				0, true, &tp);
-		if (error)
-			return error;
+	/*
+	 * Allocate the transaction once and use a rolling transaction to
+	 * convert the range of extents from unwritten to real. The rolling
+	 * transaction keeps the ILOCK held across the entire conversion,
+	 * making it atomic with respect to other concurrent extent
+	 * operations. The block reservation is automatically renewed on
+	 * each roll by XFS_TRANS_RENEW_BLKRES.
+	 *
+	 * Note that we can't risk recursing back into the filesystem here
+	 * as we might be asked to write out the same inode that we complete
+	 * here and might deadlock on the iolock.
+	 */
+	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, resblks,
+			0, true, &tp);
+	if (error)
+		return error;
+	tp->t_flags |= XFS_TRANS_RENEW_BLKRES;
 
+	do {
 		error = xfs_iomap_write_unwritten_one(tp, ip, offset_fsb,
 				count_fsb, end, update_isize, resblks, &imap);
-		if (error) {
-			xfs_trans_cancel(tp);
-			xfs_iunlock(ip, XFS_ILOCK_EXCL);
-			return error;
-		}
+		if (error)
+			goto out_cancel;
 
-		error = xfs_trans_commit(tp);
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+		/*
+		 * Roll the transaction to commit the conversion and renew
+		 * the block reservation. The startblock validation must be
+		 * done after the roll commits the transaction, so that a
+		 * corruption detection does not cancel a dirty transaction
+		 * and shut down the filesystem.
+		 */
+		error = xfs_defer_finish(&tp);
 		if (error)
-			return error;
+			goto out_cancel;
 
 		if (unlikely(!xfs_valid_startblock(ip, imap.br_startblock))) {
 			xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
-			return xfs_alert_fsblock_zero(ip, &imap);
+			error = xfs_alert_fsblock_zero(ip, &imap);
+			goto out_cancel;
 		}
 
-		if ((numblks_fsb = imap.br_blockcount) == 0) {
-			/*
-			 * The numblks_fsb value should always get
-			 * smaller, otherwise the loop is stuck.
-			 */
+		numblks_fsb = imap.br_blockcount;
+		if (numblks_fsb == 0) {
 			ASSERT(imap.br_blockcount);
 			break;
 		}
@@ -747,7 +753,14 @@ xfs_iomap_write_unwritten(
 		count_fsb -= numblks_fsb;
 	} while (count_fsb > 0);
 
-	return 0;
+	error = xfs_trans_commit(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	return error;
+
+out_cancel:
+	xfs_trans_cancel(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	return error;
 }
 
 static inline bool
-- 
2.55.0

[PATCH 26/38] xfs: plumb struct xfs_trans *tp into xfs_reflink_end_cow_extent

[Dave Chinner]

Add a struct xfs_trans *tp parameter to xfs_reflink_end_cow_extent().
When a non-NULL transaction is provided, use it directly for the
COW extent remapping and return the error to the caller without
managing the transaction or lock lifecycle.

When tp is NULL, the existing behaviour is preserved.

All callers currently pass NULL, so there is no functional change.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index e40f7afc2c7b..b175a549ee55 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -889,15 +889,19 @@ xfs_reflink_end_cow_extent_locked(
  */
 STATIC int
 xfs_reflink_end_cow_extent(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	xfs_fileoff_t		*offset_fsb,
 	xfs_fileoff_t		end_fsb)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*tp;
 	unsigned int		resblks;
+	bool			local_tp = false;
 	int			error;
 
+	if (tp)
+		goto end_cow;
+
 	resblks = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
 	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
 			XFS_TRANS_RESERVE, &tp);
@@ -905,8 +909,12 @@ xfs_reflink_end_cow_extent(
 		return error;
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
+	local_tp = true;
 
+end_cow:
 	error = xfs_reflink_end_cow_extent_locked(tp, ip, offset_fsb, end_fsb);
+	if (!local_tp)
+		return error;
 	if (error)
 		xfs_trans_cancel(tp);
 	else
@@ -966,7 +974,8 @@ xfs_reflink_end_cow(
 	 * blocks will be remapped.
 	 */
 	while (end_fsb > offset_fsb && !error)
-		error = xfs_reflink_end_cow_extent(ip, &offset_fsb, end_fsb);
+		error = xfs_reflink_end_cow_extent(NULL, ip, &offset_fsb,
+				end_fsb);
 
 	if (error)
 		trace_xfs_reflink_end_cow_error(ip, error, _RET_IP_);
-- 
2.55.0

[PATCH 27/38] xfs: convert xfs_reflink_end_cow to rolling transactions

[Dave Chinner]

Convert xfs_reflink_end_cow() from per-extent transaction allocation
to a single rolling transaction that keeps the ILOCK held across the
entire COW remapping loop.

The rolling transaction keeps the ILOCK held throughout, making the
COW remapping operation atomic with respect to other ILOCK-protected
extent manipulations such as truncate, reflink remapping, and other
concurrent end_cow operations on overlapping regions.

XFS_TRANS_RENEW_BLKRES is set so that the btree split block
reservation is automatically renewed by xfs_defer_finish() after
each iteration's deferred operations are processed.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 75 ++++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index b175a549ee55..7e62f05499be 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -925,6 +925,13 @@ xfs_reflink_end_cow_extent(
 
 /*
  * Remap parts of a file's data fork after a successful CoW.
+ *
+ * The rolling transaction keeps the ILOCK held across the entire remapping
+ * loop, making the operation atomic with respect to other ILOCK-protected
+ * extent manipulations such as truncate, reflink remapping, and other
+ * concurrent end_cow operations on overlapping regions.
+ * XFS_TRANS_RENEW_BLKRES ensures the btree split block reservation is
+ * renewed after each xfs_defer_finish() call.
  */
 int
 xfs_reflink_end_cow(
@@ -932,54 +939,48 @@ xfs_reflink_end_cow(
 	xfs_off_t			offset,
 	xfs_off_t			count)
 {
+	struct xfs_mount		*mp = ip->i_mount;
 	xfs_fileoff_t			offset_fsb;
 	xfs_fileoff_t			end_fsb;
-	int				error = 0;
+	struct xfs_trans		*tp;
+	unsigned int			resblks;
+	int				error;
 
 	trace_xfs_reflink_end_cow(ip, offset, count);
 
-	offset_fsb = XFS_B_TO_FSBT(ip->i_mount, offset);
-	end_fsb = XFS_B_TO_FSB(ip->i_mount, offset + count);
+	offset_fsb = XFS_B_TO_FSBT(mp, offset);
+	end_fsb = XFS_B_TO_FSB(mp, offset + count);
 
-	/*
-	 * Walk forwards until we've remapped the I/O range.  The loop function
-	 * repeatedly cycles the ILOCK to allocate one transaction per remapped
-	 * extent.
-	 *
-	 * If we're being called by writeback then the folios will still
-	 * have the writeback flag set, which prevents races with reflink
-	 * remapping and truncate.  Reflink remapping prevents races with
-	 * writeback by taking the iolock and mmaplock before flushing
-	 * the folios and remapping, which means there won't be any further
-	 * writeback or page cache dirtying until the reflink completes.
-	 *
-	 * We should never have two threads issuing writeback for the same file
-	 * region.  There are also have post-eof checks in the writeback
-	 * preparation code so that we don't bother writing out folios that are
-	 * about to be truncated.
-	 *
-	 * If we're being called as part of directio write completion, the dio
-	 * count is still elevated, which reflink and truncate will wait for.
-	 * Reflink remapping takes the iolock and mmaplock and waits for
-	 * pending dio to finish, which should prevent any directio until the
-	 * remap completes.  Multiple concurrent directio writes to the same
-	 * region are handled by end_cow processing only occurring for the
-	 * threads which succeed; the outcome of multiple overlapping direct
-	 * writes is not well defined anyway.
-	 *
-	 * It's possible that a buffered write and a direct write could collide
-	 * here (the buffered write stumbles in after the dio flushes and
-	 * invalidates the page cache and immediately queues writeback), but we
-	 * have never supported this 100%.  If either disk write succeeds the
-	 * blocks will be remapped.
-	 */
-	while (end_fsb > offset_fsb && !error)
-		error = xfs_reflink_end_cow_extent(NULL, ip, &offset_fsb,
+	resblks = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
+	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
+			XFS_TRANS_RESERVE | XFS_TRANS_RENEW_BLKRES, &tp);
+	if (error)
+		return error;
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	xfs_trans_ijoin(tp, ip, 0);
+
+	while (end_fsb > offset_fsb) {
+		error = xfs_reflink_end_cow_extent(tp, ip, &offset_fsb,
 				end_fsb);
+		if (error)
+			goto out_cancel;
+
+		error = xfs_defer_finish(&tp);
+		if (error)
+			goto out_cancel;
+	}
 
+	error = xfs_trans_commit(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	if (error)
 		trace_xfs_reflink_end_cow_error(ip, error, _RET_IP_);
 	return error;
+
+out_cancel:
+	xfs_trans_cancel(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	trace_xfs_reflink_end_cow_error(ip, error, _RET_IP_);
+	return error;
 }
 
 /*
-- 
2.55.0

[PATCH 28/38] xfs: remove xfs_reflink_end_cow_extent wrapper and rename locked variant

[Dave Chinner]

Now that xfs_reflink_end_cow() calls xfs_reflink_end_cow_extent_locked()
directly with its rolling transaction, the xfs_reflink_end_cow_extent()
wrapper function is no longer needed. Remove it entirely and rename
xfs_reflink_end_cow_extent_locked() to xfs_reflink_end_cow_extent()
since there is no longer an unlocked variant.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_reflink.c | 40 ++--------------------------------------
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 7e62f05499be..597300f39611 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -775,7 +775,7 @@ xfs_reflink_update_quota(
  * requirements as low as possible.
  */
 STATIC int
-xfs_reflink_end_cow_extent_locked(
+xfs_reflink_end_cow_extent(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	xfs_fileoff_t		*offset_fsb,
@@ -887,42 +887,6 @@ xfs_reflink_end_cow_extent_locked(
  * every remap operation and we'd like to keep the block reservation
  * requirements as low as possible.
  */
-STATIC int
-xfs_reflink_end_cow_extent(
-	struct xfs_trans	*tp,
-	struct xfs_inode	*ip,
-	xfs_fileoff_t		*offset_fsb,
-	xfs_fileoff_t		end_fsb)
-{
-	struct xfs_mount	*mp = ip->i_mount;
-	unsigned int		resblks;
-	bool			local_tp = false;
-	int			error;
-
-	if (tp)
-		goto end_cow;
-
-	resblks = XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK);
-	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
-			XFS_TRANS_RESERVE, &tp);
-	if (error)
-		return error;
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	xfs_trans_ijoin(tp, ip, 0);
-	local_tp = true;
-
-end_cow:
-	error = xfs_reflink_end_cow_extent_locked(tp, ip, offset_fsb, end_fsb);
-	if (!local_tp)
-		return error;
-	if (error)
-		xfs_trans_cancel(tp);
-	else
-		error = xfs_trans_commit(tp);
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	return error;
-}
-
 /*
  * Remap parts of a file's data fork after a successful CoW.
  *
@@ -1023,7 +987,7 @@ xfs_reflink_end_atomic_cow(
 	xfs_trans_ijoin(tp, ip, 0);
 
 	while (end_fsb > offset_fsb && !error) {
-		error = xfs_reflink_end_cow_extent_locked(tp, ip, &offset_fsb,
+		error = xfs_reflink_end_cow_extent(tp, ip, &offset_fsb,
 				end_fsb);
 	}
 	if (error) {
-- 
2.55.0

[PATCH 29/38] xfs: convert xfs_zoned_end_io to rolling transactions

[Dave Chinner]

Convert xfs_zoned_end_io() from per-extent transaction allocation
to a single rolling transaction that keeps the ILOCK held across
the entire extent mapping loop.

The rolling transaction keeps the ILOCK held throughout, making the
zoned IO completion operation atomic with respect to other concurrent
extent manipulations. XFS_TRANS_RENEW_BLKRES is set so that the
block reservation is automatically renewed by xfs_defer_finish()
after each iteration's deferred operations are processed.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_zone_alloc.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index 61b483d24c88..25134298a5d3 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -13,6 +13,7 @@
 #include "xfs_inode.h"
 #include "xfs_iomap.h"
 #include "xfs_trans.h"
+#include "xfs_defer.h"
 #include "xfs_alloc.h"
 #include "xfs_bmap.h"
 #include "xfs_bmap_btree.h"
@@ -314,6 +315,13 @@ xfs_zoned_map_extent(
 	return 0;
 }
 
+/*
+ * Map written extents into the data fork after zoned IO completion.
+ *
+ * The rolling transaction keeps the ILOCK held across the entire mapping
+ * loop, making the operation atomic with respect to other concurrent
+ * extent manipulations.
+ */
 int
 xfs_zoned_end_io(
 	struct xfs_inode	*ip,
@@ -341,24 +349,23 @@ xfs_zoned_end_io(
 	if (xfs_is_shutdown(mp))
 		return -EIO;
 
+	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
+			XFS_TRANS_RESERVE | XFS_TRANS_RENEW_BLKRES, &tp);
+	if (error)
+		return error;
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	xfs_trans_ijoin(tp, ip, 0);
+
 	while (new.br_startoff < end_fsb) {
 		new.br_blockcount = end_fsb - new.br_startoff;
 
-		error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0,
-				XFS_TRANS_RESERVE, &tp);
-		if (error)
-			return error;
-		xfs_ilock(ip, XFS_ILOCK_EXCL);
-		xfs_trans_ijoin(tp, ip, 0);
-
 		error = xfs_zoned_map_extent(tp, ip, &new, oz, old_startblock);
 		if (error)
-			xfs_trans_cancel(tp);
-		else
-			error = xfs_trans_commit(tp);
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+			goto out_cancel;
+
+		error = xfs_defer_finish(&tp);
 		if (error)
-			return error;
+			goto out_cancel;
 
 		new.br_startoff += new.br_blockcount;
 		new.br_startblock += new.br_blockcount;
@@ -366,7 +373,14 @@ xfs_zoned_end_io(
 			old_startblock += new.br_blockcount;
 	}
 
-	return 0;
+	error = xfs_trans_commit(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	return error;
+
+out_cancel:
+	xfs_trans_cancel(tp);
+	xfs_iunlock(ip, XFS_ILOCK_EXCL);
+	return error;
 }
 
 /*
-- 
2.55.0

[PATCH 30/38] xfs: plumb struct xfs_trans *tp into xfs_iomap_write_direct

[Dave Chinner]

Add a struct xfs_trans *tp parameter to xfs_iomap_write_direct().
When a non-NULL transaction is provided, use it directly for the
allocation instead of allocating one internally. The block
reservation is added to the caller's transaction via
xfs_trans_reserve_more_inode().

On success with a caller-supplied transaction, the function returns
without committing or unlocking. On error, it returns the error
without cancelling.

When tp is NULL, the existing behaviour is preserved.

All callers currently pass NULL, so there is no functional change.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c | 41 ++++++++++++++++++++++++++++-------------
 fs/xfs/xfs_iomap.h |  7 ++++---
 fs/xfs/xfs_pnfs.c  |  2 +-
 3 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index e43ce4811283..f3e4f243e877 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -267,6 +267,7 @@ xfs_iomap_eof_align_last_fsb(
 
 int
 xfs_iomap_write_direct(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	xfs_fileoff_t		offset_fsb,
 	xfs_fileoff_t		count_fsb,
@@ -275,7 +276,7 @@ xfs_iomap_write_direct(
 	u64			*seq)
 {
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*tp;
+	struct xfs_trans	*local_tp = NULL;
 	xfs_filblks_t		resaligned;
 	int			nimaps;
 	unsigned int		dblocks, rblocks;
@@ -296,7 +297,10 @@ xfs_iomap_write_direct(
 		rblocks = 0;
 	}
 
-	error = xfs_qm_dqattach(ip);
+	if (tp)
+		error = xfs_qm_dqattach_locked(ip, false);
+	else
+		error = xfs_qm_dqattach(ip);
 	if (error)
 		return error;
 
@@ -322,10 +326,18 @@ xfs_iomap_write_direct(
 		}
 	}
 
-	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
-			rblocks, force, &tp);
-	if (error)
-		return error;
+	if (tp) {
+		error = xfs_trans_reserve_more_inode(tp, ip, dblocks, rblocks,
+				force);
+		if (error)
+			return error;
+	} else {
+		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
+				rblocks, force, &tp);
+		if (error)
+			return error;
+		local_tp = tp;
+	}
 
 	error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK, nr_exts);
 	if (error)
@@ -341,9 +353,9 @@ xfs_iomap_write_direct(
 	if (error)
 		goto out_trans_cancel;
 
-	/*
-	 * Complete the transaction
-	 */
+	if (!local_tp)
+		return 0;
+
 	error = xfs_trans_commit(tp);
 	if (error)
 		goto out_unlock;
@@ -359,8 +371,11 @@ xfs_iomap_write_direct(
 	return error;
 
 out_trans_cancel:
-	xfs_trans_cancel(tp);
-	goto out_unlock;
+	if (local_tp) {
+		xfs_trans_cancel(tp);
+		goto out_unlock;
+	}
+	return error;
 }
 
 STATIC bool
@@ -1154,8 +1169,8 @@ xfs_direct_write_iomap_begin(
 		end_fsb = min(end_fsb, imap.br_startoff + imap.br_blockcount);
 	xfs_iunlock(ip, lockmode);
 
-	error = xfs_iomap_write_direct(ip, offset_fsb, end_fsb - offset_fsb,
-			flags, &imap, &seq);
+	error = xfs_iomap_write_direct(NULL, ip, offset_fsb,
+			end_fsb - offset_fsb, flags, &imap, &seq);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index ebcce7d49446..937a61c7a610 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -12,9 +12,10 @@ struct xfs_inode;
 struct xfs_bmbt_irec;
 struct xfs_zone_alloc_ctx;
 
-int xfs_iomap_write_direct(struct xfs_inode *ip, xfs_fileoff_t offset_fsb,
-		xfs_fileoff_t count_fsb, unsigned int flags,
-		struct xfs_bmbt_irec *imap, u64 *sequence);
+int xfs_iomap_write_direct(struct xfs_trans *tp, struct xfs_inode *ip,
+		xfs_fileoff_t offset_fsb, xfs_fileoff_t count_fsb,
+		unsigned int flags, struct xfs_bmbt_irec *imap,
+		u64 *sequence);
 int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
 xfs_fileoff_t xfs_iomap_eof_align_last_fsb(struct xfs_inode *ip,
 		xfs_fileoff_t end_fsb);
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index f8535ecde21a..442564967b39 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -202,7 +202,7 @@ xfs_fs_map_blocks(
 					       imap.br_blockcount);
 		xfs_iunlock(ip, lock_flags);
 
-		error = xfs_iomap_write_direct(ip, offset_fsb,
+		error = xfs_iomap_write_direct(NULL, ip, offset_fsb,
 				end_fsb - offset_fsb, 0, &imap, &seq);
 		if (error)
 			goto out_unlock;
-- 
2.55.0

[PATCH 31/38] xfs: make xfs_iomap_write_direct fill in the iomap directly

[Dave Chinner]

Move the xfs_bmbt_to_iomap() call and the xfs_iomap_alloc tracing
into xfs_iomap_write_direct() so that callers do not need to
convert the returned imap to an iomap themselves.

Both callers performed the same conversion after the function
returned. By moving this inside the function, the callers are
simplified and the iomap sequence cookie is correctly captured
after the allocation completes under the ILOCK.

Add iomap, iomap_flags, offset and length parameters so the
function has everything it needs for tracing and the iomap
conversion. Replace the seq output parameter with an internal
variable.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c | 46 ++++++++++++++++++++++------------------------
 fs/xfs/xfs_iomap.h |  3 ++-
 fs/xfs/xfs_pnfs.c  |  9 ++++-----
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index f3e4f243e877..347a451ec7cb 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -273,7 +273,10 @@ xfs_iomap_write_direct(
 	xfs_fileoff_t		count_fsb,
 	unsigned int		flags,
 	struct xfs_bmbt_irec	*imap,
-	u64			*seq)
+	loff_t			offset,
+	loff_t			length,
+	struct iomap		*iomap,
+	u16			iomap_flags)
 {
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_trans	*local_tp = NULL;
@@ -284,6 +287,7 @@ xfs_iomap_write_direct(
 	int			error;
 	int			bmapi_flags = XFS_BMAPI_PREALLOC;
 	int			nr_exts = XFS_IEXT_ADD_NOSPLIT_CNT;
+	u64			seq;
 
 	ASSERT(count_fsb > 0);
 
@@ -353,27 +357,26 @@ xfs_iomap_write_direct(
 	if (error)
 		goto out_trans_cancel;
 
-	if (!local_tp)
-		return 0;
-
-	error = xfs_trans_commit(tp);
-	if (error)
-		goto out_unlock;
-
-	if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) {
-		xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
-		error = xfs_alert_fsblock_zero(ip, imap);
+	seq = xfs_iomap_inode_sequence(ip, iomap_flags);
+	if (local_tp) {
+		error = xfs_trans_commit(tp);
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
+		if (error)
+			return error;
+		if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) {
+			xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
+			return xfs_alert_fsblock_zero(ip, imap);
+		}
 	}
 
-out_unlock:
-	*seq = xfs_iomap_inode_sequence(ip, 0);
-	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	return error;
+	trace_xfs_iomap_alloc(ip, offset, length, XFS_DATA_FORK, imap);
+	return xfs_bmbt_to_iomap(ip, iomap, imap, flags,
+			iomap_flags | IOMAP_F_NEW, seq);
 
 out_trans_cancel:
 	if (local_tp) {
 		xfs_trans_cancel(tp);
-		goto out_unlock;
+		xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	}
 	return error;
 }
@@ -1169,14 +1172,9 @@ xfs_direct_write_iomap_begin(
 		end_fsb = min(end_fsb, imap.br_startoff + imap.br_blockcount);
 	xfs_iunlock(ip, lockmode);
 
-	error = xfs_iomap_write_direct(NULL, ip, offset_fsb,
-			end_fsb - offset_fsb, flags, &imap, &seq);
-	if (error)
-		return error;
-
-	trace_xfs_iomap_alloc(ip, offset, length, XFS_DATA_FORK, &imap);
-	return xfs_bmbt_to_iomap(ip, iomap, &imap, flags,
-				 iomap_flags | IOMAP_F_NEW, seq);
+	return xfs_iomap_write_direct(NULL, ip, offset_fsb,
+			end_fsb - offset_fsb, flags, &imap,
+			offset, length, iomap, iomap_flags);
 
 out_unlock:
 	if (lockmode)
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index 937a61c7a610..c883a7175cd0 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -15,7 +15,8 @@ struct xfs_zone_alloc_ctx;
 int xfs_iomap_write_direct(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_fileoff_t offset_fsb, xfs_fileoff_t count_fsb,
 		unsigned int flags, struct xfs_bmbt_irec *imap,
-		u64 *sequence);
+		loff_t offset, loff_t length, struct iomap *iomap,
+		u16 iomap_flags);
 int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
 xfs_fileoff_t xfs_iomap_eof_align_last_fsb(struct xfs_inode *ip,
 		xfs_fileoff_t end_fsb);
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 442564967b39..f9c0e748448e 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -190,8 +190,6 @@ xfs_fs_map_blocks(
 		xfs_iunlock(ip, lock_flags);
 		goto out_unlock;
 	}
-	seq = xfs_iomap_inode_sequence(ip, 0);
-
 	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
 
 	if (write && (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
@@ -203,7 +201,8 @@ xfs_fs_map_blocks(
 		xfs_iunlock(ip, lock_flags);
 
 		error = xfs_iomap_write_direct(NULL, ip, offset_fsb,
-				end_fsb - offset_fsb, 0, &imap, &seq);
+				end_fsb - offset_fsb, 0, &imap,
+				offset, length, iomap, 0);
 		if (error)
 			goto out_unlock;
 
@@ -219,11 +218,11 @@ xfs_fs_map_blocks(
 			goto out_unlock;
 
 	} else {
+		seq = xfs_iomap_inode_sequence(ip, 0);
 		xfs_iunlock(ip, lock_flags);
+		error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0, 0, seq);
 	}
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-
-	error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0, 0, seq);
 	*device_generation = mp->m_generation;
 	return error;
 out_unlock:
-- 
2.55.0

[PATCH 32/38] xfs: plumb struct xfs_trans **tpp into xfs_direct_write_cow_iomap_begin

[Dave Chinner]

Add a struct xfs_trans **tpp parameter to
xfs_direct_write_cow_iomap_begin(). When tpp is provided and
xfs_reflink_allocate_cow() returns -EAGAIN (needing a transaction),
return -EAGAIN to the caller so they can allocate a transaction
and retry at a higher level. When tpp is NULL, handle the retry
internally as before using a local transaction.

The caller currently passes NULL, so the function continues to
manage its own transaction internally. No functional change.

This is preparation for moving the transaction allocation and
retry logic up to xfs_direct_write_iomap_begin() so that a single
unified retry loop handles both COW and non-COW allocation paths.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 347a451ec7cb..160eea8b331c 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -902,6 +902,11 @@ xfs_bmap_hw_atomic_write_possible(
  * Handle COW extent allocation and iomap setup for direct writes to reflinked
  * files.
  *
+ * 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
+ *
  * 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:
@@ -913,6 +918,7 @@ xfs_bmap_hw_atomic_write_possible(
  */
 static int
 xfs_direct_write_cow_iomap_begin(
+	struct xfs_trans	**tpp,
 	struct xfs_inode	*ip,
 	loff_t			offset,
 	loff_t			length,
@@ -927,34 +933,37 @@ xfs_direct_write_cow_iomap_begin(
 	struct xfs_mount	*mp = ip->i_mount;
 	struct xfs_bmbt_irec	cmap;
 	struct xfs_trans	*tp = NULL;
+	struct xfs_trans	*local_tp = NULL;
 	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
 	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, length);
 	bool			shared = false;
 	int			error;
 	u64			seq;
 
-	*lockmode = XFS_ILOCK_EXCL;
+	if (!tpp) {
+		*lockmode = XFS_ILOCK_EXCL;
 
-	error = xfs_ilock_for_iomap(ip, flags, lockmode);
-	if (error)
-		return error;
+		error = xfs_ilock_for_iomap(ip, flags, lockmode);
+		if (error)
+			return error;
 
 retry:
-	*nimaps = 1;
-	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, imap,
-			       nimaps, 0);
-	if (error)
-		goto out_unlock;
+		*nimaps = 1;
+		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, imap,
+				       nimaps, 0);
+		if (error)
+			goto out_unlock;
+	} else if (*tpp) {
+		tp = *tpp;
+	}
 
 	if (!imap_needs_cow(ip, flags, imap, *nimaps)) {
 		/*
 		 * Extent is not shared - return the imap and ILOCK to the
 		 * caller for normal IO path processing.
 		 */
-		if (tp) {
-			xfs_trans_cancel(tp);
-			tp = NULL;
-		}
+		if (local_tp)
+			xfs_trans_cancel(local_tp);
 		return 0;
 	}
 
@@ -962,13 +971,21 @@ xfs_direct_write_cow_iomap_begin(
 	if (flags & IOMAP_NOWAIT)
 		goto out_unlock;
 
-	/* may drop and re-acquire the ilock */
 	error = xfs_reflink_allocate_cow(&tp, ip, imap, &cmap, &shared,
 			lockmode,
 			(flags & IOMAP_DIRECT) || IS_DAX(VFS_I(ip)));
 	if (error == -EAGAIN) {
+		ASSERT(!tp);
+
 		/*
-		 * COW allocation needs a transaction. Drop the ILOCK and
+		 * If the caller can handle the retry, return -EAGAIN so
+		 * they can allocate a transaction and call again.
+		 */
+		if (tpp)
+			goto out_unlock;
+
+		/*
+		 * Otherwise 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
@@ -980,22 +997,21 @@ xfs_direct_write_cow_iomap_begin(
 		 * Retry the imap lookup since the extent tree may have changed
 		 * while the ILOCK was not held.
 		 */
-		ASSERT(!tp);
-
 		xfs_iunlock(ip, *lockmode);
 
 		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
 				0, 0, false, &tp);
 		if (error)
 			return error;
+		local_tp = tp;
 
 		goto retry;
 	}
 	if (error)
 		goto out_unlock;
 
-	if (tp) {
-		error = xfs_trans_commit(tp);
+	if (local_tp) {
+		error = xfs_trans_commit(local_tp);
 		tp = NULL;
 		if (error)
 			goto out_unlock;
@@ -1026,13 +1042,14 @@ xfs_direct_write_cow_iomap_begin(
 			goto out_unlock;
 	}
 	seq = xfs_iomap_inode_sequence(ip, IOMAP_F_SHARED);
-	xfs_iunlock(ip, *lockmode);
+	if (!tpp)
+		xfs_iunlock(ip, *lockmode);
 	return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
 
 out_unlock:
-	if (tp)
-		xfs_trans_cancel(tp);
-	if (*lockmode)
+	if (local_tp && tp)
+		xfs_trans_cancel(local_tp);
+	if (!tpp)
 		xfs_iunlock(ip, *lockmode);
 	return error;
 }
@@ -1076,9 +1093,9 @@ xfs_direct_write_iomap_begin(
 		iomap_flags |= IOMAP_F_ATOMIC_BIO;
 
 	if (xfs_is_cow_inode(ip)) {
-		error = xfs_direct_write_cow_iomap_begin(ip, offset, length,
-				flags, iomap, srcmap, &imap, &nimaps,
-				&lockmode, iomap_flags);
+		error = xfs_direct_write_cow_iomap_begin(NULL, ip, offset,
+				length, flags, iomap, srcmap, &imap,
+				&nimaps, &lockmode, iomap_flags);
 		if (error)
 			return error;
 		if (!nimaps)
-- 
2.55.0

[PATCH 33/38] xfs: introduce struct xfs_direct_write_args for direct write call chain

[Dave Chinner]

Introduce struct xfs_direct_write_args to pass context through the
direct write extent allocation call chain instead of long lists of
individual parameters. The struct carries the transaction pointer,
inode, block allocation range (offset_fsb/count_fsb), IO byte range
(offset/length for tracing), iomap flags, data and COW fork extent
mappings, shared state, convert_now flag, and iomap/srcmap output
pointers.

Convert all functions in the direct write allocation chain:
xfs_iomap_write_direct(), xfs_direct_write_cow_iomap_begin(),
xfs_reflink_allocate_cow(), xfs_reflink_fill_cow_hole(), and
xfs_reflink_fill_delalloc().

xfs_direct_write_cow_iomap_begin() takes an additional bool
needs_tp parameter that controls whether the function manages
locking and transaction allocation internally (true for current
callers) or expects the caller to have already set up the
transaction and ILOCK (false, for future restructured callers).

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_iomap.c   | 218 +++++++++++++++++++++----------------------
 fs/xfs/xfs_iomap.h   |  25 ++++-
 fs/xfs/xfs_pnfs.c    |  16 +++-
 fs/xfs/xfs_reflink.c | 100 +++++++++-----------
 fs/xfs/xfs_reflink.h |   6 +-
 5 files changed, 190 insertions(+), 175 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 160eea8b331c..6126b559fd70 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -267,17 +267,9 @@ xfs_iomap_eof_align_last_fsb(
 
 int
 xfs_iomap_write_direct(
-	struct xfs_trans	*tp,
-	struct xfs_inode	*ip,
-	xfs_fileoff_t		offset_fsb,
-	xfs_fileoff_t		count_fsb,
-	unsigned int		flags,
-	struct xfs_bmbt_irec	*imap,
-	loff_t			offset,
-	loff_t			length,
-	struct iomap		*iomap,
-	u16			iomap_flags)
+	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_filblks_t		resaligned;
@@ -289,9 +281,9 @@ xfs_iomap_write_direct(
 	int			nr_exts = XFS_IEXT_ADD_NOSPLIT_CNT;
 	u64			seq;
 
-	ASSERT(count_fsb > 0);
+	ASSERT(args->count_fsb > 0);
 
-	resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
+	resaligned = xfs_aligned_fsb_count(args->offset_fsb, args->count_fsb,
 					   xfs_get_extsz_hint(ip));
 	if (unlikely(XFS_IS_REALTIME_INODE(ip))) {
 		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
@@ -301,7 +293,7 @@ xfs_iomap_write_direct(
 		rblocks = 0;
 	}
 
-	if (tp)
+	if (args->tp)
 		error = xfs_qm_dqattach_locked(ip, false);
 	else
 		error = xfs_qm_dqattach(ip);
@@ -321,61 +313,61 @@ xfs_iomap_write_direct(
 	 * the reserve block pool for bmbt block allocation if there is no space
 	 * left but we need to do unwritten extent conversion.
 	 */
-	if (flags & IOMAP_DAX) {
+	if (args->flags & IOMAP_DAX) {
 		bmapi_flags = XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO;
-		if (imap->br_state == XFS_EXT_UNWRITTEN) {
+		if (args->imap.br_state == XFS_EXT_UNWRITTEN) {
 			force = true;
 			nr_exts = XFS_IEXT_WRITE_UNWRITTEN_CNT;
 			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0) << 1;
 		}
 	}
 
-	if (tp) {
-		error = xfs_trans_reserve_more_inode(tp, ip, dblocks, rblocks,
-				force);
+	if (args->tp) {
+		error = xfs_trans_reserve_more_inode(args->tp, ip, dblocks,
+				rblocks, force);
 		if (error)
 			return error;
 	} else {
 		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
-				rblocks, force, &tp);
+				rblocks, force, &args->tp);
 		if (error)
 			return error;
-		local_tp = tp;
+		local_tp = args->tp;
 	}
 
-	error = xfs_iext_count_extend(tp, ip, XFS_DATA_FORK, nr_exts);
+	error = xfs_iext_count_extend(args->tp, ip, XFS_DATA_FORK, nr_exts);
 	if (error)
 		goto out_trans_cancel;
 
-	/*
-	 * From this point onwards we overwrite the imap pointer that the
-	 * caller gave to us.
-	 */
 	nimaps = 1;
-	error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb, bmapi_flags, 0,
-				imap, &nimaps);
+	error = xfs_bmapi_write(args->tp, ip, args->offset_fsb, args->count_fsb,
+				bmapi_flags, 0, &args->imap, &nimaps);
 	if (error)
 		goto out_trans_cancel;
 
-	seq = xfs_iomap_inode_sequence(ip, iomap_flags);
+	seq = xfs_iomap_inode_sequence(ip, args->iomap_flags);
 	if (local_tp) {
-		error = xfs_trans_commit(tp);
+		error = xfs_trans_commit(args->tp);
+		args->tp = NULL;
 		xfs_iunlock(ip, XFS_ILOCK_EXCL);
 		if (error)
 			return error;
-		if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) {
+		if (unlikely(!xfs_valid_startblock(ip,
+				args->imap.br_startblock))) {
 			xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
-			return xfs_alert_fsblock_zero(ip, imap);
+			return xfs_alert_fsblock_zero(ip, &args->imap);
 		}
 	}
 
-	trace_xfs_iomap_alloc(ip, offset, length, XFS_DATA_FORK, imap);
-	return xfs_bmbt_to_iomap(ip, iomap, imap, flags,
-			iomap_flags | IOMAP_F_NEW, seq);
+	trace_xfs_iomap_alloc(ip, args->offset, args->length, XFS_DATA_FORK,
+			&args->imap);
+	return xfs_bmbt_to_iomap(ip, args->iomap, &args->imap, args->flags,
+			args->iomap_flags | IOMAP_F_NEW, seq);
 
 out_trans_cancel:
 	if (local_tp) {
-		xfs_trans_cancel(tp);
+		xfs_trans_cancel(args->tp);
+		args->tp = NULL;
 		xfs_iunlock(ip, XFS_ILOCK_EXCL);
 	}
 	return error;
@@ -918,92 +910,72 @@ xfs_bmap_hw_atomic_write_possible(
  */
 static int
 xfs_direct_write_cow_iomap_begin(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip,
-	loff_t			offset,
-	loff_t			length,
-	unsigned		flags,
-	struct iomap		*iomap,
-	struct iomap		*srcmap,
-	struct xfs_bmbt_irec	*imap,
-	int			*nimaps,
-	unsigned int		*lockmode,
-	u16			iomap_flags)
+	struct xfs_direct_write_args *args,
+	bool			needs_tp)
 {
+	struct xfs_inode	*ip = args->ip;
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_bmbt_irec	cmap;
-	struct xfs_trans	*tp = NULL;
 	struct xfs_trans	*local_tp = NULL;
-	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
-	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, length);
-	bool			shared = false;
+	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 (!tpp) {
-		*lockmode = XFS_ILOCK_EXCL;
+	if (needs_tp) {
+		args->lockmode = XFS_ILOCK_EXCL;
 
-		error = xfs_ilock_for_iomap(ip, flags, lockmode);
+		error = xfs_ilock_for_iomap(ip, args->flags, &args->lockmode);
 		if (error)
 			return error;
 
 retry:
-		*nimaps = 1;
-		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, imap,
-				       nimaps, 0);
+		args->nimaps = 1;
+		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
+				&args->imap, &args->nimaps, 0);
 		if (error)
 			goto out_unlock;
-	} else if (*tpp) {
-		tp = *tpp;
 	}
 
-	if (!imap_needs_cow(ip, flags, imap, *nimaps)) {
-		/*
-		 * Extent is not shared - return the imap and ILOCK to the
-		 * caller for normal IO path processing.
-		 */
-		if (local_tp)
+	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 (flags & IOMAP_NOWAIT)
+	if (args->flags & IOMAP_NOWAIT)
 		goto out_unlock;
 
-	error = xfs_reflink_allocate_cow(&tp, ip, imap, &cmap, &shared,
-			lockmode,
-			(flags & IOMAP_DIRECT) || IS_DAX(VFS_I(ip)));
+	error = xfs_reflink_allocate_cow(args);
 	if (error == -EAGAIN) {
-		ASSERT(!tp);
+		ASSERT(!args->tp);
 
-		/*
-		 * If the caller can handle the retry, return -EAGAIN so
-		 * they can allocate a transaction and call again.
-		 */
-		if (tpp)
+		if (!needs_tp)
 			goto out_unlock;
 
 		/*
-		 * Otherwise 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.
+		 * 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, *lockmode);
+		xfs_iunlock(ip, args->lockmode);
 
 		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
-				0, 0, false, &tp);
+				0, 0, false, &args->tp);
 		if (error)
 			return error;
-		local_tp = tp;
+		local_tp = args->tp;
 
 		goto retry;
 	}
@@ -1012,16 +984,16 @@ xfs_direct_write_cow_iomap_begin(
 
 	if (local_tp) {
 		error = xfs_trans_commit(local_tp);
-		tp = NULL;
+		args->tp = NULL;
 		if (error)
 			goto out_unlock;
 	}
 
-	if (!shared)
+	if (!args->shared)
 		return 0;
 
-	if ((flags & IOMAP_ATOMIC) &&
-	    !xfs_bmap_hw_atomic_write_possible(ip, &cmap,
+	if ((args->flags & IOMAP_ATOMIC) &&
+	    !xfs_bmap_hw_atomic_write_possible(ip, &args->cmap,
 			offset_fsb, end_fsb)) {
 		error = -ENOPROTOOPT;
 		goto out_unlock;
@@ -1031,26 +1003,30 @@ xfs_direct_write_cow_iomap_begin(
 	 * COW extent found and allocated. Set up iomap/srcmap and return
 	 * with *nimaps = 0 to tell the caller the COW path is complete.
 	 */
-	*nimaps = 0;
-	length = XFS_FSB_TO_B(mp, cmap.br_startoff + cmap.br_blockcount);
-	trace_xfs_iomap_found(ip, offset, length - offset, XFS_COW_FORK,
-			&cmap);
-	if (imap->br_startblock != HOLESTARTBLOCK) {
+	args->nimaps = 0;
+	args->length = XFS_FSB_TO_B(mp,
+			args->cmap.br_startoff + args->cmap.br_blockcount);
+	trace_xfs_iomap_found(ip, args->offset,
+			args->length - args->offset, XFS_COW_FORK,
+			&args->cmap);
+	if (args->imap.br_startblock != HOLESTARTBLOCK) {
 		seq = xfs_iomap_inode_sequence(ip, 0);
-		error = xfs_bmbt_to_iomap(ip, srcmap, imap, flags, 0, seq);
+		error = xfs_bmbt_to_iomap(ip, args->srcmap, &args->imap,
+				args->flags, 0, seq);
 		if (error)
 			goto out_unlock;
 	}
 	seq = xfs_iomap_inode_sequence(ip, IOMAP_F_SHARED);
-	if (!tpp)
-		xfs_iunlock(ip, *lockmode);
-	return xfs_bmbt_to_iomap(ip, iomap, &cmap, flags, IOMAP_F_SHARED, seq);
+	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 && tp)
+	if (local_tp && args->tp)
 		xfs_trans_cancel(local_tp);
-	if (!tpp)
-		xfs_iunlock(ip, *lockmode);
+	if (needs_tp)
+		xfs_iunlock(ip, args->lockmode);
 	return error;
 }
 
@@ -1093,14 +1069,26 @@ xfs_direct_write_iomap_begin(
 		iomap_flags |= IOMAP_F_ATOMIC_BIO;
 
 	if (xfs_is_cow_inode(ip)) {
-		error = xfs_direct_write_cow_iomap_begin(NULL, ip, offset,
-				length, flags, iomap, srcmap, &imap,
-				&nimaps, &lockmode, iomap_flags);
+		struct xfs_direct_write_args cow_args = {
+			.ip		= ip,
+			.offset		= offset,
+			.length		= length,
+			.flags		= flags,
+			.iomap_flags	= iomap_flags,
+			.convert_now	= (flags & IOMAP_DIRECT) ||
+					  IS_DAX(inode),
+			.iomap		= iomap,
+			.srcmap		= srcmap,
+		};
+
+		error = xfs_direct_write_cow_iomap_begin(&cow_args, true);
 		if (error)
 			return error;
-		if (!nimaps)
+		if (!cow_args.nimaps)
 			return 0;
 
+		imap = cow_args.imap;
+		lockmode = cow_args.lockmode;
 		end_fsb = imap.br_startoff + imap.br_blockcount;
 		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
 	} else {
@@ -1189,9 +1177,21 @@ xfs_direct_write_iomap_begin(
 		end_fsb = min(end_fsb, imap.br_startoff + imap.br_blockcount);
 	xfs_iunlock(ip, lockmode);
 
-	return xfs_iomap_write_direct(NULL, ip, offset_fsb,
-			end_fsb - offset_fsb, flags, &imap,
-			offset, length, iomap, iomap_flags);
+	{
+		struct xfs_direct_write_args args = {
+			.ip		= ip,
+			.offset_fsb	= offset_fsb,
+			.count_fsb	= end_fsb - offset_fsb,
+			.offset		= offset,
+			.length		= length,
+			.flags		= flags,
+			.iomap_flags	= iomap_flags,
+			.imap		= imap,
+			.iomap		= iomap,
+		};
+
+		return xfs_iomap_write_direct(&args);
+	}
 
 out_unlock:
 	if (lockmode)
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index c883a7175cd0..7dc0b5993bed 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -12,11 +12,26 @@ struct xfs_inode;
 struct xfs_bmbt_irec;
 struct xfs_zone_alloc_ctx;
 
-int xfs_iomap_write_direct(struct xfs_trans *tp, struct xfs_inode *ip,
-		xfs_fileoff_t offset_fsb, xfs_fileoff_t count_fsb,
-		unsigned int flags, struct xfs_bmbt_irec *imap,
-		loff_t offset, loff_t length, struct iomap *iomap,
-		u16 iomap_flags);
+struct xfs_direct_write_args {
+	struct xfs_trans	*tp;
+	struct xfs_inode	*ip;
+	xfs_fileoff_t		offset_fsb;	/* block range to allocate */
+	xfs_fileoff_t		count_fsb;
+	loff_t			offset;		/* IO byte range for tracing */
+	loff_t			length;
+	unsigned int		flags;
+	u16			iomap_flags;
+	unsigned int		lockmode;
+	struct xfs_bmbt_irec	imap;
+	struct xfs_bmbt_irec	cmap;
+	int			nimaps;
+	bool			shared;
+	bool			convert_now;
+	struct iomap		*iomap;
+	struct iomap		*srcmap;
+};
+
+int xfs_iomap_write_direct(struct xfs_direct_write_args *args);
 int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
 xfs_fileoff_t xfs_iomap_eof_align_last_fsb(struct xfs_inode *ip,
 		xfs_fileoff_t end_fsb);
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index f9c0e748448e..495e081838d0 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -200,9 +200,19 @@ xfs_fs_map_blocks(
 					       imap.br_blockcount);
 		xfs_iunlock(ip, lock_flags);
 
-		error = xfs_iomap_write_direct(NULL, ip, offset_fsb,
-				end_fsb - offset_fsb, 0, &imap,
-				offset, length, iomap, 0);
+		{
+			struct xfs_direct_write_args args = {
+				.ip		= ip,
+				.offset_fsb	= offset_fsb,
+				.count_fsb	= end_fsb - offset_fsb,
+				.offset		= offset,
+				.length		= length,
+				.imap		= imap,
+				.iomap		= iomap,
+			};
+
+			error = xfs_iomap_write_direct(&args);
+		}
 		if (error)
 			goto out_unlock;
 
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 597300f39611..f0dcc66c91ec 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -430,13 +430,9 @@ xfs_reflink_convert_unwritten(
 
 static int
 xfs_reflink_fill_cow_hole(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip,
-	struct xfs_bmbt_irec	*imap,
-	struct xfs_bmbt_irec	*cmap,
-	bool			*shared,
-	bool			convert_now)
+	struct xfs_direct_write_args *args)
 {
+	struct xfs_inode	*ip = args->ip;
 	struct xfs_mount	*mp = ip->i_mount;
 	xfs_filblks_t		resaligned;
 	unsigned int		dblocks = 0, rblocks = 0;
@@ -444,10 +440,11 @@ xfs_reflink_fill_cow_hole(
 	int			error;
 	bool			found;
 
-	ASSERT(*tpp);
+	ASSERT(args->tp);
 
-	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
-	if (error || !*shared)
+	error = xfs_find_trim_cow_extent(ip, &args->imap, &args->cmap,
+			&args->shared, &found);
+	if (error || !args->shared)
 		return error;
 
 	if (found)
@@ -460,10 +457,10 @@ xfs_reflink_fill_cow_hole(
 	 * state was known under the ILOCK. The transaction has not been
 	 * dirtied yet, so on ENOSPC it can safely be cancelled by the caller.
 	 */
-	ASSERT(!((*tpp)->t_flags & XFS_TRANS_DIRTY));
+	ASSERT(!(args->tp->t_flags & XFS_TRANS_DIRTY));
 
-	resaligned = xfs_aligned_fsb_count(imap->br_startoff,
-			imap->br_blockcount, xfs_get_cowextsz_hint(ip));
+	resaligned = xfs_aligned_fsb_count(args->imap.br_startoff,
+			args->imap.br_blockcount, xfs_get_cowextsz_hint(ip));
 	if (XFS_IS_REALTIME_INODE(ip)) {
 		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
 		rblocks = resaligned;
@@ -471,36 +468,33 @@ xfs_reflink_fill_cow_hole(
 		dblocks = XFS_DIOSTRAT_SPACE_RES(mp, resaligned);
 	}
 
-	error = xfs_trans_reserve_more_inode(*tpp, ip, dblocks, rblocks,
+	error = xfs_trans_reserve_more_inode(args->tp, ip, dblocks, rblocks,
 			false);
 	if (error)
 		return error;
 
 	/* Allocate the entire reservation as unwritten blocks. */
 	nimaps = 1;
-	error = xfs_bmapi_write(*tpp, ip, imap->br_startoff,
-			imap->br_blockcount,
-			XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0, cmap,
-			&nimaps);
+	error = xfs_bmapi_write(args->tp, ip, args->imap.br_startoff,
+			args->imap.br_blockcount,
+			XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0,
+			&args->cmap, &nimaps);
 	if (error)
 		return error;
 
 	xfs_inode_set_cowblocks_tag(ip);
 
 convert:
-	return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
+	return xfs_reflink_convert_unwritten(ip, &args->imap, &args->cmap,
+			args->convert_now);
 }
 
 static int
 xfs_reflink_fill_delalloc(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip,
-	struct xfs_bmbt_irec	*imap,
-	struct xfs_bmbt_irec	*cmap,
-	bool			*shared,
-	bool			convert_now)
+	struct xfs_direct_write_args *args)
 {
-	struct xfs_trans	*tp = *tpp;
+	struct xfs_inode	*ip = args->ip;
+	struct xfs_trans	*tp = args->tp;
 	int			nimaps;
 	int			error;
 	bool			found;
@@ -508,25 +502,25 @@ xfs_reflink_fill_delalloc(
 	ASSERT(tp);
 
 	do {
-		error = xfs_find_trim_cow_extent(ip, imap, cmap, shared,
-				&found);
-		if (error || !*shared)
+		error = xfs_find_trim_cow_extent(ip, &args->imap, &args->cmap,
+				&args->shared, &found);
+		if (error || !args->shared)
 			goto out_error;
 
 		if (found)
 			break;
 
-		ASSERT(isnullstartblock(cmap->br_startblock) ||
-		       cmap->br_startblock == DELAYSTARTBLOCK);
+		ASSERT(isnullstartblock(args->cmap.br_startblock) ||
+		       args->cmap.br_startblock == DELAYSTARTBLOCK);
 
 		/*
 		 * Replace delalloc reservation with an unwritten extent.
 		 */
 		nimaps = 1;
-		error = xfs_bmapi_write(tp, ip, cmap->br_startoff,
-				cmap->br_blockcount,
+		error = xfs_bmapi_write(tp, ip, args->cmap.br_startoff,
+				args->cmap.br_blockcount,
 				XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC, 0,
-				cmap, &nimaps);
+				&args->cmap, &nimaps);
 		if (error)
 			goto out_error;
 
@@ -535,25 +529,22 @@ xfs_reflink_fill_delalloc(
 		error = xfs_defer_finish(&tp);
 		if (error)
 			goto out_error;
-	} while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
+	} while (args->cmap.br_startoff + args->cmap.br_blockcount <=
+			args->imap.br_startoff);
 
-	error = xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
+	error = xfs_reflink_convert_unwritten(ip, &args->imap, &args->cmap,
+			args->convert_now);
 out_error:
-	*tpp = tp;
+	args->tp = tp;
 	return error;
 }
 
 /* Allocate all CoW reservations covering a range of blocks in a file. */
 int
 xfs_reflink_allocate_cow(
-	struct xfs_trans	**tpp,
-	struct xfs_inode	*ip,
-	struct xfs_bmbt_irec	*imap,
-	struct xfs_bmbt_irec	*cmap,
-	bool			*shared,
-	uint			*lockmode,
-	bool			convert_now)
+	struct xfs_direct_write_args *args)
 {
+	struct xfs_inode	*ip = args->ip;
 	int			error;
 	bool			found;
 
@@ -563,34 +554,33 @@ xfs_reflink_allocate_cow(
 		xfs_ifork_init_cow(ip);
 	}
 
-	error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
-	if (error || !*shared)
+	error = xfs_find_trim_cow_extent(ip, &args->imap, &args->cmap,
+			&args->shared, &found);
+	if (error || !args->shared)
 		return error;
 
 	/* CoW fork has a real extent */
 	if (found)
-		return xfs_reflink_convert_unwritten(ip, imap, cmap,
-				convert_now);
+		return xfs_reflink_convert_unwritten(ip, &args->imap,
+				&args->cmap, args->convert_now);
 
 	/*
 	 * Allocation is now required, so we need a transaction context from
 	 * the caller if it hasn't already supplied one.
 	 */
-	if (!*tpp)
+	if (!args->tp)
 		return -EAGAIN;
 
-	if (cmap->br_startoff > imap->br_startoff)
-		return xfs_reflink_fill_cow_hole(tpp, ip, imap, cmap, shared,
-				convert_now);
+	if (args->cmap.br_startoff > args->imap.br_startoff)
+		return xfs_reflink_fill_cow_hole(args);
 
 	/*
 	 * CoW fork has a delalloc reservation. Replace it with a real extent.
 	 * There may or may not be a data fork mapping.
 	 */
-	if (isnullstartblock(cmap->br_startblock) ||
-	    cmap->br_startblock == DELAYSTARTBLOCK)
-		return xfs_reflink_fill_delalloc(tpp, ip, imap, cmap, shared,
-				convert_now);
+	if (isnullstartblock(args->cmap.br_startblock) ||
+	    args->cmap.br_startblock == DELAYSTARTBLOCK)
+		return xfs_reflink_fill_delalloc(args);
 
 	/* Shouldn't get here. */
 	ASSERT(0);
diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 3da8374829c3..57c8a9a018bd 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -6,6 +6,8 @@
 #ifndef __XFS_REFLINK_H
 #define __XFS_REFLINK_H 1
 
+struct xfs_direct_write_args;
+
 /*
  * Check whether it is safe to free COW fork blocks from an inode. It is unsafe
  * to do so when an inode has dirty cache or I/O in-flight, even if no shared
@@ -30,9 +32,7 @@ int xfs_reflink_trim_around_shared(struct xfs_inode *ip,
 int xfs_bmap_trim_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
 		bool *shared);
 
-int xfs_reflink_allocate_cow(struct xfs_trans **tpp, struct xfs_inode *ip,
-		struct xfs_bmbt_irec *imap, struct xfs_bmbt_irec *cmap,
-		bool *shared, uint *lockmode, bool convert_now);
+int xfs_reflink_allocate_cow(struct xfs_direct_write_args *args);
 extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset,
 		xfs_off_t count);
 int xfs_reflink_convert_cow_locked(struct xfs_inode *ip,
-- 
2.55.0

[PATCH 34/38] xfs: convert xfs_direct_write_iomap_begin to use dwa struct throughout

[Dave Chinner]

Convert xfs_direct_write_iomap_begin() to use a single
xfs_direct_write_args struct (dwa) allocated on the stack instead
of individual local variables for the fields that are passed down
the call chain.

This replaces the temporary args struct construction at each call
site with a single struct initialised at function entry. The
imap, nimaps, lockmode, iomap_flags, and other fields that were
previously local variables are now accessed through dwa.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 6126b559fd70..4bf33addb3a4 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1041,14 +1041,21 @@ xfs_direct_write_iomap_begin(
 {
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_bmbt_irec	imap;
-	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSBT(mp, offset);
+	struct xfs_direct_write_args dwa = {
+		.ip		= ip,
+		.offset_fsb	= XFS_B_TO_FSBT(mp, offset),
+		.offset		= offset,
+		.length		= length,
+		.flags		= flags,
+		.nimaps		= 1,
+		.convert_now	= (flags & IOMAP_DIRECT) || IS_DAX(inode),
+		.iomap		= iomap,
+		.srcmap		= srcmap,
+	};
 	xfs_fileoff_t		end_fsb = xfs_iomap_end_fsb(mp, offset, length);
 	xfs_fileoff_t		orig_end_fsb = end_fsb;
-	int			nimaps = 1, error = 0;
-	u16			iomap_flags = 0;
 	bool			needs_alloc;
-	unsigned int		lockmode;
+	int			error = 0;
 	u64			seq;
 
 	ASSERT(flags & (IOMAP_WRITE | IOMAP_ZERO));
@@ -1062,49 +1069,36 @@ xfs_direct_write_iomap_begin(
 	 * there is no other metadata changes pending or have been made here.
 	 */
 	if (offset + length > i_size_read(inode))
-		iomap_flags |= IOMAP_F_DIRTY;
+		dwa.iomap_flags |= IOMAP_F_DIRTY;
 
 	/* HW-offload atomics are always used in this path */
 	if (flags & IOMAP_ATOMIC)
-		iomap_flags |= IOMAP_F_ATOMIC_BIO;
+		dwa.iomap_flags |= IOMAP_F_ATOMIC_BIO;
 
 	if (xfs_is_cow_inode(ip)) {
-		struct xfs_direct_write_args cow_args = {
-			.ip		= ip,
-			.offset		= offset,
-			.length		= length,
-			.flags		= flags,
-			.iomap_flags	= iomap_flags,
-			.convert_now	= (flags & IOMAP_DIRECT) ||
-					  IS_DAX(inode),
-			.iomap		= iomap,
-			.srcmap		= srcmap,
-		};
-
-		error = xfs_direct_write_cow_iomap_begin(&cow_args, true);
+		error = xfs_direct_write_cow_iomap_begin(&dwa, true);
 		if (error)
 			return error;
-		if (!cow_args.nimaps)
+		if (!dwa.nimaps)
 			return 0;
 
-		imap = cow_args.imap;
-		lockmode = cow_args.lockmode;
-		end_fsb = imap.br_startoff + imap.br_blockcount;
-		length = XFS_FSB_TO_B(mp, end_fsb) - offset;
+		end_fsb = dwa.imap.br_startoff + dwa.imap.br_blockcount;
+		dwa.length = XFS_FSB_TO_B(mp, end_fsb) - offset;
 	} else {
-		lockmode = XFS_ILOCK_SHARED;
+		dwa.lockmode = XFS_ILOCK_SHARED;
 
-		error = xfs_ilock_for_iomap(ip, flags, &lockmode);
+		error = xfs_ilock_for_iomap(ip, flags, &dwa.lockmode);
 		if (error)
 			return error;
 
-		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
-				&imap, &nimaps, 0);
+		error = xfs_bmapi_read(ip, dwa.offset_fsb,
+				end_fsb - dwa.offset_fsb,
+				&dwa.imap, &dwa.nimaps, 0);
 		if (error)
 			goto out_unlock;
 	}
 
-	needs_alloc = imap_needs_alloc(inode, flags, &imap, nimaps);
+	needs_alloc = imap_needs_alloc(inode, flags, &dwa.imap, dwa.nimaps);
 
 	if (flags & IOMAP_ATOMIC) {
 		error = -ENOPROTOOPT;
@@ -1113,11 +1107,11 @@ xfs_direct_write_iomap_begin(
 		 * then we may end up with multiple extents, which means that
 		 * REQ_ATOMIC-based cannot be used, so avoid this possibility.
 		 */
-		if (needs_alloc && orig_end_fsb - offset_fsb > 1)
+		if (needs_alloc && orig_end_fsb - dwa.offset_fsb > 1)
 			goto out_unlock;
 
-		if (!xfs_bmap_hw_atomic_write_possible(ip, &imap, offset_fsb,
-				orig_end_fsb))
+		if (!xfs_bmap_hw_atomic_write_possible(ip, &dwa.imap,
+				dwa.offset_fsb, orig_end_fsb))
 			goto out_unlock;
 	}
 
@@ -1132,7 +1126,7 @@ xfs_direct_write_iomap_begin(
 	 */
 	if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY)) {
 		error = -EAGAIN;
-		if (!imap_spans_range(&imap, offset_fsb, end_fsb))
+		if (!imap_spans_range(&dwa.imap, dwa.offset_fsb, end_fsb))
 			goto out_unlock;
 	}
 
@@ -1144,15 +1138,17 @@ xfs_direct_write_iomap_begin(
 	 */
 	if (flags & IOMAP_OVERWRITE_ONLY) {
 		error = -EAGAIN;
-		if (imap.br_state != XFS_EXT_NORM &&
-	            ((offset | length) & mp->m_blockmask))
+		if (dwa.imap.br_state != XFS_EXT_NORM &&
+		    ((offset | dwa.length) & mp->m_blockmask))
 			goto out_unlock;
 	}
 
-	seq = xfs_iomap_inode_sequence(ip, iomap_flags);
-	xfs_iunlock(ip, lockmode);
-	trace_xfs_iomap_found(ip, offset, length, XFS_DATA_FORK, &imap);
-	return xfs_bmbt_to_iomap(ip, iomap, &imap, flags, iomap_flags, seq);
+	seq = xfs_iomap_inode_sequence(ip, dwa.iomap_flags);
+	xfs_iunlock(ip, dwa.lockmode);
+	trace_xfs_iomap_found(ip, offset, dwa.length, XFS_DATA_FORK,
+			&dwa.imap);
+	return xfs_bmbt_to_iomap(ip, iomap, &dwa.imap, flags,
+			dwa.iomap_flags, seq);
 
 allocate_blocks:
 	error = -EAGAIN;
@@ -1168,34 +1164,23 @@ xfs_direct_write_iomap_begin(
 	 * Note that the values needs to be less than 32-bits wide until the
 	 * lower level functions are updated.
 	 */
-	length = min_t(loff_t, length, 1024 * PAGE_SIZE);
-	end_fsb = xfs_iomap_end_fsb(mp, offset, length);
+	dwa.length = min_t(loff_t, dwa.length, 1024 * PAGE_SIZE);
+	end_fsb = xfs_iomap_end_fsb(mp, offset, dwa.length);
 
-	if (offset + length > XFS_ISIZE(ip))
+	if (offset + dwa.length > XFS_ISIZE(ip))
 		end_fsb = xfs_iomap_eof_align_last_fsb(ip, end_fsb);
-	else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
-		end_fsb = min(end_fsb, imap.br_startoff + imap.br_blockcount);
-	xfs_iunlock(ip, lockmode);
+	else if (dwa.nimaps && dwa.imap.br_startblock == HOLESTARTBLOCK)
+		end_fsb = min(end_fsb, dwa.imap.br_startoff +
+				dwa.imap.br_blockcount);
 
-	{
-		struct xfs_direct_write_args args = {
-			.ip		= ip,
-			.offset_fsb	= offset_fsb,
-			.count_fsb	= end_fsb - offset_fsb,
-			.offset		= offset,
-			.length		= length,
-			.flags		= flags,
-			.iomap_flags	= iomap_flags,
-			.imap		= imap,
-			.iomap		= iomap,
-		};
+	dwa.count_fsb = end_fsb - dwa.offset_fsb;
+	xfs_iunlock(ip, dwa.lockmode);
 
-		return xfs_iomap_write_direct(&args);
-	}
+	return xfs_iomap_write_direct(&dwa);
 
 out_unlock:
-	if (lockmode)
-		xfs_iunlock(ip, lockmode);
+	if (dwa.lockmode)
+		xfs_iunlock(ip, dwa.lockmode);
 	return error;
 }
 
-- 
2.55.0

[PATCH 35/38] xfs: restructure xfs_direct_write_iomap_begin with unified retry loop

[Dave Chinner]

Replace the split COW/non-COW structure in xfs_direct_write_iomap_begin()
with a unified do {} while (dwa.tp) loop that handles both COW extent
allocation and data fork allocation atomically under a single ILOCK
hold.

The loop structure:
  - xfs_ilock_for_iomap() is called once before the loop
  - xfs_bmapi_read() at the top of each iteration reads fresh state
  - COW check calls xfs_direct_write_cow_iomap_begin() with
    needs_tp=false, which returns -EAGAIN if a transaction is needed
  - If allocation is needed (needs_alloc) but no transaction, falls
    through to alloc_trans
  - If a transaction is available, calls xfs_iomap_write_direct()
    with the transaction, commits, and returns
  - Overwrite path cancels unused tp and returns the mapping
  - alloc_trans at the tail: ASSERT(!tp), checks NOWAIT/OVERWRITE,
    drops the ILOCK, allocates a zero-block transaction via
    xfs_trans_alloc_inode() (which reacquires ILOCK_EXCL), and
    loops back to re-read the extent tree

This makes the extent lookup and allocation atomic with respect to
the ILOCK for both COW and non-COW paths.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 4bf33addb3a4..77f71fdd125a 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1075,110 +1075,162 @@ xfs_direct_write_iomap_begin(
 	if (flags & IOMAP_ATOMIC)
 		dwa.iomap_flags |= IOMAP_F_ATOMIC_BIO;
 
-	if (xfs_is_cow_inode(ip)) {
-		error = xfs_direct_write_cow_iomap_begin(&dwa, true);
-		if (error)
-			return error;
-		if (!dwa.nimaps)
-			return 0;
-
-		end_fsb = dwa.imap.br_startoff + dwa.imap.br_blockcount;
-		dwa.length = XFS_FSB_TO_B(mp, end_fsb) - offset;
-	} else {
+	if (xfs_is_cow_inode(ip))
+		dwa.lockmode = XFS_ILOCK_EXCL;
+	else
 		dwa.lockmode = XFS_ILOCK_SHARED;
 
-		error = xfs_ilock_for_iomap(ip, flags, &dwa.lockmode);
-		if (error)
-			return error;
+	error = xfs_ilock_for_iomap(ip, flags, &dwa.lockmode);
+	if (error)
+		return error;
 
+	do {
+		dwa.nimaps = 1;
 		error = xfs_bmapi_read(ip, dwa.offset_fsb,
 				end_fsb - dwa.offset_fsb,
 				&dwa.imap, &dwa.nimaps, 0);
 		if (error)
 			goto out_unlock;
-	}
 
-	needs_alloc = imap_needs_alloc(inode, flags, &dwa.imap, dwa.nimaps);
-
-	if (flags & IOMAP_ATOMIC) {
-		error = -ENOPROTOOPT;
-		/*
-		 * If we allocate less than what is required for the write
-		 * then we may end up with multiple extents, which means that
-		 * REQ_ATOMIC-based cannot be used, so avoid this possibility.
-		 */
-		if (needs_alloc && orig_end_fsb - dwa.offset_fsb > 1)
-			goto out_unlock;
+		if (xfs_is_cow_inode(ip)) {
+			error = xfs_direct_write_cow_iomap_begin(&dwa, false);
+			if (error == -EAGAIN)
+				goto alloc_trans;
+			if (error)
+				goto out_unlock;
+			if (!dwa.nimaps) {
+				/*
+				 * COW iomaps filled in. Commit the
+				 * transaction if one was needed and
+				 * return.
+				 */
+				if (dwa.tp)
+					error = xfs_trans_commit(dwa.tp);
+				xfs_iunlock(ip, dwa.lockmode);
+				return error;
+			}
 
-		if (!xfs_bmap_hw_atomic_write_possible(ip, &dwa.imap,
-				dwa.offset_fsb, orig_end_fsb))
-			goto out_unlock;
-	}
+			end_fsb = dwa.imap.br_startoff +
+					dwa.imap.br_blockcount;
+			dwa.length = XFS_FSB_TO_B(mp, end_fsb) - offset;
+		}
 
-	if (needs_alloc)
-		goto allocate_blocks;
+		needs_alloc = imap_needs_alloc(inode, flags, &dwa.imap,
+				dwa.nimaps);
+
+		if (flags & IOMAP_ATOMIC) {
+			error = -ENOPROTOOPT;
+			/*
+			 * If we allocate less than what is required for the
+			 * write then we may end up with multiple extents,
+			 * which means that REQ_ATOMIC-based cannot be used,
+			 * so avoid this possibility.
+			 */
+			if (needs_alloc &&
+			    orig_end_fsb - dwa.offset_fsb > 1)
+				goto out_unlock;
+			if (!xfs_bmap_hw_atomic_write_possible(ip, &dwa.imap,
+					dwa.offset_fsb, orig_end_fsb))
+				goto out_unlock;
+		}
 
-	/*
-	 * NOWAIT and OVERWRITE I/O needs to span the entire requested I/O with
-	 * a single map so that we avoid partial IO failures due to the rest of
-	 * the I/O range not covered by this map triggering an EAGAIN condition
-	 * when it is subsequently mapped and aborting the I/O.
-	 */
-	if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY)) {
-		error = -EAGAIN;
-		if (!imap_spans_range(&dwa.imap, dwa.offset_fsb, end_fsb))
-			goto out_unlock;
-	}
+		if (needs_alloc) {
+			if (!dwa.tp)
+				goto alloc_trans;
+
+			/*
+			 * Cap the maximum length we map to a sane size to
+			 * keep the chunks of work done where somewhat
+			 * symmetric with the work writeback does.
+			 *
+			 * Note that the values needs to be less than
+			 * 32-bits wide until the lower level functions
+			 * are updated.
+			 */
+			dwa.length = min_t(loff_t, dwa.length,
+					1024 * PAGE_SIZE);
+			end_fsb = xfs_iomap_end_fsb(mp, offset, dwa.length);
+
+			if (offset + dwa.length > XFS_ISIZE(ip))
+				end_fsb = xfs_iomap_eof_align_last_fsb(ip,
+						end_fsb);
+			else if (dwa.nimaps &&
+				 dwa.imap.br_startblock == HOLESTARTBLOCK)
+				end_fsb = min(end_fsb, dwa.imap.br_startoff +
+						dwa.imap.br_blockcount);
+
+			dwa.count_fsb = end_fsb - dwa.offset_fsb;
+
+			error = xfs_iomap_write_direct(&dwa);
+			if (error)
+				goto out_unlock;
 
-	/*
-	 * For overwrite only I/O, we cannot convert unwritten extents without
-	 * requiring sub-block zeroing.  This can only be done under an
-	 * exclusive IOLOCK, hence return -EAGAIN if this is not a written
-	 * extent to tell the caller to try again.
-	 */
-	if (flags & IOMAP_OVERWRITE_ONLY) {
-		error = -EAGAIN;
-		if (dwa.imap.br_state != XFS_EXT_NORM &&
-		    ((offset | dwa.length) & mp->m_blockmask))
-			goto out_unlock;
-	}
+			error = xfs_trans_commit(dwa.tp);
+			xfs_iunlock(ip, dwa.lockmode);
+			return error;
+		}
 
-	seq = xfs_iomap_inode_sequence(ip, dwa.iomap_flags);
-	xfs_iunlock(ip, dwa.lockmode);
-	trace_xfs_iomap_found(ip, offset, dwa.length, XFS_DATA_FORK,
-			&dwa.imap);
-	return xfs_bmbt_to_iomap(ip, iomap, &dwa.imap, flags,
-			dwa.iomap_flags, seq);
+		/*
+		 * Overwrite path - no allocation needed. Cancel unused
+		 * transaction if allocated and return the mapping.
+		 */
+		if (dwa.tp) {
+			xfs_trans_cancel(dwa.tp);
+			dwa.tp = NULL;
+		}
 
-allocate_blocks:
-	error = -EAGAIN;
-	if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY))
-		goto out_unlock;
+		/*
+		 * NOWAIT and OVERWRITE I/O needs to span the entire
+		 * requested I/O with a single map so that we avoid
+		 * partial IO failures due to the rest of the I/O range
+		 * not covered by this map triggering an EAGAIN condition
+		 * when it is subsequently mapped and aborting the I/O.
+		 */
+		if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY)) {
+			error = -EAGAIN;
+			if (!imap_spans_range(&dwa.imap, dwa.offset_fsb,
+					end_fsb))
+				goto out_unlock;
+		}
+		/*
+		 * For overwrite only I/O, we cannot convert unwritten
+		 * extents without requiring sub-block zeroing. This
+		 * can only be done under an exclusive IOLOCK, hence
+		 * return -EAGAIN if this is not a written extent to
+		 * tell the caller to try again.
+		 */
+		if (flags & IOMAP_OVERWRITE_ONLY) {
+			error = -EAGAIN;
+			if (dwa.imap.br_state != XFS_EXT_NORM &&
+			    ((offset | dwa.length) & mp->m_blockmask))
+				goto out_unlock;
+		}
 
-	/*
-	 * We cap the maximum length we map to a sane size  to keep the chunks
-	 * of work done where somewhat symmetric with the work writeback does.
-	 * This is a completely arbitrary number pulled out of thin air as a
-	 * best guess for initial testing.
-	 *
-	 * Note that the values needs to be less than 32-bits wide until the
-	 * lower level functions are updated.
-	 */
-	dwa.length = min_t(loff_t, dwa.length, 1024 * PAGE_SIZE);
-	end_fsb = xfs_iomap_end_fsb(mp, offset, dwa.length);
+		seq = xfs_iomap_inode_sequence(ip, dwa.iomap_flags);
+		xfs_iunlock(ip, dwa.lockmode);
+		trace_xfs_iomap_found(ip, offset, dwa.length, XFS_DATA_FORK,
+				&dwa.imap);
+		return xfs_bmbt_to_iomap(ip, iomap, &dwa.imap, flags,
+				dwa.iomap_flags, seq);
 
-	if (offset + dwa.length > XFS_ISIZE(ip))
-		end_fsb = xfs_iomap_eof_align_last_fsb(ip, end_fsb);
-	else if (dwa.nimaps && dwa.imap.br_startblock == HOLESTARTBLOCK)
-		end_fsb = min(end_fsb, dwa.imap.br_startoff +
-				dwa.imap.br_blockcount);
+alloc_trans:
+		ASSERT(!dwa.tp);
 
-	dwa.count_fsb = end_fsb - dwa.offset_fsb;
-	xfs_iunlock(ip, dwa.lockmode);
+		error = -EAGAIN;
+		if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY))
+			goto out_unlock;
 
-	return xfs_iomap_write_direct(&dwa);
+		xfs_iunlock(ip, dwa.lockmode);
+		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
+				0, 0, false, &dwa.tp);
+		if (error)
+			return error;
+		dwa.lockmode = XFS_ILOCK_EXCL;
+	} while (dwa.tp);
 
 out_unlock:
+	if (dwa.tp)
+		xfs_trans_cancel(dwa.tp);
 	if (dwa.lockmode)
 		xfs_iunlock(ip, dwa.lockmode);
 	return error;
-- 
2.55.0

[PATCH 36/38] xfs: clean up xfs_direct_write_cow_iomap_begin after restructure

[Dave Chinner]

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 | 113 +++++++--------------------------------------
 1 file changed, 16 insertions(+), 97 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 77f71fdd125a..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,20 +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 && args->tp)
-		xfs_trans_cancel(local_tp);
-	if (needs_tp)
-		xfs_iunlock(ip, args->lockmode);
-	return error;
 }
 
 static int
@@ -1092,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

[PATCH 37/38] xfs: make pNFS block allocation atomic with inode update

[Dave Chinner]

Restructure xfs_fs_map_blocks() to perform the extent allocation
and inode update in a single synchronous transaction, making the
operation atomic with respect to the ILOCK.

Previously the function used two separate transactions: one for
the extent allocation (via xfs_iomap_write_direct) and one for the
inode update (xfs_fs_map_update_inode), followed by an explicit
log force to ensure persistence.

Now the function uses a do {} while (!dwa.tp) loop to allocate a
transaction and re-read the mapping atomically. The inode update
(SUID/SGID strip, timestamps, PREALLOC flag) is performed in the
same transaction as the allocation, and the transaction is committed
synchronously via xfs_trans_set_sync().

xfs_fs_map_update_inode() is converted to take a transaction
parameter and no longer allocates or commits its own transaction.

Assisted-by: LLM
Signed-off-by: Dave Chinner <dgc@kernel.org>
---
 fs/xfs/xfs_pnfs.c | 124 +++++++++++++++++++++++-----------------------
 1 file changed, 61 insertions(+), 63 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 495e081838d0..37194e62d3cb 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -88,29 +88,17 @@ xfs_fs_get_uuid(
  * is from the client to indicate that data has been written and the file size
  * can be extended.
  */
-static int
+static void
 xfs_fs_map_update_inode(
+	struct xfs_trans	*tp,
 	struct xfs_inode	*ip)
 {
-	struct xfs_trans	*tp;
-	int			error;
-
-	error = xfs_trans_alloc(ip->i_mount, &M_RES(ip->i_mount)->tr_writeid,
-			0, 0, 0, &tp);
-	if (error)
-		return error;
-
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
-
 	VFS_I(ip)->i_mode &= ~S_ISUID;
 	if (VFS_I(ip)->i_mode & S_IXGRP)
 		VFS_I(ip)->i_mode &= ~S_ISGID;
 	xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
 	ip->i_diflags |= XFS_DIFLAG_PREALLOC;
-
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	return xfs_trans_commit(tp);
 }
 
 /*
@@ -127,6 +115,10 @@ xfs_fs_map_blocks(
 {
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
+	struct xfs_direct_write_args dwa = {
+		.ip		= ip,
+		.iomap		= iomap,
+	};
 	struct xfs_bmbt_irec	imap;
 	xfs_fileoff_t		offset_fsb, end_fsb;
 	loff_t			limit;
@@ -182,61 +174,67 @@ xfs_fs_map_blocks(
 	end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + length);
 	offset_fsb = XFS_B_TO_FSBT(mp, offset);
 
+	/*
+	 * Extent allocation requires a transaction. If we find a hole, drop
+	 * the ILOCK, allocate a transaction and re-read the mapping so we
+	 * don't use a stale imap for determining the allocation.
+	 */
 	lock_flags = xfs_ilock_data_map_shared(ip);
-	/* request mappings for the specified range only */
-	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
-				&imap, &nimaps, 0);
-	if (error) {
-		xfs_iunlock(ip, lock_flags);
-		goto out_unlock;
-	}
-	ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
-
-	if (write && (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
-		if (offset + length > XFS_ISIZE(ip))
-			end_fsb = xfs_iomap_eof_align_last_fsb(ip, end_fsb);
-		else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
-			end_fsb = min(end_fsb, imap.br_startoff +
-					       imap.br_blockcount);
-		xfs_iunlock(ip, lock_flags);
-
-		{
-			struct xfs_direct_write_args args = {
-				.ip		= ip,
-				.offset_fsb	= offset_fsb,
-				.count_fsb	= end_fsb - offset_fsb,
-				.offset		= offset,
-				.length		= length,
-				.imap		= imap,
-				.iomap		= iomap,
-			};
-
-			error = xfs_iomap_write_direct(&args);
-		}
+	do {
+		nimaps = 1;
+		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
+					&imap, &nimaps, 0);
 		if (error)
-			goto out_unlock;
+			goto out_cancel;
+		ASSERT(!nimaps || imap.br_startblock != DELAYSTARTBLOCK);
+
+		if (!write || (nimaps &&
+			       imap.br_startblock != HOLESTARTBLOCK)) {
+			seq = xfs_iomap_inode_sequence(ip, 0);
+			error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0, 0, seq);
+			goto out_cancel;
+		}
 
-		/*
-		 * Ensure the next transaction is committed synchronously so
-		 * that the blocks allocated and handed out to the client are
-		 * guaranteed to be present even after a server crash.
-		 */
-		error = xfs_fs_map_update_inode(ip);
-		if (!error)
-			error = xfs_log_force_inode(ip);
-		if (error)
-			goto out_unlock;
+		if (!dwa.tp) {
+			xfs_iunlock(ip, lock_flags);
+			error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write,
+					0, 0, false, &dwa.tp);
+			if (error)
+				goto out_unlock;
+			lock_flags = XFS_ILOCK_EXCL;
+		}
+	} while (!dwa.tp);
+
+	if (offset + length > XFS_ISIZE(ip))
+		end_fsb = xfs_iomap_eof_align_last_fsb(ip, end_fsb);
+	else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
+		end_fsb = min(end_fsb, imap.br_startoff + imap.br_blockcount);
+
+	dwa.offset_fsb = offset_fsb;
+	dwa.count_fsb = end_fsb - offset_fsb;
+	dwa.offset = offset;
+	dwa.length = length;
+	dwa.imap = imap;
+	error = xfs_iomap_write_direct(&dwa);
+	if (error)
+		goto out_cancel;
 
-	} else {
-		seq = xfs_iomap_inode_sequence(ip, 0);
-		xfs_iunlock(ip, lock_flags);
-		error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0, 0, seq);
-	}
-	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-	*device_generation = mp->m_generation;
-	return error;
+	/*
+	 * Update the inode and commit synchronously so that the blocks
+	 * are guaranteed persistent before being handed to the client.
+	 */
+	xfs_fs_map_update_inode(dwa.tp, ip);
+	xfs_trans_set_sync(dwa.tp);
+	error = xfs_trans_commit(dwa.tp);
+	dwa.tp = NULL;
+
+out_cancel:
+	if (dwa.tp)
+		xfs_trans_cancel(dwa.tp);
+	xfs_iunlock(ip, lock_flags);
 out_unlock:
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+	*device_generation = mp->m_generation;
 	return error;
 }
 
-- 
2.55.0

[PATCH 38/38] xfs: remove dead internal transaction path from xfs_iomap_write_direct

[Dave Chinner]

Now that all callers supply a valid transaction via args->tp,
remove the dead internal transaction allocation path from
xfs_iomap_write_direct().

The function is now a pure allocation helper: it reserves blocks
via xfs_trans_reserve_more_inode(), extends the inode extent count,
calls xfs_bmapi_write(), and fills in the iomap. All error paths
return directly since the caller manages the transaction and lock
lifecycle.

The xfs_qm_dqattach() call is replaced with xfs_qm_dqattach_locked()
since the ILOCK is always held on entry.

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

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 4b2860e80d9f..4c7b11e4a6a1 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -271,7 +271,6 @@ xfs_iomap_write_direct(
 {
 	struct xfs_inode	*ip = args->ip;
 	struct xfs_mount	*mp = ip->i_mount;
-	struct xfs_trans	*local_tp = NULL;
 	xfs_filblks_t		resaligned;
 	int			nimaps;
 	unsigned int		dblocks, rblocks;
@@ -281,6 +280,7 @@ xfs_iomap_write_direct(
 	int			nr_exts = XFS_IEXT_ADD_NOSPLIT_CNT;
 	u64			seq;
 
+	ASSERT(args->tp);
 	ASSERT(args->count_fsb > 0);
 
 	resaligned = xfs_aligned_fsb_count(args->offset_fsb, args->count_fsb,
@@ -293,10 +293,7 @@ xfs_iomap_write_direct(
 		rblocks = 0;
 	}
 
-	if (args->tp)
-		error = xfs_qm_dqattach_locked(ip, false);
-	else
-		error = xfs_qm_dqattach(ip);
+	error = xfs_qm_dqattach_locked(ip, false);
 	if (error)
 		return error;
 
@@ -322,55 +319,26 @@ xfs_iomap_write_direct(
 		}
 	}
 
-	if (args->tp) {
-		error = xfs_trans_reserve_more_inode(args->tp, ip, dblocks,
-				rblocks, force);
-		if (error)
-			return error;
-	} else {
-		error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_write, dblocks,
-				rblocks, force, &args->tp);
-		if (error)
-			return error;
-		local_tp = args->tp;
-	}
+	error = xfs_trans_reserve_more_inode(args->tp, ip, dblocks, rblocks,
+			force);
+	if (error)
+		return error;
 
 	error = xfs_iext_count_extend(args->tp, ip, XFS_DATA_FORK, nr_exts);
 	if (error)
-		goto out_trans_cancel;
+		return error;
 
 	nimaps = 1;
 	error = xfs_bmapi_write(args->tp, ip, args->offset_fsb, args->count_fsb,
 				bmapi_flags, 0, &args->imap, &nimaps);
 	if (error)
-		goto out_trans_cancel;
+		return error;
 
 	seq = xfs_iomap_inode_sequence(ip, args->iomap_flags);
-	if (local_tp) {
-		error = xfs_trans_commit(args->tp);
-		args->tp = NULL;
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-		if (error)
-			return error;
-		if (unlikely(!xfs_valid_startblock(ip,
-				args->imap.br_startblock))) {
-			xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
-			return xfs_alert_fsblock_zero(ip, &args->imap);
-		}
-	}
-
 	trace_xfs_iomap_alloc(ip, args->offset, args->length, XFS_DATA_FORK,
 			&args->imap);
 	return xfs_bmbt_to_iomap(ip, args->iomap, &args->imap, args->flags,
 			args->iomap_flags | IOMAP_F_NEW, seq);
-
-out_trans_cancel:
-	if (local_tp) {
-		xfs_trans_cancel(args->tp);
-		args->tp = NULL;
-		xfs_iunlock(ip, XFS_ILOCK_EXCL);
-	}
-	return error;
 }
 
 STATIC bool
-- 
2.55.0