[PATCH] smb: client: set replay flag on the read send-error retry path

[Christopher Lusk <clusk@northecho.dev>]

smb2_async_readv() and smb2_async_writev() end with the same
send-error block: if the error is replayable and smb2_should_replay()
agrees, tell netfs to retry the subrequest. The write path also sets
wdata->replay. The read path does not set rdata->replay.

smb2_should_replay() is not a pure predicate. It consumes the retry
budget and computes the exponential back-off, doubling cur_sleep up to
CIFS_MAX_SLEEP. That back-off is only applied where the replay flag is
tested at the top of the reissued request:

	if (rdata->replay) {
		/* Back-off before retry */
		if (rdata->cur_sleep)
			msleep(rdata->cur_sleep);
		smb2_set_replay(server, &rqst);
	}

So on the read path the back-off is recomputed on every send-error
retry and then discarded, and SMB2_FLAGS_REPLAY_OPERATION is not set
on the reissued request.

netfs does not pace the retry either. netfs_reissue_read() calls
->issue_read() directly, and fs/netfs/read_retry.c contains no delay
of its own, so read send-error retries reissue immediately while the
equivalent write retries back off.

The read response callback already sets rdata->replay under the same
conditions, so the read path does use the replay mechanism. Only this
send-error path omits it.

Where the back-off belongs was settled while the commit below was
under review. David Howells asked whether netfslib should be doing the
back-off [1], and objected to sleeping inside the response callback
because that runs in the cifsd thread and would stall the socket [2].
The sleep was therefore taken out of smb2_should_replay() and moved to
just before the replay in smb2_async_readv() and smb2_async_writev()
[3]. Setting the flag here preserves that arrangement: the sleep still
happens at the top of the reissued request, not in a callback.

Set rdata->replay here, matching smb2_async_writev().

Fixes: 2c1238a7477a ("cifs: make retry logic in read/write path consistent with other paths")
Link: https://lore.kernel.org/all/1652858.1769038134@warthog.procyon.org.uk/ [1]
Link: https://lore.kernel.org/all/1653031.1769038583@warthog.procyon.org.uk/ [2]
Link: https://lore.kernel.org/all/CANT5p=pXP3+CywpmK-on2uTvxO3S=31_B85_UDR7RoK1dQVtMA@mail.gmail.com/ [3]
Assisted-by: Codex:gpt-5.5
Assisted-by: Claude:claude-opus-5
Signed-off-by: Christopher Lusk <clusk@northecho.dev>
---

Tooling and testing, per Documentation/process/generated-content.rst:

- The site was surfaced by a static audit sweeping recent merge windows
  for state and contract defects, then resolved by reading the two
  paths and smb2_should_replay() directly. The audit had left it
  undecided because it framed the question as whether a synchronous
  send failure leaves transmission ambiguous. That question governs
  only the smb2_set_replay() half; the discarded back-off does not
  depend on it.
- The patch and changelog were drafted with LLM assistance, see the
  Assisted-by trailers, and reviewed line by line by me. I am
  responsible for all of it.
- The behavioural description above is derived from reading the code,
  not from measurement. I have not observed the retry timing against a
  live server.
- Deliberately not marked for stable. The change looks correct to me on
  a reading of the two paths, but I have no user report and no measured
  impact, and that seemed too thin a basis to ask for a backport. If
  you think it warrants one, please add the tag.
- Testing: compile-tested only. x86_64, CONFIG_CIFS=m, gcc 15.2.1,
  W=1, clean. Base is cifs-2.6 for-next fa724e235cfd ("cifs: add
  fscache_resize_cookie() to cifs_setsize()"). I have no SMB server
  test rig here, so the retry pacing change is not verified at runtime
  and a test from someone with one would be welcome.

One question I could not settle, raised separately because I did not
want to put it in the changelog without evidence:

smb2_should_replay() short-circuits as

	if (tcon->retry || (*pretries)++ < tcon->ses->server->retrans)

so on a hard mount the retry counter is never incremented and the
function always returns true. netfs does not bound the loop either;
subreq->retry_count is incremented in fs/netfs/read_retry.c but never
compared against anything. That suggests a persistent replayable send
error on a hard mount could retry without a bound, which this patch
would at least pace rather than fix. I may well be missing a
terminating condition elsewhere, for example adjust_credits() blocking
in cifs_issue_read() or the reconnect path breaking the loop, so I have
not made any claim about it above. For what it is worth, the ->retries
counter was described on-list as tracking client retransmissions "when
soft mounts are used", which is at least consistent with the hard-mount
path being unbounded by design:
https://lore.kernel.org/all/CANT5p=oL+tP5_SFNRabROCqDMjriXj5osnyyAjrMeq6BiJcr1Q@mail.gmail.com/

I read the full review history of 2c1238a7477a (v1 through v4) before
sending. The read/write asymmetry in the send-error blocks was not
raised by any reviewer or bot at the time, so as far as I can tell this
is an oversight rather than a deliberate choice.
 fs/smb/client/smb2pdu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 4ce165e40657..06ab6eeb4f13 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4885,6 +4885,7 @@ smb2_async_readv(struct cifs_io_subrequest *rdata)
 	    smb2_should_replay(tcon,
 			       &rdata->retries,
 			       &rdata->cur_sleep)) {
+		rdata->replay = true;
 		trace_netfs_sreq(&rdata->subreq, netfs_sreq_trace_io_retry_needed);
 		__set_bit(NETFS_SREQ_NEED_RETRY, &rdata->subreq.flags);
 	}
-- 
2.54.0