[PATCH v2] pNFS: avoid repeatedly cancelling I/O for the same lseg

[PATCH v2] pNFS: avoid repeatedly cancelling I/O for the same lseg

[Tim Menninger]

Commit b739a5bd9d9f ("NFSv4/flexfiles: Cancel I/O if the layout is
recalled or revoked") added I/O cancellation when a layout segment is
invalidated while outstanding references remain.

The invalid segment stays on the layout's segment list until those
references drain. Subsequent scans can find it again and repeatedly
invoke the layout driver's cancel_io() callback. For flexfiles, this
cancels matching RPC tasks and disconnects data server RPC clients.
During RPC/RDMA data server recovery, repeated cancellation can cause a
reconnect loop that consumes essentially all client CPU and prevents
forward progress.

Record cancellation requests per mirror stripe in the flexfiles layout
segment. Each stripe is cancelled at most once, regardless of how many
times the invalid segment is encountered. A per-stripe bitmap rather
than a single per-segment bit preserves the ability to cancel I/O to a
different device after an earlier device-specific cancellation.

Cancellation is best-effort. An RPC that has already passed the
layout-validity check may not yet be visible to the cancellation
scan. Such I/O retains its lseg reference and remains subject to
the normal RPC completion and recovery paths. Suppress repeated
cancellation and disconnect requests without changing the existing
lseg lifetime and deferred layout-return accounting.

This reproduced on 6.8.0, 7.0.0, and 7.4.0-rc1. It was tested on
7.4.0-rc1 with sustained pNFS/flexfiles I/O over RPC/RDMA while
restarting the NFS service on a data server. Without it, repeated
cancellation led to a reconnect loop and no workload recovery. With it,
the client recovered after the data server returned.

Fixes: b739a5bd9d9f ("NFSv4/flexfiles: Cancel I/O if the layout is recalled or revoked")
Cc: stable@vger.kernel.org
Signed-off-by: Tim Menninger <tmenninger@everpuredata.com>
---
Changes in v2:
- Rework cancellation tracking to accommodate per-device cancellation.
- Track cancellation per stripe rather than per layout segment.
- Clarify that cancellation is best-effort.
- Document the data-server restart reproducer.
- Drop Reviewed-by tag due to substantial implementation changes.

v1: https://lore.kernel.org/all/20260916232906.4101987-1-tmenninger@everpuredata.com/

 fs/nfs/flexfilelayout/flexfilelayout.c | 12 ++++++++++++
 fs/nfs/flexfilelayout/flexfilelayout.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 9492d6bc1d1a7..4419ded8fe586 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -11,6 +11,7 @@
 #include <linux/nfs_mount.h>
 #include <linux/nfs_page.h>
 #include <linux/module.h>
+#include <linux/bitmap.h>
 #include <linux/file.h>
 #include <linux/sched/mm.h>
 
@@ -340,6 +341,7 @@ static void _ff_layout_free_lseg(struct nfs4_ff_layout_segment *fls)
 {
 	if (fls) {
 		ff_layout_free_mirror_array(fls);
+		bitmap_free(fls->cancelled_stripes);
 		kfree(fls);
 	}
 }
@@ -653,6 +655,12 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh,
 		fls->mirror_array[i]->report_interval = be32_to_cpup(p);
 
 out_sort_mirrors:
+	fls->cancelled_stripes = bitmap_zalloc(
+		fls->mirror_array_cnt * dss_count, gfp_flags);
+	if (!fls->cancelled_stripes) {
+		rc = -ENOMEM;
+		goto out_err_free;
+	}
 	ff_layout_sort_mirrors(fls);
 	ret = &fls->generic_hdr;
 	dprintk("<-- %s (success)\n", __func__);
@@ -2582,6 +2590,10 @@ static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg,
 			if (devid && memcmp(&mirror->dss[dss_id].devid, devid,
 					    sizeof(*devid)) != 0)
 				continue;
+			/* Do not cancel a stripe again while this lseg drains. */
+			if (test_and_set_bit(idx * mirror->dss_count + dss_id,
+					     flseg->cancelled_stripes))
+				continue;
 			rcu_read_lock();
 			mirror_ds = rcu_dereference(mirror->dss[dss_id].mirror_ds);
 			if (IS_ERR_OR_NULL(mirror_ds) ||
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 09c3cd6964fd6..5d5587fded76f 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -109,6 +109,7 @@ struct nfs4_ff_layout_segment {
 	u64				stripe_unit;
 	u32				flags;
 	u32				mirror_array_cnt;
+	unsigned long			*cancelled_stripes;
 	struct nfs4_ff_layout_mirror	*mirror_array[] __counted_by(mirror_array_cnt);
 };
 
-- 
2.53.0

Re: [PATCH v2] pNFS: avoid repeatedly cancelling I/O for the same lseg

[Trond Myklebust]

On Thu, 2026-11-05 at 00:01 +0000, Tim Menninger wrote:
> Commit b739a5bd9d9f ("NFSv4/flexfiles: Cancel I/O if the layout is
> recalled or revoked") added I/O cancellation when a layout segment is
> invalidated while outstanding references remain.
> 
> The invalid segment stays on the layout's segment list until those
> references drain. Subsequent scans can find it again and repeatedly
> invoke the layout driver's cancel_io() callback. For flexfiles, this
> cancels matching RPC tasks and disconnects data server RPC clients.
> During RPC/RDMA data server recovery, repeated cancellation can cause
> a
> reconnect loop that consumes essentially all client CPU and prevents
> forward progress.

This claim isn't making sense to me.

If a layout segment is recalled or revoked, we should always be
clearing the NFS_LSEG_VALID flag bit in order to prevent it being
looked up again. New RPC calls should not be able to add that layout
segment.

So the only thing that can be found on a second run of
rpc_cancel_tasks() is a task that has been attempted cancelled
previously, but for which that cancel failed.
If you're seeing actual spinning, where the RPC call is just being
requeued by the rpc_call_done() callback, then we need to look at why
the latter is failing to handle the situation correctly.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH v2] pNFS: avoid repeatedly cancelling I/O for the same lseg

[tmenninger]

> This claim isn't making sense to me.
> 
> If a layout segment is recalled or revoked, we should always be
> clearing the NFS_LSEG_VALID flag bit in order to prevent it being
> looked up again. New RPC calls should not be able to add that layout
> segment.
>
> So the only thing that can be found on a second run of
> rpc_cancel_tasks() is a task that has been attempted cancelled
> previously, but for which that cancel failed.
> If you're seeing actual spinning, where the RPC call is just being
> requeued by the rpc_call_done() callback, then we need to look at why
> the latter is failing to handle the situation correctly.

Agreed. I am not seeing new I/O acquire an invalid lseg.

The issue is that the set of lsegs affected by return processing can
expand while the original I/O drains. Newer lsegs can exist on the same
layout header, with I/O that has not previously been cancelled.
Cancellation and cleanup of the old I/O can then redrive return
processing against that shared header and bring the newer lsegs into the
return scope here:

  pnfs_put_lseg(A)             [final reference]
    pnfs_cache_lseg_for_layoutreturn()
      pnfs_set_plh_return_info(..., seq=0)
    pnfs_put_layout_hdr()
      ...
      pnfs_mark_matching_lsegs_return(..., lo->plh_return_seq)

The zero argument to pnfs_set_plh_return_info() resolves to the current
header sequence. If that sequence has advanced while A was draining, the
pending return cutoff can advance with it.

For example, my trace records, on the same header:

  final put:       lseg A, pls_seq=336
  return info:     input_seq=0, resolved_seq=529
                   plh_return_seq=528 -> 529
  header-put scan: seq=529
  cancel_io:       a different lseg B, pls_seq=529

A was already covered by the previous return boundary of 528. Its final
put nevertheless expanded that boundary to 529, and the next scan
invoked cancellation for B.

B does not need to have encountered an independent I/O error to be
selected by that expanded scan. Its I/O can be cancelled for the first
time because cleanup of A broadened the return scope. As B drains, the
same interaction can affect another newer lseg:

  cancel I/O on A
    -> cleanup redrives return processing with an expanded scope
    -> invalidate B and cancel its previously uncancelled I/O
    -> cleanup of B can repeat the process

Counting only newly requested cancellations does not break that sequence
because each newly selected lseg can supply previously uncancelled
tasks.

This does not require new I/O to acquire invalid A, or the same RPC to
be repeatedly requeued by rpc_call_done(). The feedback can proceed
through completion/release processing and cancellation of different
tasks on different lsegs.

The cache-path sequence update is what I tried to address in [1]. I
asked to hold off merging it because later recovery in [2] ("SUNRPC: let
cancelled RPC/RDMA requests drain before release") testing left some
ranks slow even when the reconnect loop was gone.

[1] https://lore.kernel.org/all/20260921235520.502987-1-tmenninger@everpuredata.com/
[2] https://lore.kernel.org/all/20261105030710.2652306-1-tmenninger@everpuredata.com/