Re: [PATCH] smb: client: set replay flag on the read send-error retry path
Namjae Jeon <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.cifs,gmane.network.samba.internals |
|---|---|
| Message-ID | <CAKYAXd_hupeuYgwc+ndHtytwxqDcXhpuAOiHkqkXv44fFTojhg@mail.gmail.com> |
On Thu, Jul 30, 2026 at 4:24 AM Christopher Lusk <[email protected]> wrote: > > 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/[email protected]/ [1] > Link: https://lore.kernel.org/all/[email protected]/ [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 <[email protected]> I will apply it to #for-next. Thanks!