Linux NFS development
 help / color / mirror / Atom feed
* [PATCH v4 0/9] xfs: pNFS block layout fixes
@ 2026-10-20  3:08 Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

The XFS side of the pNFS block and SCSI layout fixes, in one series as
Christoph asked [1]. A pNFS client alone can trigger the bugs fixed by
patches 1 and 3 to 6. The race fixed by patch 7 needs a local process
writing to a shared mapping, and patches 8 and 9 change how a
LAYOUTCOMMIT is made stable.

Patch 1 keeps blockgc from freeing the blocks past EOF that a write
layout handed out. Patches 2 to 4 make LAYOUTCOMMIT refuse a range
without blocks instead of allocating blocks for it, and clamp the
committed length before adding it to the offset. Patch 5 limits the
block count that a write layout asks for at once to one extent, so that
the block reservation cannot wrap, and patch 6 clamps the layout range
to the maximum file size. Patch 7 takes the invalidate lock while mapping a
layout.

Patches 8 and 9 make a LAYOUTCOMMIT stable the way fdatasync does,
instead of forcing the log for every one. With patch 9, a LAYOUTCOMMIT
that only overwrites written blocks replies before its timestamps, and
the change attribute that nfsd derives from the ctime, are stable. A
crash before the next log write can take the change attribute back to
its value before the overwrite. nfsd persists the timestamps for
FILE_SYNC writes and for COMMIT, so this is a question for the nfsd side
as much as for XFS; the fixes do not depend on patches 8 and 9.

Patches 5 to 7 come from the ->map_blocks series [2]. Patch 1 of that
series is in for-next as e3c682dd1130. Its patches 4, 5, 7 and 8, which
let ->map_blocks return more than one mapping, are left for the next
cycle [1].

Patches 1 to 6 are marked for stable. Patch 2 needs a backport for 6.9
and older, where xfs_bmapi_write() has no -ENOSR return, and a context
fix in xfs_bmap.h for 6.12; patch 3 needs one for 7.2 and older, which
do not have fs/xfs/xfs_ioend.c, and patch 5 needs MAXEXTLEN in place of
XFS_MAX_BMBT_EXTLEN for 5.15 and older. Patch 4 should not go without
patches 2 and 3, and patch 6 not without patch 5: on their own, the
clamped range reaches code that shuts XFS down.

XFS_BMAPI_CONVERT_ONLY takes bit 12 of the bmapi flags, which
"xfs: add XFS_BMAPI_UNWRITTEN to unmap unwritten extents in
__xfs_bunmapi()" in the fs-verity series [9] also uses. Whichever goes
in second needs to move to bit 13. The name was used before, by a flag
that commit 26b91c728b2d ("xfs: make COW fork unwritten extent
conversions more robust") removed. That flag only skipped the
allocation and went on to map the hole; this one stops before it.

Dave's rolling transaction series [10] rewrites
xfs_iomap_write_direct(), xfs_iomap_write_unwritten() and
xfs_fs_map_blocks(). In its 37/38 the inode update moves into the
allocation transaction and the path that allocates nothing returns
early; the case added by patch 1 would still need its own transaction
there. Whichever series goes in second needs to carry these changes
over.

The pynfs tests for patches 2 to 4, with a README on running pynfs
against a block layout export, were posted as [11].

Tested on nfsd-testing 78c334bc5ae6 with commit e3c682dd1130 and this
series on top, in QEMU guests with KASAN and lockdep, and XFS_DEBUG
where noted: an nvmet-tcp target, the server (nfsd, XFS on the nvmet
namespace, pnfs export) and one client. The client was pynfs, with the
tests of [11], except for fstests, which ran on the Linux NFSv4.2
client. "Without the series" is nfsd-testing with e3c682dd1130. The test
branch has the same changes as this series, apart from one line each
in xfs_file.c and xfs_ioend.c that differ between the two bases. The
runs for patches 5 and 6 and the pynfs and fstests runs over the block
layout were repeated on v4; the others are from v3, which differs only
in patch 5.

 - LAYOUTCOMMITs of a hole, and of a laid-out range followed by a hole
   (BLOCK7, BLOCK8): without the series XFS shut down ("Corruption of
   in-memory data (0x8) detected at xfs_trans_mod_sb"). With it,
   NFS4ERR_IO, nothing allocated and no warning; BLOCK6 to BLOCK10
   passed in 5 runs, and once with XFS_DEBUG. In both, BLOCK3 fails on a
   bug in the test.
 - The wrapped length (BLOCK10): without the series, NFS4ERR_IO with
   "nfsd: non-standard errno: -63" and a WARNING. With patches 1 to 3
   only, NFS4ERR_IO and the laid-out range stays unwritten, also with
   XFS_DEBUG, without a warning or an assertion. With the series, the
   laid-out range from 4 KiB on is converted and the commit fails at the
   hole.
 - Sashiko's case: with a test-only change that has
   xfs_iext_count_extend() log the inode in every transaction, and v2's
   handling of the refusal (cancel), a LAYOUTCOMMIT of a hole shut XFS
   down ("Corruption of in-memory data (0x8) detected at
   xfs_trans_cancel", from xfs_iomap_write_unwritten() in
   xfs_fs_commit_blocks()). With this series on the same change, BLOCK6,
   BLOCK7 and BLOCK10 passed and XFS kept running.
 - RW LAYOUTGETs (patches 5 and 6): for 16 TiB at offset 0 of an empty
   file, XFS shut down without the series. With it, NFS4ERR_NOSPC and
   XFS kept running, as for NFS4_UINT64_MAX bytes and for 100 GiB; each
   left less than 8 GiB of the 32 GiB free, allocated one extent at a
   time.
   One for 16 GiB with a loga_minlength of 16 GiB is granted in three
   extents. A READ LAYOUTGET at offset 0 for NFS4_UINT64_MAX got
   NFS4ERR_IO with "Access to block zero" without the series, and
   NFS4ERR_INVAL without a warning with it, as patch 6 says.
 - Blocks past EOF under a write layout held for 360 s, with small files
   written on the server (patch 1): kept, no warning.
 - fstests generic/075, 091 and 263 over the block layout: 075 fails
   with an fsx size error with and without the series, and all 10330
   LAYOUTCOMMITs returned 0.
 - The server's own XFS, the other callers of
   xfs_iomap_write_unwritten(): O_DIRECT and buffered writes into
   preallocated space were converted as before, also with XFS_DEBUG.
   fdatasync after an overwrite forced the log 0 times in 5 runs, fsync
   5 times, and fdatasync after an extending write 5 times.
 - fstests -g quick on a local XFS (mkfs.xfs defaults from xfsprogs
   6.6.0, 32 GiB TEST and 8 GiB SCRATCH devices, KASAN and lockdep off),
   with and without the series: 1017 tests ran, and the same 5 failed
   either way (generic/453, 454, 563, 794 and xfs/261).
 - W=1 builds of fs/xfs after each patch on XFS for-next ec8f62cc109a:
   no warnings.

Patch 9 was measured in its v2 posting [7] and is unchanged apart from
the call. No test reaches the race of patch 7 or the delalloc case of
XFS_BMAPI_CONVERT_ONLY.

[1] https://lore.kernel.org/r/20261014130557.GA15773@lst.de
[2] https://lore.kernel.org/r/20261008-xfs-nfsd-map-blocks-v1-0-560026cdccb6@samsung.com
[3] https://lore.kernel.org/r/20261014-xfs-pnfs-eof-prealloc-v2-1-f303fd5e9dfb@samsung.com
[4] https://lore.kernel.org/r/20261014-xfs-pnfs-commit-hole-v2-0-d3a02a79ab6c@samsung.com
[5] https://lore.kernel.org/r/20261014130834.GB15773@lst.de
[6] https://lore.kernel.org/r/20261008154017.GO2705364@frogsfrogsfrogs
[7] https://lore.kernel.org/r/20261014-xfs-pnfs-commit-datasync-v2-0-c11341a189b5@samsung.com
[8] https://lore.kernel.org/r/20261014130124.GA15407@lst.de
[9] https://lore.kernel.org/r/20261002223705.2175542-14-aalbersh@kernel.org
[10] https://lore.kernel.org/r/20260819001442.1451892-1-dgc@kernel.org
[11] https://lore.kernel.org/r/20261014-pynfs-block-commit-v1-0-1723979f495f@samsung.com
[12] https://lore.kernel.org/r/20261015060932.GD14035@lst.de

---
Changes in v4:
- Patch 5: limit the allocation in xfs_fs_map_blocks() to
  XFS_MAX_BMBT_EXTLEN blocks, which is all that xfs_bmapi_write() maps
  in one mapping, instead of refusing a count whose reservation would
  wrap in xfs_iomap_write_direct(), as Christoph [12] and Darrick [6]
  suggested.
  This answers Sashiko's second point on v3 patch 5: no reservation
  holds more than one extent's worth of blocks now. On the first, an
  RW LAYOUTGET for more than the free space gets the extents that fit
  in the reply, or, when fewer than XFS_MAX_BMBT_EXTLEN blocks are
  free before that, still gets NFS4ERR_NOSPC after nfsd has allocated
  them; the message of patch 5 says so. The Linux block layout client
  does not send a LAYOUTGET on OPEN.
- Patch 6: code unchanged; the result of an RW LAYOUTGET to the end of
  the file in its message updated for patch 5.
- Sashiko's point on v3 patch 6, a READ LAYOUTGET at offset 0 for
  NFS4_UINT64_MAX getting NFS4ERR_INVAL because nfsd asks again at the
  maximum file size: not changed here. With the ->map_blocks series [2],
  where nfsd asks once, that LAYOUTGET was granted.
- Reviewed-by from Christoph on patches 1 to 3 and 7 to 9.
- Link to v3: https://lore.kernel.org/r/20261015-xfs-pnfs-batch-v3-0-54fc7b2e467d@samsung.com

Changes in v3 (from the separate postings):
- Patch 1 (v2 [3]): unchanged; Christoph said it looks good.
- Patches 2 and 3 (v2 patch 1 [4]): split the new flag from its user in
  the pNFS code [5]. xfs_bmapi_write() now returns -ENOSR instead of
  -EIO when it maps nothing, and xfs_iomap_write_unwritten() then
  commits the transaction instead of cancelling it:
  xfs_iext_count_extend() may have logged the inode, and cancelling a
  dirty transaction shuts the filesystem down, as Sashiko pointed out on
  v2 [5]. Patch 2 asserts that XFS_BMAPI_CONVERT_ONLY comes with
  XFS_BMAPI_CONVERT.
- Patch 4 (v2 patch 2 [4]): code unchanged; the message now says what
  the wrap does on top of patches 2 and 3.
- Patches 5 and 6 (v1 patches 2 and 3 [2]): code unchanged, one
  sentence of the message of patch 5 fixed.
- Patch 7 (v1 patch 6 [2]): taken without the patches before it in that
  series, and without the sentence about the patch after it.
- Patch 8 (v2 patch 1 [7]): add the datasync and log_flushed arguments
  to xfs_log_force_inode() itself rather than wrapping a new
  __xfs_log_force_inode(), as Christoph suggested [8].
- Patch 9 (v2 patch 2 [7]): calls xfs_log_force_inode() with them.
- Rebased onto XFS for-next ec8f62cc109a.

---
Daejun Park (9):
      xfs: mark the inode preallocated for pNFS write layouts past EOF
      xfs: add a flag to convert unwritten extents without allocating blocks
      xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks
      xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end
      xfs: limit the count of a pNFS write allocation to one extent
      xfs: clamp the pNFS layout range to the maximum file size
      xfs: take the invalidate lock while mapping a pNFS layout
      xfs: add datasync and log_flushed arguments to xfs_log_force_inode()
      xfs: make pNFS LAYOUTCOMMIT stable the way fdatasync does

 fs/xfs/libxfs/xfs_bmap.c | 14 ++++++--
 fs/xfs/libxfs/xfs_bmap.h | 10 +++++-
 fs/xfs/xfs_export.c      |  2 +-
 fs/xfs/xfs_file.c        | 52 +++-------------------------
 fs/xfs/xfs_inode.c       | 36 +++++++++++++++++---
 fs/xfs/xfs_inode.h       |  3 +-
 fs/xfs/xfs_ioend.c       |  2 +-
 fs/xfs/xfs_iomap.c       | 20 +++++++++--
 fs/xfs/xfs_iomap.h       |  3 +-
 fs/xfs/xfs_pnfs.c        | 89 +++++++++++++++++++++++++++++++++++-------------
 fs/xfs/xfs_zone_gc.c     |  2 +-
 11 files changed, 147 insertions(+), 86 deletions(-)
---
base-commit: ec8f62cc109a3f596b3c65dc0f1011c209239051
change-id: 20261015-xfs-pnfs-batch-85459210aeeb

Best regards,
-- 
Daejun Park <daejun7.park@samsung.com>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-22 12:41   ` Pankaj Raghav (Samsung)
  2026-10-20  3:08 ` [PATCH v4 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_map_blocks() marks the inode XFS_DIFLAG_PREALLOC after it
allocates blocks for a write layout, so that xfs_free_eofblocks() does
not free blocks past EOF that a client may write to. But a write layout
for a range past EOF where blocks are already allocated, such as the
speculative preallocation of buffered writes, gets those blocks without
an allocation, and the inode is not marked. blockgc, or the release of
the file, then frees them without breaking the layout. The client still
writes to them, and they may belong to another file by then.

With pynfs as the client, a file grown to 1 MiB by two buffered writes
on the server had 960 KiB preallocated past EOF, and an RW LAYOUTGET for
that range got it as one INVALID_DATA extent, without the inode being
marked. About 300 seconds later, with the default
speculative_prealloc_lifetime, blockgc freed those blocks while the
layout was still held. When the client then wrote through the layout and
sent LAYOUTCOMMIT, xfs_fs_commit_blocks() found a hole where the
unwritten extent had been, and xfs_iomap_write_unwritten() allocated for
it with no blocks reserved:

  XFS (nvme0n1): Corruption of in-memory data (0x8) detected at
    xfs_trans_mod_sb+0x817/0x960 (fs/xfs/xfs_trans.c:351).
    Shutting down filesystem.
  ...
   xfs_trans_mod_sb+0x817/0x960
   xfs_alloc_vextent_finish+0x2e7/0x990
   xfs_alloc_vextent_start_ag+0x25d/0x720
   xfs_bmap_btalloc+0xdd2/0x1720
   xfs_bmapi_allocate+0x197/0xf90
   xfs_bmapi_write+0x4d0/0xb60
   xfs_iomap_write_unwritten+0x307/0x7e0
   xfs_fs_commit_blocks+0x1d9/0x630
   nfsd4_block_proc_layoutcommit+0x5c2/0x7d0
   nfsd4_layoutcommit+0x4b5/0xa20

The client got NFS4ERR_IO, in six runs out of six. When small files were
written on the server after blockgc, with a sync first so that the freed
extents were no longer busy, they got those blocks, and the client's
write through the layout replaced 978944 bytes of their data before the
LAYOUTCOMMIT. A LAYOUTGET and LAYOUTCOMMIT 30 seconds apart, before
blockgc runs, leave the client's data in place.

Mark the inode also when a write mapping ends past EOF and nothing was
allocated, the same way as for an allocation, before the blocks are
handed out. With this, the same test keeps the blocks past blockgc, and
the LAYOUTCOMMIT after 360 seconds succeeds with the data in place.

To share the update with the allocation case, set a local update_inode
flag in both cases, call xfs_fs_map_update_inode() in one place below
them, and move the log force into xfs_fs_map_update_inode().

Fixes: 527851124d10 ("xfs: implement pNFS export operations")
Cc: stable@vger.kernel.org
Reported-by: Chuck Lever <cel@kernel.org>
Link: https://lore.kernel.org/r/4de5356f-bc4d-4f1a-9c81-fce09b1b46fd@slotpi15m67
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index ab38561700..a9f0b472f3 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -110,7 +110,15 @@ xfs_fs_map_update_inode(
 	ip->i_diflags |= XFS_DIFLAG_PREALLOC;
 
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-	return xfs_trans_commit(tp);
+	error = xfs_trans_commit(tp);
+	if (error)
+		return error;
+
+	/*
+	 * Force the log so that the blocks handed out to the client are
+	 * guaranteed to be present even after a server crash.
+	 */
+	return xfs_log_force_inode(ip);
 }
 
 /*
@@ -134,6 +142,7 @@ xfs_fs_map_blocks(
 	uint			lock_flags;
 	int			error = 0;
 	u64			seq;
+	bool			update_inode = false;
 
 	if (xfs_is_shutdown(mp))
 		return -EIO;
@@ -221,20 +230,27 @@ xfs_fs_map_blocks(
 				end_fsb - offset_fsb, 0, &imap, &seq);
 		if (error)
 			goto out_unlock;
+		update_inode = true;
+	} else {
+		xfs_iunlock(ip, lock_flags);
 
 		/*
-		 * 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.
+		 * Blocks past EOF that were allocated for buffered writes, such
+		 * as speculative preallocation, can be freed by
+		 * xfs_free_eofblocks() without breaking the layout unless the
+		 * inode is marked as preallocated. Mark it before handing them
+		 * out for writing, as for blocks allocated above.
 		 */
+		if (write && !(ip->i_diflags & XFS_DIFLAG_PREALLOC) &&
+		    imap.br_startoff + imap.br_blockcount >
+		    XFS_B_TO_FSB(mp, XFS_ISIZE(ip)))
+			update_inode = true;
+	}
+
+	if (update_inode) {
 		error = xfs_fs_map_update_inode(ip);
-		if (!error)
-			error = xfs_log_force_inode(ip);
 		if (error)
 			goto out_unlock;
-
-	} else {
-		xfs_iunlock(ip, lock_flags);
 	}
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
 

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/9] xfs: add a flag to convert unwritten extents without allocating blocks
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-22 12:43   ` Pankaj Raghav (Samsung)
  2026-10-20  3:08 ` [PATCH v4 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks Daejun Park via B4 Relay
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_bmapi_write() with XFS_BMAPI_CONVERT converts unwritten extents to
written ones, and allocates blocks for any hole or delalloc extent it
finds in the range. A caller that may only convert blocks that are
already allocated has to look at the mapping first, and the mapping can
change once the ILOCK is dropped.

Add XFS_BMAPI_CONVERT_ONLY. With it, xfs_bmapi_write() stops at a hole
or a delalloc extent instead of allocating blocks and returns the
mappings that it converted before it. If the range starts with a hole or
a delalloc extent, it returns -ENOSR, as it does when a delalloc
conversion maps nothing in the range. Nothing has been changed then, and
the caller decides what to do with its transaction.

A flag of this name was removed by commit 26b91c728b2d ("xfs: make COW
fork unwritten extent conversions more robust"). That one skipped the
allocation and still mapped the hole; this one stops before it.

"xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks" uses it,
which is why this is marked for stable.

Suggested-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20261013120359.GB2944@lst.de
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c | 14 ++++++++++++--
 fs/xfs/libxfs/xfs_bmap.h | 10 +++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ca49bf4a50..95c0178b44 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4130,6 +4130,8 @@ xfs_bmapi_finish(
  * Returns a negative error code on failure, including -ENOSPC when it could not
  * allocate any blocks and -ENOSR when it did allocate blocks to convert a
  * delalloc range, but those blocks were before the passed in range.
+ * With XFS_BMAPI_CONVERT_ONLY, it stops at a hole or a delalloc extent instead
+ * of allocating blocks, and returns -ENOSR if it stops before mapping anything.
  */
 int
 xfs_bmapi_write(
@@ -4189,6 +4191,8 @@ xfs_bmapi_write(
 	 */
 	ASSERT((flags & (XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO)) !=
 			(XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO));
+	ASSERT(!(flags & XFS_BMAPI_CONVERT_ONLY) ||
+	       (flags & XFS_BMAPI_CONVERT));
 
 	if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ifp)) ||
 	    XFS_TEST_ERROR(mp, XFS_ERRTAG_BMAPIFORMAT)) {
@@ -4237,6 +4241,9 @@ xfs_bmapi_write(
 		 * that we found, if any.
 		 */
 		if (need_alloc || wasdelay) {
+			if (flags & XFS_BMAPI_CONVERT_ONLY)
+				break;
+
 			bma.eof = eof;
 			bma.conv = !!(flags & XFS_BMAPI_CONVERT);
 			bma.wasdel = wasdelay;
@@ -4332,10 +4339,13 @@ xfs_bmapi_write(
 	 *
 	 * To avoid a successful return with *nmap set to 0, return the magic
 	 * -ENOSR error code for this particular case so that the caller can
-	 * handle it.
+	 * handle it. The same goes for XFS_BMAPI_CONVERT_ONLY stopping at a
+	 * hole or a delalloc extent at the start of the range, in which case
+	 * nothing has been changed.
 	 */
 	if (!n) {
-		ASSERT(bma.nallocs >= *nmap);
+		ASSERT(bma.nallocs >= *nmap ||
+		       (flags & XFS_BMAPI_CONVERT_ONLY));
 		return -ENOSR;
 	}
 	*nmap = n;
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index e9f5416e4c..fb7481e819 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -90,6 +90,13 @@ struct xfs_bmalloca {
 /* Try to align allocations to the extent size hint */
 #define XFS_BMAPI_EXTSZALIGN	(1u << 11)
 
+/*
+ * Use with XFS_BMAPI_CONVERT to convert unwritten extents without allocating
+ * blocks. Stop at a hole or a delalloc extent: return the mappings found so
+ * far, or -ENOSR if there are none.
+ */
+#define XFS_BMAPI_CONVERT_ONLY	(1u << 12)
+
 #define XFS_BMAPI_FLAGS \
 	{ XFS_BMAPI_ENTIRE,	"ENTIRE" }, \
 	{ XFS_BMAPI_METADATA,	"METADATA" }, \
@@ -102,7 +109,8 @@ struct xfs_bmalloca {
 	{ XFS_BMAPI_COWFORK,	"COWFORK" }, \
 	{ XFS_BMAPI_NODISCARD,	"NODISCARD" }, \
 	{ XFS_BMAPI_NORMAP,	"NORMAP" },\
-	{ XFS_BMAPI_EXTSZALIGN,	"EXTSZALIGN" }
+	{ XFS_BMAPI_EXTSZALIGN,	"EXTSZALIGN" }, \
+	{ XFS_BMAPI_CONVERT_ONLY,	"CONVERT_ONLY" }
 
 
 static inline int xfs_bmapi_aflag(int w)

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end Daejun Park via B4 Relay
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_commit_blocks() passes every range that a pNFS client commits to
xfs_iomap_write_unwritten(), whatever is mapped there, and the range can
be a hole. nfsd does not check the extents of a block or SCSI layout
LAYOUTCOMMIT against the layouts it handed out, and for a reclaim during
the grace period it has no layouts to check against, so a client can
commit a range that it never had a layout for.

For a hole, xfs_bmapi_write() with XFS_BMAPI_CONVERT allocates new
blocks as written extents, while the transaction only reserved blocks
for bmbt splits. A hole larger than that reservation shuts XFS down:

  XFS (nvme0n1): Corruption of in-memory data (0x8) detected at
    xfs_trans_mod_sb+0x817/0x960 (fs/xfs/xfs_trans.c:351).
    Shutting down filesystem.

A smaller hole gets blocks that nobody wrote mapped into the file as
data, so readers see whatever those blocks held, and the LAYOUTCOMMIT
succeeds.

With pynfs as the client and an RW layout for the first 64 KiB of a
2 MiB sparse file, a LAYOUTCOMMIT of 64 KiB at offset 1 MiB, which no
layout covered, got NFS4ERR_IO and shut XFS down. A LAYOUTCOMMIT of
4 KiB at 1 MiB got NFS4_OK, and the file got a written block there. The
client wrote nothing to the device in either case.

A client may only write to blocks that a layout handed out, and
xfs_fs_map_blocks() allocates blocks for any hole in a write layout, so
a committed range without blocks holds nothing that can be committed.
Pass XFS_BMAPI_CONVERT_ONLY from xfs_fs_commit_blocks() through
xfs_iomap_write_unwritten(), and fail with -EIO when xfs_bmapi_write()
finds a hole or a delalloc extent. Each xfs_bmapi_write() call finds the
extent and converts it in one transaction under the ILOCK, so nothing
can change it between the check and the conversion. -EIO is what
xfs_pnfs_validate_isize() returns when the new size falls into a hole.

xfs_iomap_write_unwritten() commits the transaction instead of
cancelling it when it fails this way, as xfs_alloc_file_space() does
after -ENOSR. xfs_iext_count_extend() logs the inode when it moves the
inode to large extent counters, which happens when the data fork of an
inode without large extent counts nears 2^31 extents on a filesystem
that supports them, and cancelling a dirty transaction shuts the
filesystem down.

Ranges before the hole have been converted by the time the commit
fails, as they would have been had the client committed them in a
LAYOUTCOMMIT of their own. Later ranges are left alone, and the
timestamps and the size are not updated.

With this, both LAYOUTCOMMITs get NFS4ERR_IO, XFS keeps running and no
block is allocated.

Link: https://lore.kernel.org/r/20261014130834.GB15773@lst.de
Fixes: 527851124d10 ("xfs: implement pNFS export operations")
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_file.c  |  2 +-
 fs/xfs/xfs_ioend.c |  2 +-
 fs/xfs/xfs_iomap.c | 20 +++++++++++++++++---
 fs/xfs/xfs_iomap.h |  3 ++-
 fs/xfs/xfs_pnfs.c  |  9 ++++++++-
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 7d0c8a1975..720f4a9b40 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -666,7 +666,7 @@ xfs_dio_write_end_io(
 	 * they are converted.
 	 */
 	if (flags & IOMAP_DIO_UNWRITTEN) {
-		error = xfs_iomap_write_unwritten(ip, offset, size, true);
+		error = xfs_iomap_write_unwritten(ip, offset, size, true, 0);
 		goto out;
 	}
 
diff --git a/fs/xfs/xfs_ioend.c b/fs/xfs/xfs_ioend.c
index 48becc3bf6..7f4a4a95af 100644
--- a/fs/xfs/xfs_ioend.c
+++ b/fs/xfs/xfs_ioend.c
@@ -237,7 +237,7 @@ xfs_end_ioend_write(
 	if (ioend->io_flags & IOMAP_IOEND_SHARED)
 		error = xfs_reflink_end_cow(ip, offset, size);
 	else if (ioend->io_flags & IOMAP_IOEND_UNWRITTEN)
-		error = xfs_iomap_write_unwritten(ip, offset, size, false);
+		error = xfs_iomap_write_unwritten(ip, offset, size, false, 0);
 
 	if (!error &&
 	    !(ioend->io_flags & IOMAP_IOEND_DIRECT) &&
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 329c745e31..99d48081c5 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -614,7 +614,8 @@ xfs_iomap_write_unwritten(
 	xfs_inode_t	*ip,
 	xfs_off_t	offset,
 	xfs_off_t	count,
-	bool		update_isize)
+	bool		update_isize,
+	uint32_t	bmapi_flags)
 {
 	xfs_mount_t	*mp = ip->i_mount;
 	xfs_fileoff_t	offset_fsb;
@@ -676,8 +677,21 @@ xfs_iomap_write_unwritten(
 		 */
 		nimaps = 1;
 		error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb,
-					XFS_BMAPI_CONVERT, resblks, &imap,
-					&nimaps);
+					XFS_BMAPI_CONVERT | bmapi_flags,
+					resblks, &imap, &nimaps);
+		if (error == -ENOSR && (bmapi_flags & XFS_BMAPI_CONVERT_ONLY)) {
+			/*
+			 * Nothing was mapped: the rest of the range starts
+			 * with a hole or a delalloc extent. Nothing was
+			 * changed either, but xfs_iext_count_extend() may
+			 * have logged the inode, and cancelling a dirty
+			 * transaction shuts the filesystem down. Commit it,
+			 * as xfs_alloc_file_space() does on -ENOSR.
+			 */
+			error = xfs_trans_commit(tp);
+			xfs_iunlock(ip, XFS_ILOCK_EXCL);
+			return error ? error : -EIO;
+		}
 		if (error)
 			goto error_on_bmapi_transaction;
 
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index f2520a9b3a..8fc0c6fc8b 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_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);
+int xfs_iomap_write_unwritten(struct xfs_inode *ip, xfs_off_t offset,
+		xfs_off_t count, bool update_isize, uint32_t bmapi_flags);
 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 a9f0b472f3..7ff83274a5 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -344,7 +344,14 @@ xfs_fs_commit_blocks(
 					(end - 1) >> PAGE_SHIFT);
 		WARN_ON_ONCE(error);
 
-		error = xfs_iomap_write_unwritten(ip, start, length, false);
+		/*
+		 * A client may only write to blocks that a layout handed out,
+		 * and xfs_fs_map_blocks() allocates blocks for any hole in a
+		 * write layout, so refuse to allocate blocks for a hole or a
+		 * delalloc extent in the committed range.
+		 */
+		error = xfs_iomap_write_unwritten(ip, start, length, false,
+				XFS_BMAPI_CONVERT_ONLY);
 		if (error)
 			goto out_drop_iolock;
 	}

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
                   ` (2 preceding siblings ...)
  2026-10-20  3:08 ` [PATCH v4 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-22 12:44   ` Pankaj Raghav (Samsung)
  2026-10-20  3:08 ` [PATCH v4 5/9] xfs: limit the count of a pNFS write allocation to one extent Daejun Park via B4 Relay
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_commit_blocks() clamps the end of each committed extent to the
file size, but computes the end as offset + length first. A length
from the client can make that sum wrap around, and the loop then
passes the client's length, unclamped, to xfs_iomap_write_unwritten().
The range wraps there as well, and xfs_bmapi_write() maps nothing.
Before the previous two patches, that failed an assertion on XFS_DEBUG
kernels, which is a BUG with XFS_ASSERT_FATAL, and otherwise returned
-ENOSR, which nfsd warns about as a non-standard errno. With
XFS_BMAPI_CONVERT_ONLY, the LAYOUTCOMMIT fails with -EIO and converts
nothing, even when every block up to the size is allocated.

With pynfs as the client and an RW layout for the first 64 KiB of a
2 MiB sparse file, a LAYOUTCOMMIT of 2^64 - 4 KiB at 4 KiB got
NFS4ERR_IO, "nfsd: non-standard errno: -63" and a WARNING, or the
assertion on XFS_DEBUG. With the previous two patches, it gets
NFS4ERR_IO and the laid-out range stays unwritten.

Clamp the length to what is left below the size before adding it to
the offset. The extent then ends at the size, its allocated part is
converted, and the LAYOUTCOMMIT gets NFS4ERR_IO at the hole after it.

Fixes: 527851124d10 ("xfs: implement pNFS export operations")
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 7ff83274a5..19eb18e129 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -328,9 +328,7 @@ xfs_fs_commit_blocks(
 		if (start > size)
 			continue;
 
-		end = start + maps[i].length;
-		if (end > size)
-			end = size;
+		end = start + min_t(u64, maps[i].length, size - start);
 
 		length = end - start;
 		if (!length)

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 5/9] xfs: limit the count of a pNFS write allocation to one extent
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
                   ` (3 preceding siblings ...)
  2026-10-20  3:08 ` [PATCH v4 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-22 13:02   ` Pankaj Raghav (Samsung)
  2026-10-20  3:08 ` [PATCH v4 6/9] xfs: clamp the pNFS layout range to the maximum file size Daejun Park via B4 Relay
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_map_blocks() asks xfs_iomap_write_direct() to allocate from the
start of a hole to the end of the range of a LAYOUTGET, which a client
chooses, or to the end of the hole if the range ends before EOF.
xfs_iomap_write_direct() reserves blocks for that whole count, in an
unsigned int, but xfs_bmapi_write() maps at most XFS_MAX_BMBT_EXTLEN
blocks in one mapping. A count of about 2^32 blocks or more wraps the
reservation. If it wraps to a few blocks, the allocation uses more
blocks than the transaction reserved, and xfs_trans_mod_sb() shuts the
filesystem down. With 4 KiB blocks, an RW LAYOUTGET at offset 0 of an
empty file for 16 TiB does it.

Direct I/O, DAX and buffered writes with an extent size hint never ask
for that much, as xfs_direct_write_iomap_begin() limits them to 1024
pages to keep the count below 32 bits. Limit a pNFS allocation to
XFS_MAX_BMBT_EXTLEN blocks, as xfs_alloc_file_space() limits each of
its reservations for the same reason. This shrinks the reservation; the
mapping that xfs_bmapi_write() returns stays the same. nfsd asks again
for the rest of the range since commit cc6c40e09d7b ("NFSD/blocklayout:
Support multiple extents per LAYOUTGET"), while the reply has room for
another extent; before that, the layout ends with the extent and the
client asks again.

An RW LAYOUTGET for more than the free space used to fail with
NFS4ERR_NOSPC before anything was allocated, as the first call that
allocates tried to reserve blocks for all of it. Where nfsd maps one
extent per LAYOUTGET, it now gets the first extent. Where nfsd asks
again, it allocates one extent after the other: if the reply fills up
first (at most 92 extents with 4 KiB pages), the client gets those
extents, and if fewer than XFS_MAX_BMBT_EXTLEN blocks are free first,
nfsd fails the LAYOUTGET. The blocks it allocated stay past EOF, as
the inode is marked preallocated; xfs_alloc_file_space() likewise
keeps what it allocated when it runs out of space.

With pynfs as the client and a 32 GiB XFS, an RW LAYOUTGET at offset 0
of an empty file for 16 TiB shut the filesystem down: "Corruption of
in-memory data (0x8) detected at xfs_trans_mod_sb". With this patch it
gets NFS4ERR_NOSPC, also with XFS_DEBUG, XFS keeps running, and less
than 8 GiB of the filesystem, one extent's worth, is left free. One
for 100 GiB ends the same way, where it got NFS4ERR_NOSPC with nothing
allocated before. One for 16 GiB, which fits, is granted the same
three extents as before.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Link: https://lore.kernel.org/r/20261008154017.GO2705364@frogsfrogsfrogs
Suggested-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20261015060932.GD14035@lst.de
Fixes: 527851124d10 ("xfs: implement pNFS export operations")
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/xfs/xfs_pnfs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 19eb18e129..401a790299 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -224,6 +224,14 @@ xfs_fs_map_blocks(
 		else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
 			end_fsb = min(end_fsb, imap.br_startoff +
 					       imap.br_blockcount);
+		/*
+		 * xfs_bmapi_write() maps at most XFS_MAX_BMBT_EXTLEN blocks
+		 * in one mapping, so don't reserve blocks for more. The
+		 * range comes from a client, and its block count may not
+		 * even fit the 32-bit reservation. The rest of the range
+		 * is mapped by a later call.
+		 */
+		end_fsb = min(end_fsb, offset_fsb + XFS_MAX_BMBT_EXTLEN);
 		xfs_iunlock(ip, lock_flags);
 
 		error = xfs_iomap_write_direct(ip, offset_fsb,

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 6/9] xfs: clamp the pNFS layout range to the maximum file size
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
                   ` (4 preceding siblings ...)
  2026-10-20  3:08 ` [PATCH v4 5/9] xfs: limit the count of a pNFS write allocation to one extent Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 7/9] xfs: take the invalidate lock while mapping a pNFS layout Daejun Park via B4 Relay
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, stable, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_map_blocks() checks the range of a layout with

	if (offset > limit)
		goto out_unlock;
	if (offset > limit - length)
		length = limit - offset;

limit - length is computed in u64, so a length larger than the limit
wraps and the length is not clamped. nfsd passes the LAYOUTGET range
through, and RFC 8881 lets a client ask for NFS4_UINT64_MAX bytes, for
the rest of the file. offset + length then wraps to offset - 1, end_fsb
rounds back to offset_fsb, and xfs_bmapi_read() maps nothing. For a read
layout, xfs_bmbt_to_iomap() then gets an uninitialized imap: with a
zeroed stack it logs "Access to block zero", marks the data fork sick
and fails with -EFSCORRUPTED, and otherwise the mapping is whatever was
on the stack and can reach the client. For a write layout,
xfs_iomap_write_direct() is called with a count of zero, which trips an
ASSERT with XFS_DEBUG. An offset of 2^63 or more, which nfsd accepts
too, becomes negative in the loff_t argument and passes the first check.

Reject a negative offset and an offset at the limit, and compare the
length with what is left up to the limit, which cannot wrap once the
offset is below it.

With pynfs as the client, a READ LAYOUTGET for NFS4_UINT64_MAX bytes at
offset 0 got NFS4ERR_IO, as did an RW one on an empty file, which also
made nfsd warn
about the non-standard errno -63, and a READ LAYOUTGET at offset 2^63
got NFS4ERR_IO as well. With this patch and the previous one, the one at
2^63 gets NFS4ERR_INVAL, the RW one gets NFS4ERR_NOSPC after nfsd has
allocated one extent after the other until fewer than
XFS_MAX_BMBT_EXTLEN blocks were free, which left less than 8 GiB of the
32 GiB free, and the server logs nothing. The READ one gets NFS4ERR_INVAL
as well: nfsd, which maps a layout one extent at a time since
commit cc6c40e09d7b ("NFSD/blocklayout: Support multiple extents per
LAYOUTGET"), asks again from the maximum file size on. An RW one on a
file whose first 64 KiB are allocated, with a loga_maxcount that has
room for one extent, is granted 0+65536.

This depends on the previous patch. Without it, an RW LAYOUTGET to the
end of the file, whose length is now clamped instead of wrapping, asks
xfs_iomap_write_direct() for every block up to the maximum file size,
which wraps the block reservation and shuts the filesystem down where it
got NFS4ERR_IO before.

Fixes: 527851124d10 ("xfs: implement pNFS export operations")
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_pnfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 401a790299..7b508f63d3 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -176,9 +176,9 @@ xfs_fs_map_blocks(
 	if (!write)
 		limit = max(limit, round_up(i_size_read(inode),
 				     inode->i_sb->s_blocksize));
-	if (offset > limit)
+	if (offset < 0 || offset >= limit)
 		goto out_unlock;
-	if (offset > limit - length)
+	if (length > limit - offset)
 		length = limit - offset;
 
 	error = filemap_write_and_wait(inode->i_mapping);

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 7/9] xfs: take the invalidate lock while mapping a pNFS layout
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
                   ` (5 preceding siblings ...)
  2026-10-20  3:08 ` [PATCH v4 6/9] xfs: clamp the pNFS layout range to the maximum file size Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 8/9] xfs: add datasync and log_flushed arguments to xfs_log_force_inode() Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 9/9] xfs: make pNFS LAYOUTCOMMIT stable the way fdatasync does Daejun Park via B4 Relay
  8 siblings, 0 replies; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_map_blocks() flushes and invalidates the page cache of the file
under the iolock and then maps the range, but a write fault takes only
the invalidate lock (XFS_MMAPLOCK_SHARED in __xfs_write_fault()), not
the iolock. A fault on a shared mapping of the file can therefore add a
delalloc extent to the range between the flush and the mapping, which
trips the ASSERT on DELAYSTARTBLOCK, or is handed to nfsd, which warns
about a filesystem that returned a delalloc extent and refuses the
layout.

Take the invalidate lock in exclusive mode together with the iolock, as
__xfs_file_fallocate() does, so that page faults wait until the layout
has been mapped.

Not marked for stable: the race needs a process on the server that
writes to a shared mapping of the file while a client gets a layout for
the same range, and the comment above xfs_break_leased_layouts() already
leaves such writers unsynchronized with pNFS clients.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Link: https://lore.kernel.org/r/20260519145949.GH9555@frogsfrogsfrogs
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 7b508f63d3..8d7a71db5f 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -168,8 +168,11 @@ xfs_fs_map_blocks(
 	 * similar to direct I/O, except that the synchronization is much more
 	 * complicated.  See the comment near xfs_break_leased_layouts
 	 * for a detailed explanation.
+	 *
+	 * Take the invalidate lock as well, so that page faults cannot add
+	 * delalloc extents to the range while it is being mapped.
 	 */
-	xfs_ilock(ip, XFS_IOLOCK_EXCL);
+	xfs_ilock(ip, XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL);
 
 	error = -EINVAL;
 	limit = mp->m_super->s_maxbytes;
@@ -260,13 +263,13 @@ xfs_fs_map_blocks(
 		if (error)
 			goto out_unlock;
 	}
-	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+	xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL);
 
 	error = xfs_bmbt_to_iomap(ip, iomap, &imap, 0, 0, seq);
 	*device_generation = mp->m_generation;
 	return error;
 out_unlock:
-	xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+	xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL);
 	return error;
 }
 

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 8/9] xfs: add datasync and log_flushed arguments to xfs_log_force_inode()
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
                   ` (6 preceding siblings ...)
  2026-10-20  3:08 ` [PATCH v4 7/9] xfs: take the invalidate lock while mapping a pNFS layout Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  2026-10-20  3:08 ` [PATCH v4 9/9] xfs: make pNFS LAYOUTCOMMIT stable the way fdatasync does Daejun Park via B4 Relay
  8 siblings, 0 replies; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_log_force_inode() is xfs_fsync_flush_log() without a datasync
variant and without the log_flushed result, plus a check for an inode
without a log item. Add the datasync and log_flushed arguments to
xfs_log_force_inode(), move the body and the comment of
xfs_fsync_flush_log() into it, and call it from xfs_file_fsync(), so
that code outside xfs_file.c can force the log for a datasync. The other
callers pass false and NULL. The next patch uses it in the pNFS
LAYOUTCOMMIT path.

No functional change: xfs_file_fsync() only calls it for an inode with a
log item, and the other callers force the log as before.

Suggested-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20261014130124.GA15407@lst.de
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_export.c  |  2 +-
 fs/xfs/xfs_file.c    | 50 ++++----------------------------------------------
 fs/xfs/xfs_inode.c   | 36 ++++++++++++++++++++++++++++++++----
 fs/xfs/xfs_inode.h   |  3 ++-
 fs/xfs/xfs_pnfs.c    |  2 +-
 fs/xfs/xfs_zone_gc.c |  2 +-
 6 files changed, 41 insertions(+), 54 deletions(-)

diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
index 9afda52ed1..9dd6dd8c27 100644
--- a/fs/xfs/xfs_export.c
+++ b/fs/xfs/xfs_export.c
@@ -234,7 +234,7 @@ STATIC int
 xfs_fs_nfs_commit_metadata(
 	struct inode		*inode)
 {
-	return xfs_log_force_inode(XFS_I(inode));
+	return xfs_log_force_inode(XFS_I(inode), false, NULL);
 }
 
 const struct export_operations xfs_export_operations = {
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 720f4a9b40..27176a4808 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -76,49 +76,7 @@ xfs_dir_fsync(
 	struct xfs_inode	*ip = XFS_I(file->f_mapping->host);
 
 	trace_xfs_dir_fsync(ip);
-	return xfs_log_force_inode(ip);
-}
-
-/*
- * All metadata updates are logged, which means that we just have to push the
- * journal to the required sequence number than holds the updates. We track
- * datasync commits separately to full sync commits, and hence only need to
- * select the correct sequence number for the log force here.
- *
- * We don't have to serialise against concurrent modifications, as we do not
- * have to wait for modifications that have not yet completed. We define a
- * transaction commit as completing when the commit sequence number is updated,
- * hence if the sequence number has not updated, the sync operation has been
- * run before the commit completed and we don't have to wait for it.
- *
- * If we have concurrent fsync/fdatasync() calls, the sequence numbers remain
- * set on the log item until - at least - the journal flush completes. In
- * reality, they are only cleared when the inode is fully unpinned (i.e.
- * persistent in the journal and not dirty in the CIL), and so we rely on
- * xfs_log_force_seq() either skipping sequences that have been persisted or
- * waiting on sequences that are still in flight to correctly order concurrent
- * sync operations.
- */
-static int
-xfs_fsync_flush_log(
-	struct xfs_inode	*ip,
-	bool			datasync,
-	int			*log_flushed)
-{
-	struct xfs_inode_log_item *iip = ip->i_itemp;
-	xfs_csn_t		seq = 0;
-
-	spin_lock(&iip->ili_lock);
-	if (datasync)
-		seq = iip->ili_datasync_seq;
-	else
-		seq = iip->ili_commit_seq;
-	spin_unlock(&iip->ili_lock);
-
-	if (!seq)
-		return 0;
-
-	return xfs_log_force_seq(ip->i_mount, seq, XFS_LOG_SYNC, log_flushed);
+	return xfs_log_force_inode(ip, false, NULL);
 }
 
 STATIC int
@@ -151,7 +109,7 @@ xfs_file_fsync(
 	 * no modifications to this inode that need to be pushed out.
 	 */
 	if (ip->i_itemp) {
-		error = xfs_fsync_flush_log(ip, datasync, &log_flushed);
+		error = xfs_log_force_inode(ip, datasync, &log_flushed);
 		if (error)
 			return error;
 	}
@@ -1599,7 +1557,7 @@ __xfs_file_fallocate(
 	}
 
 	if (!error && xfs_file_sync_writes(file))
-		error = xfs_log_force_inode(ip);
+		error = xfs_log_force_inode(ip, false, NULL);
 
 out_unlock:
 	xfs_iunlock(ip, iolock);
@@ -1750,7 +1708,7 @@ xfs_file_remap_range(
 		goto out_unlock;
 
 	if (xfs_file_sync_writes(file_in) || xfs_file_sync_writes(file_out))
-		xfs_log_force_inode(dest);
+		xfs_log_force_inode(dest, false, NULL);
 out_unlock:
 	xfs_iunlock2_remapping(src, dest);
 	if (ret)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 15b62574b8..0d1c9531cd 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2669,11 +2669,36 @@ xfs_irele(
 }
 
 /*
- * Ensure all committed transactions touching the inode are written to the log.
+ * Ensure all committed transactions touching the inode are written to the log,
+ * or with @datasync only those that fdatasync has to wait for.
+ *
+ * All metadata updates are logged, which means that we just have to push the
+ * journal to the required sequence number that holds the updates. We track
+ * datasync commits separately to full sync commits, and hence only need to
+ * select the correct sequence number for the log force here.
+ *
+ * We don't have to serialise against concurrent modifications, as we do not
+ * have to wait for modifications that have not yet completed. We define a
+ * transaction commit as completing when the commit sequence number is updated,
+ * hence if the sequence number has not updated, the sync operation has been
+ * run before the commit completed and we don't have to wait for it.
+ *
+ * If we have concurrent fsync/fdatasync() calls, the sequence numbers remain
+ * set on the log item until - at least - the journal flush completes. In
+ * reality, they are only cleared when the inode is fully unpinned (i.e.
+ * persistent in the journal and not dirty in the CIL), and so we rely on
+ * xfs_log_force_seq() either skipping sequences that have been persisted or
+ * waiting on sequences that are still in flight to correctly order concurrent
+ * sync operations.
+ *
+ * If @log_flushed is not NULL, it is set when the log force wrote an iclog,
+ * which flushes the device caches before the log write.
  */
 int
 xfs_log_force_inode(
-	struct xfs_inode	*ip)
+	struct xfs_inode	*ip,
+	bool			datasync,
+	int			*log_flushed)
 {
 	struct xfs_inode_log_item *iip = ip->i_itemp;
 	xfs_csn_t		seq = 0;
@@ -2682,12 +2707,15 @@ xfs_log_force_inode(
 		return 0;
 
 	spin_lock(&iip->ili_lock);
-	seq = iip->ili_commit_seq;
+	if (datasync)
+		seq = iip->ili_datasync_seq;
+	else
+		seq = iip->ili_commit_seq;
 	spin_unlock(&iip->ili_lock);
 
 	if (!seq)
 		return 0;
-	return xfs_log_force_seq(ip->i_mount, seq, XFS_LOG_SYNC, NULL);
+	return xfs_log_force_seq(ip->i_mount, seq, XFS_LOG_SYNC, log_flushed);
 }
 
 /*
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 1fa442b646..6dfd9271e6 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -595,7 +595,8 @@ int		xfs_ifree(struct xfs_trans *, struct xfs_inode *);
 int		xfs_itruncate_extents_flags(struct xfs_trans **,
 				struct xfs_inode *, int, xfs_fsize_t, int);
 
-int		xfs_log_force_inode(struct xfs_inode *ip);
+int		xfs_log_force_inode(struct xfs_inode *ip, bool datasync,
+				int *log_flushed);
 void		xfs_iunpin_wait(xfs_inode_t *);
 #define xfs_ipincount(ip)	((unsigned int) atomic_read(&ip->i_pincount))
 
diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 8d7a71db5f..1e47d0aa6a 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -118,7 +118,7 @@ xfs_fs_map_update_inode(
 	 * Force the log so that the blocks handed out to the client are
 	 * guaranteed to be present even after a server crash.
 	 */
-	return xfs_log_force_inode(ip);
+	return xfs_log_force_inode(ip, false, NULL);
 }
 
 /*
diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index 54b70ed292..cea5447a88 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -1018,7 +1018,7 @@ xfs_zone_gc_reset_zones(
 		struct xfs_gc_bio	*chunk;
 		struct bio		*bio;
 
-		xfs_log_force_inode(rtg_rmap(rtg));
+		xfs_log_force_inode(rtg_rmap(rtg), false, NULL);
 
 		next = rtg_group(rtg)->xg_next_reset;
 		rtg_group(rtg)->xg_next_reset = NULL;

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 9/9] xfs: make pNFS LAYOUTCOMMIT stable the way fdatasync does
  2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
                   ` (7 preceding siblings ...)
  2026-10-20  3:08 ` [PATCH v4 8/9] xfs: add datasync and log_flushed arguments to xfs_log_force_inode() Daejun Park via B4 Relay
@ 2026-10-20  3:08 ` Daejun Park via B4 Relay
  8 siblings, 0 replies; 14+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-20  3:08 UTC (permalink / raw)
  To: Carlos Maiolino, linux-xfs
  Cc: Christoph Hellwig, Darrick J. Wong, Dave Chinner, Chuck Lever,
	Jeff Layton, Dai Ngo, Sergey Bashirov, Andrey Albershteyn,
	linux-nfs, linux-kernel, Daejun Park

From: Daejun Park <daejun7.park@samsung.com>

xfs_fs_commit_blocks() commits the timestamp update of every
LAYOUTCOMMIT as a synchronous transaction, so every LAYOUTCOMMIT forces
the log. When the client only overwrote blocks that were already written
on disk and the size does not change, the only metadata that this log
force makes stable for the file is the new timestamps. fdatasync after
the same overwrite does not force the log: a timestamp-only change does
not set ili_datasync_seq, and without it xfs_file_fsync() only flushes
the cache of the device that holds the file data. The comment above
xfs_fs_commit_blocks() says that it relies on always committing a
transaction, and that it would otherwise have to flush the cache the way
fsync does for a datasync.

Do it the way fdatasync does: log the timestamps as XFS_ILOG_TIMESTAMP
when the size does not change, commit the transaction without
XFS_TRANS_SYNC, force the log up to ili_datasync_seq, and flush the
cache of the device that holds the file data unless that log force wrote
an iclog. The unwritten extent conversions and a new size set
ili_datasync_seq, so a LAYOUTCOMMIT that converts extents or changes the
size still forces the log. So does a conversion that was done earlier,
for example by a direct write on the server: ili_datasync_seq stays set
until that conversion is on disk, so the log force covers it.

With the block layout on an NVMe/TCP namespace in QEMU, backed by a
partition of an NVMe SSD with a volatile write cache, one client process
overwriting a 16 MiB file in 64 KiB writes, each followed by fsync, took
473 ms instead of 800 ms (median of 5 runs). On average a LAYOUTCOMMIT
spent 1.1 ms in xfs_fs_commit_blocks() instead of 2.4 ms, and none wrote
an iclog. With 4 KiB writes to a 4 MiB file it took 1841 ms instead of
3101 ms. A LAYOUTCOMMIT now issues an empty cache flush instead of a log
write with REQ_PREFLUSH and REQ_FUA. The guest that ran nvmet reached
the SSD through virtio-blk, which has no FUA, so there the FUA cost a
second cache flush. The gain should be smaller with native FUA and on a
device without a volatile write cache. It is also smaller with several
writers: 4 threads overwriting 16 MiB files in 64 KiB writes took
1011 ms instead of 1083 ms. LAYOUTCOMMITs of new files force the log as
before.

As with fdatasync, the timestamps of a LAYOUTCOMMIT that converts
nothing and keeps the size, and the change attribute that nfsd derives
from the ctime, reach the disk with a later log write instead of before
the reply, at the latest the one that the log worker issues every 30 s
by default. After a crash in that window, the change attribute can go
back to the value it had before the LAYOUTCOMMIT, while the data the
client wrote is on disk. The Linux client sends LAYOUTCOMMIT from both
fsync() and fdatasync() (pnfs_generic_sync() ignores datasync), so
fsync() on a block layout client now gets the timestamp guarantee of
fdatasync(). RFC 8881 section 12.5.4 says that LAYOUTCOMMIT may require
committing the block list to stable storage, which this keeps, and
section 12.5.4.1 only asks that the updated attributes be visible when
it completes.

Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_pnfs.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
index 1e47d0aa6a..2d238a0e5a 100644
--- a/fs/xfs/xfs_pnfs.c
+++ b/fs/xfs/xfs_pnfs.c
@@ -304,10 +304,12 @@ xfs_pnfs_validate_isize(
  * converting any unwritten extents, flushing the disk cache and updating the
  * time stamps.
  *
- * Note that we rely on the caller to always send us a timestamp update so that
- * we always commit a transaction here.  If that stops being true we will have
- * to manually flush the cache here similar to what the fsync code path does
- * for datasyncs on files that have no dirty metadata.
+ * Like fdatasync, a commit that only updates the timestamps does not force the
+ * log: they are logged as XFS_ILOG_TIMESTAMP and reach the disk with a later
+ * log write. The log is forced up to the last change that a datasync needs,
+ * such as the unwritten extent conversions or a new size, and the cache of the
+ * device holding the file data is flushed if that log force did not write an
+ * iclog itself.
  */
 static int
 xfs_fs_commit_blocks(
@@ -321,6 +323,7 @@ xfs_fs_commit_blocks(
 	struct xfs_trans	*tp;
 	struct timespec64	now;
 	bool			update_isize = false;
+	int			log_flushed = 0;
 	int			error, i;
 	loff_t			size;
 
@@ -377,7 +380,8 @@ xfs_fs_commit_blocks(
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
-	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+	xfs_trans_log_inode(tp, ip,
+			update_isize ? XFS_ILOG_CORE : XFS_ILOG_TIMESTAMP);
 
 	now = inode_set_ctime_current(inode);
 	inode_set_atime_to_ts(inode, now);
@@ -388,8 +392,13 @@ xfs_fs_commit_blocks(
 		ip->i_disk_size = new_size;
 	}
 
-	xfs_trans_set_sync(tp);
 	error = xfs_trans_commit(tp);
+	if (error)
+		goto out_drop_iolock;
+
+	error = xfs_log_force_inode(ip, true, &log_flushed);
+	if (!error && !log_flushed)
+		error = blkdev_issue_flush(xfs_inode_buftarg(ip)->bt_bdev);
 
 out_drop_iolock:
 	xfs_iunlock(ip, XFS_IOLOCK_EXCL);

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF
  2026-10-20  3:08 ` [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
@ 2026-10-22 12:41   ` Pankaj Raghav (Samsung)
  0 siblings, 0 replies; 14+ messages in thread
From: Pankaj Raghav (Samsung) @ 2026-10-22 12:41 UTC (permalink / raw)
  To: Daejun Park
  Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Darrick J. Wong,
	Dave Chinner, Chuck Lever, Jeff Layton, Dai Ngo, Sergey Bashirov,
	Andrey Albershteyn, linux-nfs, linux-kernel, stable

On Tue, Oct 20, 2026 at 12:08:44PM +0900, Daejun Park wrote:
> xfs_fs_map_blocks() marks the inode XFS_DIFLAG_PREALLOC after it
> allocates blocks for a write layout, so that xfs_free_eofblocks() does
> not free blocks past EOF that a client may write to. But a write layout
> for a range past EOF where blocks are already allocated, such as the
> speculative preallocation of buffered writes, gets those blocks without
> an allocation, and the inode is not marked. blockgc, or the release of
> the file, then frees them without breaking the layout. The client still
> writes to them, and they may belong to another file by then.
> 
> With pynfs as the client, a file grown to 1 MiB by two buffered writes
> on the server had 960 KiB preallocated past EOF, and an RW LAYOUTGET for
> that range got it as one INVALID_DATA extent, without the inode being
> marked. About 300 seconds later, with the default
> speculative_prealloc_lifetime, blockgc freed those blocks while the
> layout was still held. When the client then wrote through the layout and
> sent LAYOUTCOMMIT, xfs_fs_commit_blocks() found a hole where the
> unwritten extent had been, and xfs_iomap_write_unwritten() allocated for
> it with no blocks reserved:
> 
>   XFS (nvme0n1): Corruption of in-memory data (0x8) detected at
>     xfs_trans_mod_sb+0x817/0x960 (fs/xfs/xfs_trans.c:351).
>     Shutting down filesystem.
>   ...
>    xfs_trans_mod_sb+0x817/0x960
>    xfs_alloc_vextent_finish+0x2e7/0x990
>    xfs_alloc_vextent_start_ag+0x25d/0x720
>    xfs_bmap_btalloc+0xdd2/0x1720
>    xfs_bmapi_allocate+0x197/0xf90
>    xfs_bmapi_write+0x4d0/0xb60
>    xfs_iomap_write_unwritten+0x307/0x7e0
>    xfs_fs_commit_blocks+0x1d9/0x630
>    nfsd4_block_proc_layoutcommit+0x5c2/0x7d0
>    nfsd4_layoutcommit+0x4b5/0xa20
> 
> The client got NFS4ERR_IO, in six runs out of six. When small files were
> written on the server after blockgc, with a sync first so that the freed
> extents were no longer busy, they got those blocks, and the client's
> write through the layout replaced 978944 bytes of their data before the
> LAYOUTCOMMIT. A LAYOUTGET and LAYOUTCOMMIT 30 seconds apart, before
> blockgc runs, leave the client's data in place.
> 
> Mark the inode also when a write mapping ends past EOF and nothing was
> allocated, the same way as for an allocation, before the blocks are
> handed out. With this, the same test keeps the blocks past blockgc, and
> the LAYOUTCOMMIT after 360 seconds succeeds with the data in place.
> 
> To share the update with the allocation case, set a local update_inode
> flag in both cases, call xfs_fs_map_update_inode() in one place below
> them, and move the log force into xfs_fs_map_update_inode().
> 
> Fixes: 527851124d10 ("xfs: implement pNFS export operations")
> Cc: stable@vger.kernel.org
> Reported-by: Chuck Lever <cel@kernel.org>
> Link: https://lore.kernel.org/r/4de5356f-bc4d-4f1a-9c81-fce09b1b46fd@slotpi15m67
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_pnfs.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)

Looks good,
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>

-- 
Pankaj

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/9] xfs: add a flag to convert unwritten extents without allocating blocks
  2026-10-20  3:08 ` [PATCH v4 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
@ 2026-10-22 12:43   ` Pankaj Raghav (Samsung)
  0 siblings, 0 replies; 14+ messages in thread
From: Pankaj Raghav (Samsung) @ 2026-10-22 12:43 UTC (permalink / raw)
  To: Daejun Park
  Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Darrick J. Wong,
	Dave Chinner, Chuck Lever, Jeff Layton, Dai Ngo, Sergey Bashirov,
	Andrey Albershteyn, linux-nfs, linux-kernel, stable

On Tue, Oct 20, 2026 at 12:08:45PM +0900, Daejun Park wrote:
> xfs_bmapi_write() with XFS_BMAPI_CONVERT converts unwritten extents to
> written ones, and allocates blocks for any hole or delalloc extent it
> finds in the range. A caller that may only convert blocks that are
> already allocated has to look at the mapping first, and the mapping can
> change once the ILOCK is dropped.
> 
> Add XFS_BMAPI_CONVERT_ONLY. With it, xfs_bmapi_write() stops at a hole
> or a delalloc extent instead of allocating blocks and returns the
> mappings that it converted before it. If the range starts with a hole or
> a delalloc extent, it returns -ENOSR, as it does when a delalloc
> conversion maps nothing in the range. Nothing has been changed then, and
> the caller decides what to do with its transaction.
> 
> A flag of this name was removed by commit 26b91c728b2d ("xfs: make COW
> fork unwritten extent conversions more robust"). That one skipped the
> allocation and still mapped the hole; this one stops before it.
> 
> "xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks" uses it,
> which is why this is marked for stable.
> 
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Link: https://lore.kernel.org/r/20261013120359.GB2944@lst.de
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 14 ++++++++++++--
>  fs/xfs/libxfs/xfs_bmap.h | 10 +++++++++-
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index ca49bf4a50..95c0178b44 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4130,6 +4130,8 @@ xfs_bmapi_finish(
>   * Returns a negative error code on failure, including -ENOSPC when it could not
>   * allocate any blocks and -ENOSR when it did allocate blocks to convert a
>   * delalloc range, but those blocks were before the passed in range.
> + * With XFS_BMAPI_CONVERT_ONLY, it stops at a hole or a delalloc extent instead
> + * of allocating blocks, and returns -ENOSR if it stops before mapping anything.
>   */
>  int
>  xfs_bmapi_write(
> @@ -4189,6 +4191,8 @@ xfs_bmapi_write(
>  	 */
>  	ASSERT((flags & (XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO)) !=
>  			(XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO));
> +	ASSERT(!(flags & XFS_BMAPI_CONVERT_ONLY) ||
> +	       (flags & XFS_BMAPI_CONVERT));
>  
>  	if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ifp)) ||
>  	    XFS_TEST_ERROR(mp, XFS_ERRTAG_BMAPIFORMAT)) {
> @@ -4237,6 +4241,9 @@ xfs_bmapi_write(
>  		 * that we found, if any.
>  		 */
>  		if (need_alloc || wasdelay) {
> +			if (flags & XFS_BMAPI_CONVERT_ONLY)
> +				break;
> +
>  			bma.eof = eof;
>  			bma.conv = !!(flags & XFS_BMAPI_CONVERT);
>  			bma.wasdel = wasdelay;
> @@ -4332,10 +4339,13 @@ xfs_bmapi_write(
>  	 *
>  	 * To avoid a successful return with *nmap set to 0, return the magic
>  	 * -ENOSR error code for this particular case so that the caller can
> -	 * handle it.
> +	 * handle it. The same goes for XFS_BMAPI_CONVERT_ONLY stopping at a
> +	 * hole or a delalloc extent at the start of the range, in which case
> +	 * nothing has been changed.
>  	 */
>  	if (!n) {
> -		ASSERT(bma.nallocs >= *nmap);
> +		ASSERT(bma.nallocs >= *nmap ||
> +		       (flags & XFS_BMAPI_CONVERT_ONLY));
Minor nit:

The following might be more explicit here:

ASSERT((flags & XFS_BMAPI_CONVERT_ONLY) ? bma.nallocs == 0 :
                                         bma.nallocs >= *nmap);

Otherwise, the patch looks good.
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>

-- 
Pankaj

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end
  2026-10-20  3:08 ` [PATCH v4 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end Daejun Park via B4 Relay
@ 2026-10-22 12:44   ` Pankaj Raghav (Samsung)
  0 siblings, 0 replies; 14+ messages in thread
From: Pankaj Raghav (Samsung) @ 2026-10-22 12:44 UTC (permalink / raw)
  To: Daejun Park
  Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Darrick J. Wong,
	Dave Chinner, Chuck Lever, Jeff Layton, Dai Ngo, Sergey Bashirov,
	Andrey Albershteyn, linux-nfs, linux-kernel, stable

On Tue, Oct 20, 2026 at 12:08:47PM +0900, Daejun Park wrote:
> xfs_fs_commit_blocks() clamps the end of each committed extent to the
> file size, but computes the end as offset + length first. A length
> from the client can make that sum wrap around, and the loop then
> passes the client's length, unclamped, to xfs_iomap_write_unwritten().
> The range wraps there as well, and xfs_bmapi_write() maps nothing.
> Before the previous two patches, that failed an assertion on XFS_DEBUG
> kernels, which is a BUG with XFS_ASSERT_FATAL, and otherwise returned
> -ENOSR, which nfsd warns about as a non-standard errno. With
> XFS_BMAPI_CONVERT_ONLY, the LAYOUTCOMMIT fails with -EIO and converts
> nothing, even when every block up to the size is allocated.
> 
> With pynfs as the client and an RW layout for the first 64 KiB of a
> 2 MiB sparse file, a LAYOUTCOMMIT of 2^64 - 4 KiB at 4 KiB got
> NFS4ERR_IO, "nfsd: non-standard errno: -63" and a WARNING, or the
> assertion on XFS_DEBUG. With the previous two patches, it gets
> NFS4ERR_IO and the laid-out range stays unwritten.
> 
> Clamp the length to what is left below the size before adding it to
> the offset. The extent then ends at the size, its allocated part is
> converted, and the LAYOUTCOMMIT gets NFS4ERR_IO at the hole after it.
> 
> Fixes: 527851124d10 ("xfs: implement pNFS export operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_pnfs.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
Looks good.
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>

-- 
Pankaj

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 5/9] xfs: limit the count of a pNFS write allocation to one extent
  2026-10-20  3:08 ` [PATCH v4 5/9] xfs: limit the count of a pNFS write allocation to one extent Daejun Park via B4 Relay
@ 2026-10-22 13:02   ` Pankaj Raghav (Samsung)
  0 siblings, 0 replies; 14+ messages in thread
From: Pankaj Raghav (Samsung) @ 2026-10-22 13:02 UTC (permalink / raw)
  To: Daejun Park
  Cc: Carlos Maiolino, linux-xfs, Christoph Hellwig, Darrick J. Wong,
	Dave Chinner, Chuck Lever, Jeff Layton, Dai Ngo, Sergey Bashirov,
	Andrey Albershteyn, linux-nfs, linux-kernel, stable

> With pynfs as the client and a 32 GiB XFS, an RW LAYOUTGET at offset 0
> of an empty file for 16 TiB shut the filesystem down: "Corruption of
> in-memory data (0x8) detected at xfs_trans_mod_sb". With this patch it
> gets NFS4ERR_NOSPC, also with XFS_DEBUG, XFS keeps running, and less
> than 8 GiB of the filesystem, one extent's worth, is left free. One
> for 100 GiB ends the same way, where it got NFS4ERR_NOSPC with nothing
> allocated before. One for 16 GiB, which fits, is granted the same
> three extents as before.
> 
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Link: https://lore.kernel.org/r/20261008154017.GO2705364@frogsfrogsfrogs
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Link: https://lore.kernel.org/r/20261015060932.GD14035@lst.de
> Fixes: 527851124d10 ("xfs: implement pNFS export operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
>  fs/xfs/xfs_pnfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> index 19eb18e129..401a790299 100644
> --- a/fs/xfs/xfs_pnfs.c
> +++ b/fs/xfs/xfs_pnfs.c
> @@ -224,6 +224,14 @@ xfs_fs_map_blocks(
>  		else if (nimaps && imap.br_startblock == HOLESTARTBLOCK)
>  			end_fsb = min(end_fsb, imap.br_startoff +
>  					       imap.br_blockcount);
> +		/*
> +		 * xfs_bmapi_write() maps at most XFS_MAX_BMBT_EXTLEN blocks
> +		 * in one mapping, so don't reserve blocks for more. The
> +		 * range comes from a client, and its block count may not
> +		 * even fit the 32-bit reservation. The rest of the range
> +		 * is mapped by a later call.
> +		 */
> +		end_fsb = min(end_fsb, offset_fsb + XFS_MAX_BMBT_EXTLEN);

Isn't it better we add this in xfs_direct_write_iomap_begin() instead of
adding it in pnfs?

Something like this:

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 329c745e3120..50cf7b0cfc76 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -284,6 +284,7 @@ xfs_iomap_write_direct(
        int                     nr_exts = XFS_IEXT_ADD_NOSPLIT_CNT;
 
        ASSERT(count_fsb > 0);
+       count_fsb = min_t(xfs_filblks_t, count_fsb, XFS_MAX_BMBT_EXTLEN);
 
        resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
                                           xfs_get_extsz_hint(ip));

-- 
Pankaj

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-10-22 13:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-20  3:08 [PATCH v4 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
2026-10-20  3:08 ` [PATCH v4 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
2026-10-22 12:41   ` Pankaj Raghav (Samsung)
2026-10-20  3:08 ` [PATCH v4 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
2026-10-22 12:43   ` Pankaj Raghav (Samsung)
2026-10-20  3:08 ` [PATCH v4 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks Daejun Park via B4 Relay
2026-10-20  3:08 ` [PATCH v4 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end Daejun Park via B4 Relay
2026-10-22 12:44   ` Pankaj Raghav (Samsung)
2026-10-20  3:08 ` [PATCH v4 5/9] xfs: limit the count of a pNFS write allocation to one extent Daejun Park via B4 Relay
2026-10-22 13:02   ` Pankaj Raghav (Samsung)
2026-10-20  3:08 ` [PATCH v4 6/9] xfs: clamp the pNFS layout range to the maximum file size Daejun Park via B4 Relay
2026-10-20  3:08 ` [PATCH v4 7/9] xfs: take the invalidate lock while mapping a pNFS layout Daejun Park via B4 Relay
2026-10-20  3:08 ` [PATCH v4 8/9] xfs: add datasync and log_flushed arguments to xfs_log_force_inode() Daejun Park via B4 Relay
2026-10-20  3:08 ` [PATCH v4 9/9] xfs: make pNFS LAYOUTCOMMIT stable the way fdatasync does Daejun Park via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox