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

Namjae Jeon <[email protected]>
Newsgroups org.kernel.vger.linux-cifs,dev.linux.lists.netfs,org.kernel.vger.linux-kernel
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!
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.