Re: [PATCH v3 5/5] sunrpc: turn swapper_enable/disable functions into rpc_xprt_ops

[Chuck Lever <chuck.lever@oracle.com>]

On Jun 3, 2015, at 4:14 PM, Jeff Layton <jlayton@poochiereds.net> wrote:

> RDMA xprts don't have a sock_xprt, but an rdma_xprt, so the
> xs_swapper_enable/disable functions will likely oops when fed an RDMA
> xprt. Turn these functions into rpc_xprt_ops so that that doesn't
> occur. For now the RDMA versions are no-ops that just return -EINVAL
> on an attempt to swapon.
> 
> Cc: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>

Thanks, Jeff, that works for me.

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>


> ---
> include/linux/sunrpc/xprt.h     | 16 ++++++++++++++--
> net/sunrpc/clnt.c               |  4 ++--
> net/sunrpc/xprtrdma/transport.c | 15 ++++++++++++++-
> net/sunrpc/xprtsock.c           | 31 +++++++++++++++++++++++++------
> 4 files changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
> index 26b1624128ec..7eb58610eb94 100644
> --- a/include/linux/sunrpc/xprt.h
> +++ b/include/linux/sunrpc/xprt.h
> @@ -133,6 +133,8 @@ struct rpc_xprt_ops {
> 	void		(*close)(struct rpc_xprt *xprt);
> 	void		(*destroy)(struct rpc_xprt *xprt);
> 	void		(*print_stats)(struct rpc_xprt *xprt, struct seq_file *seq);
> +	int		(*enable_swap)(struct rpc_xprt *xprt);
> +	void		(*disable_swap)(struct rpc_xprt *xprt);
> };
> 
> /*
> @@ -327,6 +329,18 @@ static inline __be32 *xprt_skip_transport_header(struct rpc_xprt *xprt, __be32 *
> 	return p + xprt->tsh_size;
> }
> 
> +static inline int
> +xprt_enable_swap(struct rpc_xprt *xprt)
> +{
> +	return xprt->ops->enable_swap(xprt);
> +}
> +
> +static inline void
> +xprt_disable_swap(struct rpc_xprt *xprt)
> +{
> +	xprt->ops->disable_swap(xprt);
> +}
> +
> /*
>  * Transport switch helper functions
>  */
> @@ -345,8 +359,6 @@ void			xprt_release_rqst_cong(struct rpc_task *task);
> void			xprt_disconnect_done(struct rpc_xprt *xprt);
> void			xprt_force_disconnect(struct rpc_xprt *xprt);
> void			xprt_conditional_disconnect(struct rpc_xprt *xprt, unsigned int cookie);
> -int			xs_swapper_enable(struct rpc_xprt *xprt);
> -void			xs_swapper_disable(struct rpc_xprt *xprt);
> 
> bool			xprt_lock_connect(struct rpc_xprt *, struct rpc_task *, void *);
> void			xprt_unlock_connect(struct rpc_xprt *, void *);
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 804a75e71e84..60d1835edb26 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2492,7 +2492,7 @@ retry:
> 			goto retry;
> 		}
> 
> -		ret = xs_swapper_enable(xprt);
> +		ret = xprt_enable_swap(xprt);
> 		xprt_put(xprt);
> 	}
> 	return ret;
> @@ -2519,7 +2519,7 @@ retry:
> 			goto retry;
> 		}
> 
> -		xs_swapper_disable(xprt);
> +		xprt_disable_swap(xprt);
> 		xprt_put(xprt);
> 	}
> }
> diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
> index 54f23b1be986..ebf6fe759f0e 100644
> --- a/net/sunrpc/xprtrdma/transport.c
> +++ b/net/sunrpc/xprtrdma/transport.c
> @@ -682,6 +682,17 @@ static void xprt_rdma_print_stats(struct rpc_xprt *xprt, struct seq_file *seq)
> 	   r_xprt->rx_stats.bad_reply_count);
> }
> 
> +static int
> +xprt_rdma_enable_swap(struct rpc_xprt *xprt)
> +{
> +	return -EINVAL;
> +}
> +
> +static void
> +xprt_rdma_disable_swap(struct rpc_xprt *xprt)
> +{
> +}
> +
> /*
>  * Plumbing for rpc transport switch and kernel module
>  */
> @@ -700,7 +711,9 @@ static struct rpc_xprt_ops xprt_rdma_procs = {
> 	.send_request		= xprt_rdma_send_request,
> 	.close			= xprt_rdma_close,
> 	.destroy		= xprt_rdma_destroy,
> -	.print_stats		= xprt_rdma_print_stats
> +	.print_stats		= xprt_rdma_print_stats,
> +	.enable_swap		= xprt_rdma_enable_swap,
> +	.disable_swap		= xprt_rdma_disable_swap,
> };
> 
> static struct xprt_class xprt_rdma = {
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index e3596fe184a0..600194bfdbce 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1985,14 +1985,14 @@ static void xs_set_memalloc(struct rpc_xprt *xprt)
> }
> 
> /**
> - * xs_swapper_enable - Tag this transport as being used for swap.
> + * xs_enable_swap - Tag this transport as being used for swap.
>  * @xprt: transport to tag
>  *
>  * Take a reference to this transport on behalf of the rpc_clnt, and
>  * optionally mark it for swapping if it wasn't already.
>  */
> -int
> -xs_swapper_enable(struct rpc_xprt *xprt)
> +static int
> +xs_enable_swap(struct rpc_xprt *xprt)
> {
> 	struct sock_xprt *xs = container_of(xprt, struct sock_xprt, xprt);
> 
> @@ -2007,14 +2007,14 @@ xs_swapper_enable(struct rpc_xprt *xprt)
> }
> 
> /**
> - * xs_swapper_disable - Untag this transport as being used for swap.
> + * xs_disable_swap - Untag this transport as being used for swap.
>  * @xprt: transport to tag
>  *
>  * Drop a "swapper" reference to this xprt on behalf of the rpc_clnt. If the
>  * swapper refcount goes to 0, untag the socket as a memalloc socket.
>  */
> -void
> -xs_swapper_disable(struct rpc_xprt *xprt)
> +static void
> +xs_disable_swap(struct rpc_xprt *xprt)
> {
> 	struct sock_xprt *xs = container_of(xprt, struct sock_xprt, xprt);
> 
> @@ -2030,6 +2030,17 @@ xs_swapper_disable(struct rpc_xprt *xprt)
> static void xs_set_memalloc(struct rpc_xprt *xprt)
> {
> }
> +
> +static int
> +xs_enable_swap(struct rpc_xprt *xprt)
> +{
> +	return -EINVAL;
> +}
> +
> +static void
> +xs_disable_swap(struct rpc_xprt *xprt)
> +{
> +}
> #endif
> 
> static void xs_udp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
> @@ -2496,6 +2507,8 @@ static struct rpc_xprt_ops xs_local_ops = {
> 	.close			= xs_close,
> 	.destroy		= xs_destroy,
> 	.print_stats		= xs_local_print_stats,
> +	.enable_swap		= xs_enable_swap,
> +	.disable_swap		= xs_disable_swap,
> };
> 
> static struct rpc_xprt_ops xs_udp_ops = {
> @@ -2515,6 +2528,8 @@ static struct rpc_xprt_ops xs_udp_ops = {
> 	.close			= xs_close,
> 	.destroy		= xs_destroy,
> 	.print_stats		= xs_udp_print_stats,
> +	.enable_swap		= xs_enable_swap,
> +	.disable_swap		= xs_disable_swap,
> };
> 
> static struct rpc_xprt_ops xs_tcp_ops = {
> @@ -2531,6 +2546,8 @@ static struct rpc_xprt_ops xs_tcp_ops = {
> 	.close			= xs_tcp_shutdown,
> 	.destroy		= xs_destroy,
> 	.print_stats		= xs_tcp_print_stats,
> +	.enable_swap		= xs_enable_swap,
> +	.disable_swap		= xs_disable_swap,
> };
> 
> /*
> @@ -2548,6 +2565,8 @@ static struct rpc_xprt_ops bc_tcp_ops = {
> 	.close			= bc_close,
> 	.destroy		= bc_destroy,
> 	.print_stats		= xs_tcp_print_stats,
> +	.enable_swap		= xs_enable_swap,
> +	.disable_swap		= xs_disable_swap,
> };
> 
> static int xs_init_anyaddr(const int family, struct sockaddr *sap)
> -- 
> 2.4.2
> 

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>