2023-09-17 23:38:10

by Trond Myklebust

Subject: [PATCH] Revert "SUNRPC dont update timeout value on connection reset"

From: Trond Myklebust <[email protected]>

This reverts commit 88428cc4ae7abcc879295fbb19373dd76aad2bdd.

The problem this commit is intended to fix was comprehensively fixed
in commit 7de62bc09fe6 ("SUNRPC dont update timeout value on connection
reset").
Since then, this commit has been preventing the correct timeout of soft
mounted requests.

Cc: [email protected] # 5.9.x: 09252177d5f9: SUNRPC: Handle major timeout in xprt_adjust_timeout()
Cc: [email protected] # 5.9.x: 7de62bc09fe6: SUNRPC dont update timeout value on connection
reset
Cc: [email protected] # 5.9.x
Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/clnt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 5a7de7e55548..7f533c1041a4 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2476,8 +2476,7 @@ call_status(struct rpc_task *task)
goto out_exit;
}
task->tk_action = call_encode;
- if (status != -ECONNRESET && status != -ECONNABORTED)
- rpc_check_timeout(task);
+ rpc_check_timeout(task);
return;
out_exit:
rpc_call_rpcerror(task, status);
--
2.41.0

2023-09-18 03:30:58

by Olga Kornievskaia

Subject: Re: [PATCH] Revert "SUNRPC dont update timeout value on connection reset"

On Sun, Sep 17, 2023 at 7:38 PM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> This reverts commit 88428cc4ae7abcc879295fbb19373dd76aad2bdd.
>
> The problem this commit is intended to fix was comprehensively fixed
> in commit 7de62bc09fe6 ("SUNRPC dont update timeout value on connection
> reset").
> Since then, this commit has been preventing the correct timeout of soft
> mounted requests.

And if we revert this commit then we get back the problem that when
the server RSTs the connection between the timeouts then the client
waits double the time (instead of the correct time).

> Cc: [email protected] # 5.9.x: 09252177d5f9: SUNRPC: Handle major timeout in xprt_adjust_timeout()
> Cc: [email protected] # 5.9.x: 7de62bc09fe6: SUNRPC dont update timeout value on connection
> reset
> Cc: [email protected] # 5.9.x
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> net/sunrpc/clnt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 5a7de7e55548..7f533c1041a4 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2476,8 +2476,7 @@ call_status(struct rpc_task *task)
> goto out_exit;
> }
> task->tk_action = call_encode;
> - if (status != -ECONNRESET && status != -ECONNABORTED)
> - rpc_check_timeout(task);
> + rpc_check_timeout(task);
> return;
> out_exit:
> rpc_call_rpcerror(task, status);
> --
> 2.41.0
>

2023-09-18 21:49:06

by Trond Myklebust

Subject: Re: [PATCH] Revert "SUNRPC dont update timeout value on connection reset"

On Sun, 2023-09-17 at 22:55 -0400, Olga Kornievskaia wrote:
> On Sun, Sep 17, 2023 at 7:38 PM <[email protected]> wrote:
> >
> > From: Trond Myklebust <[email protected]>
> >
> > This reverts commit 88428cc4ae7abcc879295fbb19373dd76aad2bdd.
> >
> > The problem this commit is intended to fix was comprehensively
> > fixed
> > in commit 7de62bc09fe6 ("SUNRPC dont update timeout value on
> > connection
> > reset").
> > Since then, this commit has been preventing the correct timeout of
> > soft
> > mounted requests.
>
> And if we revert this commit then we get back the problem that when
> the server RSTs the connection between the timeouts then the client
> waits double the time (instead of the correct time).

No, we don't. That's what commit 7de62bc09fe6 (mentioned above) fixes.

>
> > Cc: [email protected] # 5.9.x: 09252177d5f9: SUNRPC: Handle
> > major timeout in xprt_adjust_timeout()
> > Cc: [email protected] # 5.9.x: 7de62bc09fe6: SUNRPC dont
> > update timeout value on connection
> > reset
> > Cc: [email protected] # 5.9.x
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> >  net/sunrpc/clnt.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> > index 5a7de7e55548..7f533c1041a4 100644
> > --- a/net/sunrpc/clnt.c
> > +++ b/net/sunrpc/clnt.c
> > @@ -2476,8 +2476,7 @@ call_status(struct rpc_task *task)
> >                 goto out_exit;
> >         }
> >         task->tk_action = call_encode;
> > -       if (status != -ECONNRESET && status != -ECONNABORTED)
> > -               rpc_check_timeout(task);
> > +       rpc_check_timeout(task);
> >         return;
> >  out_exit:
> >         rpc_call_rpcerror(task, status);
> > --
> > 2.41.0
> >

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]