[PATCH] pNFS: Check lseg validity before marking a layout for return

[PATCH] pNFS: Check lseg validity before marking a layout for return

[tmenninger]

From: Tim Menninger <tmenninger@purestorage.com>

pnfs_error_mark_layout_for_return() receives the lseg associated with
the failed I/O but previously used only its I/O mode, operating on the
inode's current layout header regardless of whether the lseg itself was
still valid.

A layout stateid can be invalidated while RPCs still hold references to
its lsegs. pnfs_mark_layout_stateid_invalid() clears NFS_LSEG_VALID on
those lsegs through pnfs_clear_lseg_state(). A subsequent LAYOUTGET can
install a replacement stateid in the same pnfs_layout_hdr. If an RPC
using one of the old lsegs later reports an error, the current code can
therefore mark the replacement layout for return.

Once NFS_LSEG_VALID has been cleared, the lseg is no longer eligible for
selection for new I/O and must not initiate another error-driven return
of the inode's current layout. Fold pnfs_mark_layout_for_return() into
pnfs_error_mark_layout_for_return() and check pnfs_is_valid_lseg()
alongside pnfs_layout_is_valid() before setting return info, so a stale
lseg cannot drive an error return of the current layout.

This was reproduced by restarting a FlexFiles data server during a
high-throughput read workload. Stale lsegs repeatedly caused the
replacement layout to be marked for return, triggering I/O cancellation,
RPC/RDMA transport reconnects, and sustained contention on
inode->i_lock. The client did not recover on its own and consumed about
94 CPU cores on a 96-CPU system. With this change, I/O recovered within
about 20 seconds and recovery load peaked at about 15 CPU cores.

Cc: stable@vger.kernel.org
Signed-off-by: Tim Menninger <tmenninger@purestorage.com>
---
 fs/nfs/pnfs.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 7715e2bd5871..9f32dd7c4c63 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -2708,26 +2708,30 @@ pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 	return -ENOENT;
 }
 
-static void
-pnfs_mark_layout_for_return(struct inode *inode,
-			    const struct pnfs_layout_range *range)
+void pnfs_error_mark_layout_for_return(struct inode *inode,
+				       struct pnfs_layout_segment *lseg)
 {
 	struct pnfs_layout_hdr *lo;
 	bool return_now = false;
+	struct pnfs_layout_range range = {
+		.iomode = lseg->pls_range.iomode,
+		.offset = 0,
+		.length = NFS4_MAX_UINT64,
+	};
 
 	spin_lock(&inode->i_lock);
 	lo = NFS_I(inode)->layout;
-	if (!pnfs_layout_is_valid(lo)) {
+	if (!pnfs_layout_is_valid(lo) || !pnfs_is_valid_lseg(lseg)) {
 		spin_unlock(&inode->i_lock);
 		return;
 	}
-	pnfs_set_plh_return_info(lo, range->iomode, 0);
+	pnfs_set_plh_return_info(lo, range.iomode, 0);
 	/*
 	 * mark all matching lsegs so that we are sure to have no live
 	 * segments at hand when sending layoutreturn. See pnfs_put_lseg()
 	 * for how it works.
 	 */
-	if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs, range, 0) != -EBUSY) {
+	if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs, &range, 0) != -EBUSY) {
 		const struct cred *cred;
 		nfs4_stateid stateid;
 		enum pnfs_iomode iomode;
@@ -2742,18 +2746,6 @@ pnfs_mark_layout_for_return(struct inode *inode,
 		nfs_commit_inode(inode, 0);
 	}
 }
-
-void pnfs_error_mark_layout_for_return(struct inode *inode,
-				       struct pnfs_layout_segment *lseg)
-{
-	struct pnfs_layout_range range = {
-		.iomode = lseg->pls_range.iomode,
-		.offset = 0,
-		.length = NFS4_MAX_UINT64,
-	};
-
-	pnfs_mark_layout_for_return(inode, &range);
-}
 EXPORT_SYMBOL_GPL(pnfs_error_mark_layout_for_return);
 
 static bool
-- 
2.34.1

Re: [PATCH] pNFS: Check lseg validity before marking a layout for return

[Trond Myklebust]

On Thu, 2026-08-13 at 13:58 +0000, tmenninger@purestorage.com wrote:
> From: Tim Menninger <tmenninger@purestorage.com>
> 
> pnfs_error_mark_layout_for_return() receives the lseg associated with
> the failed I/O but previously used only its I/O mode, operating on
> the
> inode's current layout header regardless of whether the lseg itself
> was
> still valid.
> 
> A layout stateid can be invalidated while RPCs still hold references
> to
> its lsegs. pnfs_mark_layout_stateid_invalid() clears NFS_LSEG_VALID
> on
> those lsegs through pnfs_clear_lseg_state(). A subsequent LAYOUTGET
> can
> install a replacement stateid in the same pnfs_layout_hdr. If an RPC
> using one of the old lsegs later reports an error, the current code
> can
> therefore mark the replacement layout for return.
> 
> Once NFS_LSEG_VALID has been cleared, the lseg is no longer eligible
> for
> selection for new I/O and must not initiate another error-driven
> return
> of the inode's current layout. Fold pnfs_mark_layout_for_return()

I strongly disagree with the above premise that NFS_LSEG_VALID should
be used to gate layout returns. If the client tries to do I/O and
fails, then it is required to report the error, the type of I/O, and
the byte range to which it applies.
We attempt to redrive that layoutreturn as the outstanding I/O requests
fail. The EBUSY is there to ensure that we don't fire off the
layoutreturn until all the errors have been collected.


So NACK to this patch.
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com