linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 00/38] XFS: Atomic multi-extent operations via rolling transactions
@ 2026-08-19  0:12 Dave Chinner
  2026-08-19  0:12 ` [PATCH 01/38] xfs: fix dirty transaction cancellation in xfs_bmapi_convert_one_delalloc Dave Chinner
                   ` (37 more replies)
  0 siblings, 38 replies; 39+ messages in thread
From: Dave Chinner @ 2026-08-19  0:12 UTC (permalink / raw)
  To: linux-xfs; +Cc: cem

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(-)

^ permalink raw reply	[flat|nested] 39+ messages in thread
end of thread, other threads:[~2026-08-19  0:15 UTC | newest]

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).