On Mon, 2022-06-20 at 11:23 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
>
> Iterate thru available transports in the xprt_switch for all
> trunkable transports offline and possibly remote them as well.
>
> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> ---
> include/linux/sunrpc/clnt.h | 1 +
> net/sunrpc/clnt.c | 42
> +++++++++++++++++++++++++++++++++++++
> 2 files changed, 43 insertions(+)
>
> diff --git a/include/linux/sunrpc/clnt.h
> b/include/linux/sunrpc/clnt.h
> index 90501404fa49..e74a0740603b 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -234,6 +234,7 @@
> int rpc_clnt_setup_test_and_add_xprt(struct rpc_clnt *,
> struct rpc_xprt_switch *,
> struct rpc_xprt *,
> void *);
> +void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *, void
> *);
>
> const char *rpc_proc_name(const struct rpc_task *task);
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index e2c6eca0271b..544b55a3aa20 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2999,6 +2999,48 @@ int rpc_clnt_add_xprt(struct rpc_clnt *clnt,
> }
> EXPORT_SYMBOL_GPL(rpc_clnt_add_xprt);
>
> +static int rpc_xprt_offline_destroy(struct rpc_clnt *clnt,
> + struct rpc_xprt *xprt,
> + void *data)
> +{
> + struct rpc_xprt *main_xprt;
> + struct rpc_xprt_switch *xps;
> + int err = 0;
> + int *offline_destroy = (int *)data;
> +
> + xprt_get(xprt);
> +
> + rcu_read_lock();
> + main_xprt = xprt_get(rcu_dereference(clnt->cl_xprt));
> + xps = xprt_switch_get(rcu_dereference(clnt-
> >cl_xpi.xpi_xpswitch));
> + err = rpc_cmp_addr_port((struct sockaddr *)&xprt->addr,
> + (struct sockaddr *)&main_xprt->addr);
> + rcu_read_unlock();
> + xprt_put(main_xprt);
> + if (err)
> + goto out;
> +
> + if (wait_on_bit_lock(&xprt->state, XPRT_LOCKED,
> TASK_KILLABLE)) {
> + err = -EINTR;
> + goto out;
> + }
> + xprt_set_offline_locked(xprt, xps);
> + if (*offline_destroy)
> + xprt_delete_locked(xprt, xps);
> +
> + xprt_release_write(xprt, NULL);
> +out:
> + xprt_put(xprt);
> + xprt_switch_put(xps);
> + return err;
> +}
> +
> +void rpc_clnt_manage_trunked_xprts(struct rpc_clnt *clnt, void
> *data)
> +{
> + rpc_clnt_iterate_for_each_xprt(clnt,
> rpc_xprt_offline_destroy, data);
> +}
> +EXPORT_SYMBOL_GPL(rpc_clnt_manage_trunked_xprts);
Why is this function taking a 'void *' argument when
rpc_xprt_offline_destroy() won't accept anything other than an 'int *'.
It would be much cleaner to have 2 separate functions, neither or which
need more than one argument. Then you can hide the pointer to the 'int'
in each function and avoid exposing it as part of the API.
In addition, a function like this that is intended for use by a
different layer needs a proper kerneldoc comment so that we know what
the API is for, and what it does.
> +
> struct connect_timeout_data {
> unsigned long connect_timeout;
> unsigned long reconnect_timeout;
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
