On 08/05/2016 12:41 PM, Adamson, Andy wrote:
>
>> On Aug 4, 2016, at 3:26 PM, Schumaker, Anna <Anna.Schumaker@xxxxxxxxxx> wrote:
>>
>> Hi Andy,
>>
>> On 07/27/2016 02:43 PM, andros@xxxxxxxxxx wrote:
>>> From: Andy Adamson <andros@xxxxxxxxxx>
>>>
>>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>>> ---
>>> include/linux/sunrpc/clnt.h | 2 ++
>>> net/sunrpc/clnt.c | 18 ++++++++++++++++++
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
>>> index 99410bb..ebc83df 100644
>>> --- a/include/linux/sunrpc/clnt.h
>>> +++ b/include/linux/sunrpc/clnt.h
>>> @@ -189,6 +189,8 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>>> struct rpc_xprt_switch *xps,
>>> struct rpc_xprt *xprt,
>>> void *dummy);
>>> +int rpc_clnt_test_xprt(struct rpc_clnt *clnt,
>>> + struct rpc_xprt *xprt);
>>> int rpc_clnt_add_xprt(struct rpc_clnt *, struct xprt_create *,
>>> int (*setup)(struct rpc_clnt *,
>>> struct rpc_xprt_switch *,
>>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>>> index 459f9b1..822060f 100644
>>> --- a/net/sunrpc/clnt.c
>>> +++ b/net/sunrpc/clnt.c
>>> @@ -2614,6 +2614,24 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt *clnt,
>>> }
>>> EXPORT_SYMBOL_GPL(rpc_clnt_test_and_add_xprt);
>>>
>>> +int rpc_clnt_test_xprt(struct rpc_clnt *clnt, struct rpc_xprt *xprt)
>>
>> Looks like rpc_clnt_test_and_add_xprt() runs the same testing code. Can you swap the order of these functions in clnt.c and then have test_and_add_xprt() call test_xprt()?
>
> rpc_clnt_test_xprt uses a SYNC null call while test_and_add_xprt uses an ASYNC call. Futhermore, while both test_xprt and test_and_add_xprt return an error if the rpc_call_null_helper task cannot be allocated (ERR_PTR(task) in rpc_new_task), test_and_add_xprt ignores the tk_status and returns 1 while test_xprt returns the tk_status.
Ah, I missed that one is sync and the other isn't. Thanks for pointing that out! I still wish there was a common function they both could tall to handle the cred lookup, but maybe that's not as easy as it seems.
Anna
>
> For these reasons I think it is cleaner and easier to read to keep them separate functions.
>
> —>Andy
> :
>>
>> Thanks,
>> Anna
>>
>>> +{
>>> + struct rpc_cred *cred;
>>> + struct rpc_task *task;
>>> + int status;
>>> +
>>> + cred = authnull_ops.lookup_cred(NULL, NULL, 0);
>>> + task = rpc_call_null_helper(clnt, xprt, cred,
>>> + RPC_TASK_SOFT | RPC_TASK_SOFTCONN, NULL, NULL);
>>> + put_rpccred(cred);
>>> + if (IS_ERR(task))
>>> + return PTR_ERR(task);
>>> + status = task->tk_status;
>>> + rpc_put_task(task);
>>> + return status;
>>> +}
>>> +EXPORT_SYMBOL_GPL(rpc_clnt_test_xprt);
>>> +
>>> /**
>>> * rpc_clnt_add_xprt - Add a new transport to a rpc_clnt
>>> * @clnt: pointer to struct rpc_clnt
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
