On Thu, 2020-04-23 at 15:43 -0400, Chuck Lever wrote:
> Prevent a (temporary) hang when shutting down an rpc_clnt that has
> used one or more GSS creds.
>
> I noticed that callers of rpc_call_null_helper() use a common set of
> flags, so I collected them all in that helper function.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> net/sunrpc/auth_gss/auth_gss.c | 2 +-
> net/sunrpc/clnt.c | 10 ++++------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/auth_gss.c
> b/net/sunrpc/auth_gss/auth_gss.c
> index ac5cac0dd24b..16cec9755b86 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -1285,7 +1285,7 @@ static void gss_pipe_free(struct gss_pipe *p)
> ctx->gc_proc = RPC_GSS_PROC_DESTROY;
>
> task = rpc_call_null(gss_auth->client, &new->gc_base,
> - RPC_TASK_ASYNC|RPC_TASK_SOFT);
> + RPC_TASK_ASYNC);
No. This means we can end with silently hanging clients that never go
away. Besides, this function is destroying the creds, so there should
be no problem with them timing out.
> if (!IS_ERR(task))
> rpc_put_task(task);
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 325a0858700f..ddc98b97c170 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2742,7 +2742,8 @@ struct rpc_task *rpc_call_null_helper(struct
> rpc_clnt *clnt,
> .rpc_op_cred = cred,
> .callback_ops = (ops != NULL) ? ops : &rpc_default_ops,
> .callback_data = data,
> - .flags = flags | RPC_TASK_NULLCREDS,
> + .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
> + RPC_TASK_NULLCREDS,
> };
>
> return rpc_run_task(&task_setup_data);
> @@ -2805,8 +2806,7 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt
> *clnt,
> goto success;
> }
>
> - task = rpc_call_null_helper(clnt, xprt, NULL,
> - RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC|
> RPC_TASK_NULLCREDS,
> + task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_ASYNC,
> &rpc_cb_add_xprt_call_ops, data);
> if (IS_ERR(task))
> return PTR_ERR(task);
> @@ -2850,9 +2850,7 @@ int rpc_clnt_setup_test_and_add_xprt(struct
> rpc_clnt *clnt,
> goto out_err;
>
> /* Test the connection */
> - task = rpc_call_null_helper(clnt, xprt, NULL,
> - RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
> RPC_TASK_NULLCREDS,
> - NULL, NULL);
> + task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
> if (IS_ERR(task)) {
> status = PTR_ERR(task);
> goto out_err;
>
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
