Thread (23 messages) 23 messages, 5 authors, 2023-05-23

Re: [PATCH RFC 04/12] SUNRPC: Refactor rpc_call_null_helper()

From: Anna Schumaker <hidden>
Date: 2023-05-22 20:36:35

Hi Chuck,

On Tue, May 16, 2023 at 3:42 PM Chuck Lever [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Chuck Lever <chuck.lever@oracle.com>

I'm about to add a use case that does not want RPC_TASK_NULLCREDS.
Refactor rpc_call_null_helper() so that callers provide NULLCREDS
when they need it.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/clnt.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 4cdb539b5854..2dca0ae489ec 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2811,8 +2811,7 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,
                .rpc_op_cred = cred,
                .callback_ops = ops ?: &rpc_null_ops,
                .callback_data = data,
-               .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN |
-                        RPC_TASK_NULLCREDS,
+               .flags = flags | RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
        };

        return rpc_run_task(&task_setup_data);
@@ -2820,7 +2819,8 @@ struct rpc_task *rpc_call_null_helper(struct rpc_clnt *clnt,

 struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred, int flags)
 {
-       return rpc_call_null_helper(clnt, NULL, cred, flags, NULL, NULL);
+       return rpc_call_null_helper(clnt, NULL, cred, flags | RPC_TASK_NULLCREDS,
+                                   NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(rpc_call_null);
I think you missed updating rpc_ping() right below this function as
well, I'm unable to mount without the flag. Although I do wonder if it
would be easier to slightly rename rpc_call_null_helper(), and then
create a new rpc_call_null_helper() that appends the
RPC_TASK_NULLCREDS flag. Then we don't need to touch current callers,
and your new use case could call the renamed function.

What do you think?
Anna
quoted hunk ↗ jump to hunk
@@ -2920,12 +2920,13 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
                goto success;
        }

-       task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_ASYNC,
-                       &rpc_cb_add_xprt_call_ops, data);
+       task = rpc_call_null_helper(clnt, xprt, NULL,
+                                   RPC_TASK_ASYNC | RPC_TASK_NULLCREDS,
+                                   &rpc_cb_add_xprt_call_ops, data);
        if (IS_ERR(task))
                return PTR_ERR(task);
-
        data->xps->xps_nunique_destaddr_xprts++;
+
        rpc_put_task(task);
 success:
        return 1;
@@ -2940,7 +2941,8 @@ static int rpc_clnt_add_xprt_helper(struct rpc_clnt *clnt,
        int status = -EADDRINUSE;

        /* Test the connection */
-       task = rpc_call_null_helper(clnt, xprt, NULL, 0, NULL, NULL);
+       task = rpc_call_null_helper(clnt, xprt, NULL, RPC_TASK_NULLCREDS,
+                                   NULL, NULL);
        if (IS_ERR(task))
                return PTR_ERR(task);

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help