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@xxxxxxxxxxxxxxxx/
[2] https://lore.kernel.org/all/20261105030710.2652306-1-tmenninger@xxxxxxxxxxxxxxxx/