* [PATCH v3 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF
2026-10-15 4:07 [PATCH v3 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
@ 2026-10-15 4:08 ` Daejun Park via B4 Relay
2026-10-15 6:02 ` Christoph Hellwig
2026-10-15 4:08 ` [PATCH v3 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-15 4: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>
---
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] 12+ messages in thread* Re: [PATCH v3 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF
2026-10-15 4:08 ` [PATCH v3 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
@ 2026-10-15 6:02 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-10-15 6:02 UTC (permalink / raw)
To: daejun7.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
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/9] xfs: add a flag to convert unwritten extents without allocating blocks
2026-10-15 4:07 [PATCH v3 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
2026-10-15 4:08 ` [PATCH v3 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
@ 2026-10-15 4:08 ` Daejun Park via B4 Relay
2026-10-15 6:04 ` Christoph Hellwig
2026-10-15 4:08 ` [PATCH v3 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks Daejun Park via B4 Relay
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-15 4: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>
---
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] 12+ messages in thread* Re: [PATCH v3 2/9] xfs: add a flag to convert unwritten extents without allocating blocks
2026-10-15 4:08 ` [PATCH v3 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
@ 2026-10-15 6:04 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-10-15 6:04 UTC (permalink / raw)
To: daejun7.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
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks
2026-10-15 4:07 [PATCH v3 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
2026-10-15 4:08 ` [PATCH v3 1/9] xfs: mark the inode preallocated for pNFS write layouts past EOF Daejun Park via B4 Relay
2026-10-15 4:08 ` [PATCH v3 2/9] xfs: add a flag to convert unwritten extents without allocating blocks Daejun Park via B4 Relay
@ 2026-10-15 4:08 ` Daejun Park via B4 Relay
2026-10-15 6:04 ` Christoph Hellwig
2026-10-15 4:08 ` [PATCH v3 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end Daejun Park via B4 Relay
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-15 4: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>
---
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] 12+ messages in thread* Re: [PATCH v3 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks
2026-10-15 4:08 ` [PATCH v3 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks Daejun Park via B4 Relay
@ 2026-10-15 6:04 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-10-15 6:04 UTC (permalink / raw)
To: daejun7.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
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end
2026-10-15 4:07 [PATCH v3 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
` (2 preceding siblings ...)
2026-10-15 4:08 ` [PATCH v3 3/9] xfs: refuse a pNFS LAYOUTCOMMIT of a range without blocks Daejun Park via B4 Relay
@ 2026-10-15 4:08 ` Daejun Park via B4 Relay
2026-10-15 4:08 ` [PATCH v3 5/9] xfs: refuse a direct allocation whose block reservation would wrap Daejun Park via B4 Relay
2026-10-15 4:08 ` [PATCH v3 6/9] xfs: clamp the pNFS layout range to the maximum file size Daejun Park via B4 Relay
5 siblings, 0 replies; 12+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-15 4: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] 12+ messages in thread* [PATCH v3 5/9] xfs: refuse a direct allocation whose block reservation would wrap
2026-10-15 4:07 [PATCH v3 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
` (3 preceding siblings ...)
2026-10-15 4:08 ` [PATCH v3 4/9] xfs: clamp a pNFS LAYOUTCOMMIT extent before computing its end Daejun Park via B4 Relay
@ 2026-10-15 4:08 ` Daejun Park via B4 Relay
2026-10-15 6:09 ` Christoph Hellwig
2026-10-15 4:08 ` [PATCH v3 6/9] xfs: clamp the pNFS layout range to the maximum file size Daejun Park via B4 Relay
5 siblings, 1 reply; 12+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-15 4: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_iomap_write_direct() maps one extent, but it reserves blocks for the
whole count it is given, in an unsigned int. 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. If it wraps to nearly 2^32
blocks, the allocation fails with -ENOSPC.
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. xfs_fs_map_blocks() has no
such limit: it asks for everything from the start of a hole to the end
of the range of a pNFS layout, which a client chooses, and an RW
LAYOUTGET at offset 0 of an empty file for 16 TiB does it.
Fail such a count with -ENOSPC before anything is reserved, also on a
filesystem with that much free space, as the transaction cannot take it.
A count that fits is reserved in full, as before, so a request for more
blocks than are free still fails before anything is allocated.
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 and nothing is allocated, as with 100 GiB, and one
for 16 GiB, which fits, is still granted, in three extents.
Fixes: 527851124d10 ("xfs: implement pNFS export operations")
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
fs/xfs/xfs_iomap.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 99d48081c5..92b9973425 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -287,6 +287,13 @@ xfs_iomap_write_direct(
resaligned = xfs_aligned_fsb_count(offset_fsb, count_fsb,
xfs_get_extsz_hint(ip));
+ /*
+ * The transaction takes the block reservation as an unsigned int.
+ * Refuse a count that does not fit rather than reserve too little;
+ * only a pNFS layout for a huge range asks for that much.
+ */
+ if (resaligned > UINT_MAX - XFS_DIOSTRAT_SPACE_RES(mp, 0))
+ return -ENOSPC;
if (unlikely(XFS_IS_REALTIME_INODE(ip))) {
dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
rblocks = resaligned;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 5/9] xfs: refuse a direct allocation whose block reservation would wrap
2026-10-15 4:08 ` [PATCH v3 5/9] xfs: refuse a direct allocation whose block reservation would wrap Daejun Park via B4 Relay
@ 2026-10-15 6:09 ` Christoph Hellwig
2026-10-15 7:23 ` Daejun Park
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2026-10-15 6:09 UTC (permalink / raw)
To: daejun7.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 Thu, Oct 15, 2026 at 01:08:04PM +0900, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
>
> xfs_iomap_write_direct() maps one extent, but it reserves blocks for the
> whole count it is given, in an unsigned int. 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. If it wraps to nearly 2^32
> blocks, the allocation fails with -ENOSPC.
>
> 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. xfs_fs_map_blocks() has no
> such limit: it asks for everything from the start of a hole to the end
> of the range of a pNFS layout, which a client chooses, and an RW
> LAYOUTGET at offset 0 of an empty file for 16 TiB does it.
>
> Fail such a count with -ENOSPC before anything is reserved, also on a
> filesystem with that much free space, as the transaction cannot take it.
> A count that fits is reserved in full, as before, so a request for more
> blocks than are free still fails before anything is allocated.
I think the limit needs to be lower, as xfs_bmapi_write can't hand
out such large limits. We'll also need to make sure pnfs can
request multiple maps.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE:(2) [PATCH v3 5/9] xfs: refuse a direct allocation whose block reservation would wrap
2026-10-15 6:09 ` Christoph Hellwig
@ 2026-10-15 7:23 ` Daejun Park
0 siblings, 0 replies; 12+ messages in thread
From: Daejun Park @ 2026-10-15 7:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Carlos Maiolino, linux-xfs@vger.kernel.org, Darrick J. Wong,
Dave Chinner, Chuck Lever, Jeff Layton, Dai Ngo, Sergey Bashirov,
Andrey Albershteyn, linux-nfs@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org, Daejun Park
On Thu, Oct 15, 2026 at 08:09:32AM +0200, Christoph Hellwig wrote:
> I think the limit needs to be lower, as xfs_bmapi_write can't hand
> out such large limits. We'll also need to make sure pnfs can
> request multiple maps.
Thanks, I'll limit the count to XFS_MAX_BMBT_EXTLEN in v4. nfsd asks
again for the rest of the range since commit cc6c40e09d7b
("NFSD/blocklayout: Support multiple extents per LAYOUTGET"), and
letting a single ->map_blocks call return several mappings is what
patches 4, 5, 7 and 8 of the ->map_blocks series did [1]; I'll resend
those for the next cycle.
[1] https://lore.kernel.org/r/20261008-xfs-nfsd-map-blocks-v1-0-560026cdccb6@samsung.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 6/9] xfs: clamp the pNFS layout range to the maximum file size
2026-10-15 4:07 [PATCH v3 0/9] xfs: pNFS block layout fixes Daejun Park via B4 Relay
` (4 preceding siblings ...)
2026-10-15 4:08 ` [PATCH v3 5/9] xfs: refuse a direct allocation whose block reservation would wrap Daejun Park via B4 Relay
@ 2026-10-15 4:08 ` Daejun Park via B4 Relay
5 siblings, 0 replies; 12+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-15 4: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, 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 before anything
is allocated, as the range up to the maximum file size cannot be
reserved, 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 19eb18e129..9a578a2d02 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] 12+ messages in thread