diff mbox

[09/11] SUNRPC: Remove TCP socket linger code

Message ID 1423451262-84493-10-git-send-email-trond.myklebust@primarydata.com (mailing list archive)
State New, archived
Headers show

Commit Message

Trond Myklebust Feb. 9, 2015, 3:07 a.m. UTC 
Now that we no longer use the partial shutdown code when closing the
socket, we no longer need to worry about the TCP linger2 state.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 net/sunrpc/xprtsock.c | 35 -----------------------------------
 1 file changed, 35 deletions(-)

Comments

Schumaker, Anna Feb. 9, 2015, 3:31 p.m. UTC | #1 
Hi Trond,

On 02/08/2015 10:07 PM, Trond Myklebust wrote:
> Now that we no longer use the partial shutdown code when closing the
> socket, we no longer need to worry about the TCP linger2 state.

Can you also remove the "#define XS_TCP_LINGER_TO" and xs_tcp_fin_timeout from the top of xprtsock.c?  It looks like they don't have any users after this patch.

Thanks,
Anna

> 
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
>  net/sunrpc/xprtsock.c | 35 -----------------------------------
>  1 file changed, 35 deletions(-)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index b5dfb4f14ef9..ca45f1c1c36d 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1399,37 +1399,6 @@ out:
>  	read_unlock_bh(&sk->sk_callback_lock);
>  }
>  
> -/*
> - * Do the equivalent of linger/linger2 handling for dealing with
> - * broken servers that don't close the socket in a timely
> - * fashion
> - */
> -static void xs_tcp_schedule_linger_timeout(struct rpc_xprt *xprt,
> -		unsigned long timeout)
> -{
> -	struct sock_xprt *transport;
> -
> -	if (xprt_test_and_set_connecting(xprt))
> -		return;
> -	set_bit(XPRT_CONNECTION_ABORT, &xprt->state);
> -	transport = container_of(xprt, struct sock_xprt, xprt);
> -	queue_delayed_work(rpciod_workqueue, &transport->connect_worker,
> -			   timeout);
> -}
> -
> -static void xs_tcp_cancel_linger_timeout(struct rpc_xprt *xprt)
> -{
> -	struct sock_xprt *transport;
> -
> -	transport = container_of(xprt, struct sock_xprt, xprt);
> -
> -	if (!test_bit(XPRT_CONNECTION_ABORT, &xprt->state) ||
> -	    !cancel_delayed_work(&transport->connect_worker))
> -		return;
> -	clear_bit(XPRT_CONNECTION_ABORT, &xprt->state);
> -	xprt_clear_connecting(xprt);
> -}
> -
>  static void xs_sock_mark_closed(struct rpc_xprt *xprt)
>  {
>  	xs_sock_reset_connection_flags(xprt);
> @@ -1485,7 +1454,6 @@ static void xs_tcp_state_change(struct sock *sk)
>  		clear_bit(XPRT_CONNECTED, &xprt->state);
>  		clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
>  		smp_mb__after_atomic();
> -		xs_tcp_schedule_linger_timeout(xprt, xs_tcp_fin_timeout);
>  		break;
>  	case TCP_CLOSE_WAIT:
>  		/* The server initiated a shutdown of the socket */
> @@ -1502,13 +1470,11 @@ static void xs_tcp_state_change(struct sock *sk)
>  		break;
>  	case TCP_LAST_ACK:
>  		set_bit(XPRT_CLOSING, &xprt->state);
> -		xs_tcp_schedule_linger_timeout(xprt, xs_tcp_fin_timeout);
>  		smp_mb__before_atomic();
>  		clear_bit(XPRT_CONNECTED, &xprt->state);
>  		smp_mb__after_atomic();
>  		break;
>  	case TCP_CLOSE:
> -		xs_tcp_cancel_linger_timeout(xprt);
>  		xs_sock_mark_closed(xprt);
>  	}
>   out:
> @@ -2106,7 +2072,6 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
>  
>  		/* socket options */
>  		sock_reset_flag(sk, SOCK_LINGER);
> -		tcp_sk(sk)->linger2 = 0;
>  		tcp_sk(sk)->nonagle |= TCP_NAGLE_OFF;
>  
>  		xprt_clear_connected(xprt);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Feb. 9, 2015, 3:37 p.m. UTC | #2 
On Mon, Feb 9, 2015 at 10:31 AM, Anna Schumaker
<Anna.Schumaker@netapp.com> wrote:
>
> Hi Trond,
>
> On 02/08/2015 10:07 PM, Trond Myklebust wrote:
> > Now that we no longer use the partial shutdown code when closing the
> > socket, we no longer need to worry about the TCP linger2 state.
>
> Can you also remove the "#define XS_TCP_LINGER_TO" and xs_tcp_fin_timeout from the top of xprtsock.c?  It looks like they don't have any users after this patch.
>
> Thanks,
> Anna


Technically, those are part of the user ABI (being in the
xs_tunables_table), so we should probably proceed with care before
removing them.
Schumaker, Anna Feb. 9, 2015, 3:39 p.m. UTC | #3 
On 02/09/2015 10:37 AM, Trond Myklebust wrote:
> On Mon, Feb 9, 2015 at 10:31 AM, Anna Schumaker
> <Anna.Schumaker@netapp.com> wrote:
>>
>> Hi Trond,
>>
>> On 02/08/2015 10:07 PM, Trond Myklebust wrote:
>>> Now that we no longer use the partial shutdown code when closing the
>>> socket, we no longer need to worry about the TCP linger2 state.
>>
>> Can you also remove the "#define XS_TCP_LINGER_TO" and xs_tcp_fin_timeout from the top of xprtsock.c?  It looks like they don't have any users after this patch.
>>
>> Thanks,
>> Anna
> 
> 
> Technically, those are part of the user ABI (being in the
> xs_tunables_table), so we should probably proceed with care before
> removing them.

Okay.  Maybe at least move xs_tcp_fin_timeout so it's under CONFIG_SUNRPC_DEBUG to match when the tunables_table is defined?

> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Feb. 9, 2015, 3:59 p.m. UTC | #4 
On Mon, Feb 9, 2015 at 10:39 AM, Anna Schumaker
<Anna.Schumaker@netapp.com> wrote:
> On 02/09/2015 10:37 AM, Trond Myklebust wrote:
>> On Mon, Feb 9, 2015 at 10:31 AM, Anna Schumaker
>> <Anna.Schumaker@netapp.com> wrote:
>>>
>>> Hi Trond,
>>>
>>> On 02/08/2015 10:07 PM, Trond Myklebust wrote:
>>>> Now that we no longer use the partial shutdown code when closing the
>>>> socket, we no longer need to worry about the TCP linger2 state.
>>>
>>> Can you also remove the "#define XS_TCP_LINGER_TO" and xs_tcp_fin_timeout from the top of xprtsock.c?  It looks like they don't have any users after this patch.
>>>
>>> Thanks,
>>> Anna
>>
>>
>> Technically, those are part of the user ABI (being in the
>> xs_tunables_table), so we should probably proceed with care before
>> removing them.
>
> Okay.  Maybe at least move xs_tcp_fin_timeout so it's under CONFIG_SUNRPC_DEBUG to match when the tunables_table is defined?
>

ACK. That makes perfect sense...
diff mbox

Patch

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index b5dfb4f14ef9..ca45f1c1c36d 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1399,37 +1399,6 @@  out:
 	read_unlock_bh(&sk->sk_callback_lock);
 }
 
-/*
- * Do the equivalent of linger/linger2 handling for dealing with
- * broken servers that don't close the socket in a timely
- * fashion
- */
-static void xs_tcp_schedule_linger_timeout(struct rpc_xprt *xprt,
-		unsigned long timeout)
-{
-	struct sock_xprt *transport;
-
-	if (xprt_test_and_set_connecting(xprt))
-		return;
-	set_bit(XPRT_CONNECTION_ABORT, &xprt->state);
-	transport = container_of(xprt, struct sock_xprt, xprt);
-	queue_delayed_work(rpciod_workqueue, &transport->connect_worker,
-			   timeout);
-}
-
-static void xs_tcp_cancel_linger_timeout(struct rpc_xprt *xprt)
-{
-	struct sock_xprt *transport;
-
-	transport = container_of(xprt, struct sock_xprt, xprt);
-
-	if (!test_bit(XPRT_CONNECTION_ABORT, &xprt->state) ||
-	    !cancel_delayed_work(&transport->connect_worker))
-		return;
-	clear_bit(XPRT_CONNECTION_ABORT, &xprt->state);
-	xprt_clear_connecting(xprt);
-}
-
 static void xs_sock_mark_closed(struct rpc_xprt *xprt)
 {
 	xs_sock_reset_connection_flags(xprt);
@@ -1485,7 +1454,6 @@  static void xs_tcp_state_change(struct sock *sk)
 		clear_bit(XPRT_CONNECTED, &xprt->state);
 		clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
 		smp_mb__after_atomic();
-		xs_tcp_schedule_linger_timeout(xprt, xs_tcp_fin_timeout);
 		break;
 	case TCP_CLOSE_WAIT:
 		/* The server initiated a shutdown of the socket */
@@ -1502,13 +1470,11 @@  static void xs_tcp_state_change(struct sock *sk)
 		break;
 	case TCP_LAST_ACK:
 		set_bit(XPRT_CLOSING, &xprt->state);
-		xs_tcp_schedule_linger_timeout(xprt, xs_tcp_fin_timeout);
 		smp_mb__before_atomic();
 		clear_bit(XPRT_CONNECTED, &xprt->state);
 		smp_mb__after_atomic();
 		break;
 	case TCP_CLOSE:
-		xs_tcp_cancel_linger_timeout(xprt);
 		xs_sock_mark_closed(xprt);
 	}
  out:
@@ -2106,7 +2072,6 @@  static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
 
 		/* socket options */
 		sock_reset_flag(sk, SOCK_LINGER);
-		tcp_sk(sk)->linger2 = 0;
 		tcp_sk(sk)->nonagle |= TCP_NAGLE_OFF;
 
 		xprt_clear_connected(xprt);