2021-07-19 14:54:52

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH v2 0/6] Ensure RPC_TASK_NORTO is disabled for select operations

This is a set of patches I've been toying with to get better
responsiveness from a client when a transport remains connected but
the server is not returning RPC replies.

The approach I've taken is to disable RPC_TASK_NO_RETRANS_TIMEOUT
for a few particular operations to enable them to time out even
though the connection is still operational. It could be
appropriate to take this approach for any idempotent operation
that cannot be killed with a ^C.

Changes since RFC:
- Dropped changes to async lease renewal and DESTROY_SESSION|CLIENTID
- Cleaned up some tracepoint issues I found along the way

---

Chuck Lever (6):
SUNRPC: Refactor rpc_ping()
SUNRPC: Unset RPC_TASK_NO_RETRANS_TIMEOUT for NULL RPCs
SUNRPC: Remove unneeded TRACE_DEFINE_ENUMs
SUNRPC: Update trace flags
SUNRPC: xprt_retransmit() displays the the NULL procedure incorrectly
SUNRPC: Record timeout value in xprt_retransmit tracepoint


include/trace/events/sunrpc.h | 51 ++++++++---------------------------
net/sunrpc/clnt.c | 33 ++++++++++++++++-------
2 files changed, 35 insertions(+), 49 deletions(-)

--
Chuck Lever


2021-07-19 14:54:54

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH v2 1/6] SUNRPC: Refactor rpc_ping()

Make it use the rpc_null_call_helper() so that it can share the
new rpc_call_ops structure to be introduced in the next patch.

Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/clnt.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 8b4de70e8ead..ca2000d8cf64 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2694,17 +2694,6 @@ static const struct rpc_procinfo rpcproc_null = {
.p_decode = rpcproc_decode_null,
};

-static int rpc_ping(struct rpc_clnt *clnt)
-{
- struct rpc_message msg = {
- .rpc_proc = &rpcproc_null,
- };
- int err;
- err = rpc_call_sync(clnt, &msg, RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
- RPC_TASK_NULLCREDS);
- return err;
-}
-
static
struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
struct rpc_xprt *xprt, struct rpc_cred *cred, int flags,
@@ -2733,6 +2722,19 @@ struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred, int
}
EXPORT_SYMBOL_GPL(rpc_call_null);

+static int rpc_ping(struct rpc_clnt *clnt)
+{
+ struct rpc_task *task;
+ int status;
+
+ task = rpc_call_null_helper(clnt, NULL, NULL, 0, NULL, NULL);
+ if (IS_ERR(task))
+ return PTR_ERR(task);
+ status = task->tk_status;
+ rpc_put_task(task);
+ return status;
+}
+
struct rpc_cb_add_xprt_calldata {
struct rpc_xprt_switch *xps;
struct rpc_xprt *xprt;


2021-08-09 22:40:35

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Ensure RPC_TASK_NORTO is disabled for select operations

Hi Chuck,

On Mon, Jul 19, 2021 at 10:55 AM Chuck Lever <[email protected]> wrote:
>
> This is a set of patches I've been toying with to get better
> responsiveness from a client when a transport remains connected but
> the server is not returning RPC replies.
>
> The approach I've taken is to disable RPC_TASK_NO_RETRANS_TIMEOUT
> for a few particular operations to enable them to time out even
> though the connection is still operational. It could be
> appropriate to take this approach for any idempotent operation
> that cannot be killed with a ^C.
>
> Changes since RFC:
> - Dropped changes to async lease renewal and DESTROY_SESSION|CLIENTID
> - Cleaned up some tracepoint issues I found along the way

Is this the latest version of these patches? If so I can include them
in my linux-next branch for 5.14.

Thanks,
Anna

>
> ---
>
> Chuck Lever (6):
> SUNRPC: Refactor rpc_ping()
> SUNRPC: Unset RPC_TASK_NO_RETRANS_TIMEOUT for NULL RPCs
> SUNRPC: Remove unneeded TRACE_DEFINE_ENUMs
> SUNRPC: Update trace flags
> SUNRPC: xprt_retransmit() displays the the NULL procedure incorrectly
> SUNRPC: Record timeout value in xprt_retransmit tracepoint
>
>
> include/trace/events/sunrpc.h | 51 ++++++++---------------------------
> net/sunrpc/clnt.c | 33 ++++++++++++++++-------
> 2 files changed, 35 insertions(+), 49 deletions(-)
>
> --
> Chuck Lever
>

2021-08-09 22:43:39

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Ensure RPC_TASK_NORTO is disabled for select operations



> On Aug 9, 2021, at 4:37 PM, Anna Schumaker <[email protected]> wrote:
>
> Hi Chuck,
>
> On Mon, Jul 19, 2021 at 10:55 AM Chuck Lever <[email protected]> wrote:
>>
>> This is a set of patches I've been toying with to get better
>> responsiveness from a client when a transport remains connected but
>> the server is not returning RPC replies.
>>
>> The approach I've taken is to disable RPC_TASK_NO_RETRANS_TIMEOUT
>> for a few particular operations to enable them to time out even
>> though the connection is still operational. It could be
>> appropriate to take this approach for any idempotent operation
>> that cannot be killed with a ^C.
>>
>> Changes since RFC:
>> - Dropped changes to async lease renewal and DESTROY_SESSION|CLIENTID
>> - Cleaned up some tracepoint issues I found along the way
>
> Is this the latest version of these patches? If so I can include them
> in my linux-next branch for 5.14.

AFAIR v2 is the latest, yes. Thanks!


> Thanks,
> Anna
>
>>
>> ---
>>
>> Chuck Lever (6):
>> SUNRPC: Refactor rpc_ping()
>> SUNRPC: Unset RPC_TASK_NO_RETRANS_TIMEOUT for NULL RPCs
>> SUNRPC: Remove unneeded TRACE_DEFINE_ENUMs
>> SUNRPC: Update trace flags
>> SUNRPC: xprt_retransmit() displays the the NULL procedure incorrectly
>> SUNRPC: Record timeout value in xprt_retransmit tracepoint
>>
>>
>> include/trace/events/sunrpc.h | 51 ++++++++---------------------------
>> net/sunrpc/clnt.c | 33 ++++++++++++++++-------
>> 2 files changed, 35 insertions(+), 49 deletions(-)
>>
>> --
>> Chuck Lever

--
Chuck Lever



2021-08-10 14:22:26

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Ensure RPC_TASK_NORTO is disabled for select operations

On Mon, Aug 9, 2021 at 7:00 PM Chuck Lever III <[email protected]> wrote:
>
>
>
> > On Aug 9, 2021, at 4:37 PM, Anna Schumaker <[email protected]> wrote:
> >
> > Hi Chuck,
> >
> > On Mon, Jul 19, 2021 at 10:55 AM Chuck Lever <[email protected]> wrote:
> >>
> >> This is a set of patches I've been toying with to get better
> >> responsiveness from a client when a transport remains connected but
> >> the server is not returning RPC replies.
> >>
> >> The approach I've taken is to disable RPC_TASK_NO_RETRANS_TIMEOUT
> >> for a few particular operations to enable them to time out even
> >> though the connection is still operational. It could be
> >> appropriate to take this approach for any idempotent operation
> >> that cannot be killed with a ^C.
> >>
> >> Changes since RFC:
> >> - Dropped changes to async lease renewal and DESTROY_SESSION|CLIENTID
> >> - Cleaned up some tracepoint issues I found along the way
> >
> > Is this the latest version of these patches? If so I can include them
> > in my linux-next branch for 5.14.
>
> AFAIR v2 is the latest, yes. Thanks!

Great, I have them applied!

Anna
>
>
> > Thanks,
> > Anna
> >
> >>
> >> ---
> >>
> >> Chuck Lever (6):
> >> SUNRPC: Refactor rpc_ping()
> >> SUNRPC: Unset RPC_TASK_NO_RETRANS_TIMEOUT for NULL RPCs
> >> SUNRPC: Remove unneeded TRACE_DEFINE_ENUMs
> >> SUNRPC: Update trace flags
> >> SUNRPC: xprt_retransmit() displays the the NULL procedure incorrectly
> >> SUNRPC: Record timeout value in xprt_retransmit tracepoint
> >>
> >>
> >> include/trace/events/sunrpc.h | 51 ++++++++---------------------------
> >> net/sunrpc/clnt.c | 33 ++++++++++++++++-------
> >> 2 files changed, 35 insertions(+), 49 deletions(-)
> >>
> >> --
> >> Chuck Lever
>
> --
> Chuck Lever
>
>
>