svn commit: r301601 - projects/vnet/sys/netinet

[Marko Zec]

On Wed, 8 Jun 2016 11:24:01 +0000
"Bjoern A. Zeeb" <bz at freebsd.org> wrote:

> Author: bz
> Date: Wed Jun  8 11:24:01 2016
> New Revision: 301601
> URL: https://svnweb.freebsd.org/changeset/base/301601
> 
> Log:
>   Replace the DELAY with a more sophisticated check to make sure
>   the inps are all gone before doing the cleanup.  We might decide
>   to do a partial cleanup before polling, but for now bail on the
>   safe side.
>   
>   Also use pause instead of DELAY [1].

This is a welcome improvement!  However, unconditionally pausing for
100 ms migt be an overpessimization here.  Perhaps the pause could
be moved after reading V_tcbinfo.ipi_count, in order to pause
conditionally only on non-zero reads, which should almost never happen
in practice, as the DELAY() hack was already deemed a safety belt in
the first place.  And even if / when the pause would be triggered, it
would be probably enough to sleep for 10 ms or so - the removed DELAY()
actually lasted for only a single clock tick.

Marko


>   Suggested by:	rwatson [1]
>   Sponsored by:	The FreeBSD Foundation
> 
> Modified:
>   projects/vnet/sys/netinet/tcp_subr.c
> 
> Modified: projects/vnet/sys/netinet/tcp_subr.c
> ==============================================================================
> --- projects/vnet/sys/netinet/tcp_subr.c	Wed Jun  8 11:18:49
> 2016	(r301600) +++ projects/vnet/sys/netinet/tcp_subr.c
> Wed Jun  8 11:24:01 2016	(r301601) @@ -731,18 +731,19 @@
> tcp_init(void) static void
>  tcp_destroy(void *unused __unused)
>  {
> -	int error;
> +	int error, n;
>  
>  	/*
>  	 * All our processes are gone, all our sockets should be
> cleaned
>  	 * up, which means, we should be past the tcp_discardcb()
> calls.
> -	 * Sleep to let all tcpcb timers really disappear and then
> cleanup.
> -	 * Timewait will cleanup its queue and will be ready to go.
> -	 * XXX-BZ In theory a few ticks should be good enough to
> make sure
> -	 * the timers are all really gone.  We should see if we
> could use a
> -	 * better metric here and, e.g., check a tcbcb count as an
> optimization?
> +	 * Sleep to let all tcpcb timers really disappear and
> cleanup. */
> -	DELAY(1000000 / hz);
> +	do {
> +		pause("tcpdes", hz/10);
> +		INP_LIST_RLOCK(&V_tcbinfo);
> +		n = V_tcbinfo.ipi_count;
> +		INP_LIST_RUNLOCK(&V_tcbinfo);
> +	} while (n != 0);
>  	tcp_hc_destroy();
>  	syncache_destroy();
>  	tcp_tw_destroy();
>