Re: [PATCH v3 09/12] sunrpc: Allow xprt->ops->timer method to sleep

[Trond Myklebust <trondmy-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 2335 bytes --]

On Wed, 2017-02-08 at 17:00 -0500, Chuck Lever wrote:
> The transport lock is needed to protect the xprt_adjust_cwnd() call
> in xs_udp_timer, but it is not necessary for accessing the
> rq_reply_bytes_recvd or tk_status fields. It is correct to sublimate
> the lock into UDP's xs_udp_timer method, where it is required.
> 
> The ->timer method has to take the transport lock if needed, but it
> can now sleep safely, or even call back into the RPC scheduler.
> 
> This is more a clean-up than a fix, but the "issue" was introduced
> by my transport switch patches back in 2005.
> 
> Fixes: 46c0ee8bc4ad ("RPC: separate xprt_timer implementations")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/sunrpc/xprt.c     |    2 --
>  net/sunrpc/xprtsock.c |    2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index 9a6be03..b530a28 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -897,13 +897,11 @@ static void xprt_timer(struct rpc_task *task)
>  		return;
>  	dprintk("RPC: %5u xprt_timer\n", task->tk_pid);
>  
> -	spin_lock_bh(&xprt->transport_lock);
>  	if (!req->rq_reply_bytes_recvd) {
>  		if (xprt->ops->timer)
>  			xprt->ops->timer(xprt, task);
>  	} else
>  		task->tk_status = 0;
> -	spin_unlock_bh(&xprt->transport_lock);
>  }
>  
>  /**
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index af392d9..d9bb644 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1734,7 +1734,9 @@ static void xs_udp_set_buffer_size(struct
> rpc_xprt *xprt, size_t sndsize, size_t
>   */
>  static void xs_udp_timer(struct rpc_xprt *xprt, struct rpc_task
> *task)
>  {
> +	spin_lock_bh(&xprt->transport_lock);
>  	xprt_adjust_cwnd(xprt, task, -ETIMEDOUT);
> +	spin_unlock_bh(&xprt->transport_lock);
>  }
>  
>  static unsigned short xs_get_random_port(void)
> 

Thanks! Good cleanup...

Trond

-- 



	
	


Trond Myklebust
Principal System Architect
4300 El Camino Real | Suite 100
Los Altos, CA  94022
W: 650-422-3800
C: 801-921-4583 
www.primarydata.com




N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš