svn commit: r302099 - head/sys/netinet

[John Baldwin]

On Thursday, June 23, 2016 10:17:57 AM Marko Zec wrote:
> On Thu, 23 Jun 2016 00:34:03 +0000
> "Bjoern A. Zeeb" <bz at freebsd.org> wrote:
> 
> > Author: bz
> > Date: Thu Jun 23 00:34:03 2016
> > New Revision: 302099
> > URL: https://svnweb.freebsd.org/changeset/base/302099
> > 
> > Log:
> >   Check the V_tcbinfo.ipi_count to hit 0 before doing the full TCP
> > cleanup. That way timers can finish cleanly and we do not gamble with
> > a DELAY(). 
> >   Reviewed by:		gnn, jtl
> >   Approved by:		re (gjb)
> >   Obtained from:		projects/vnet
> >   MFC after:		2 weeks
> >   Sponsored by:		The FreeBSD Foundation
> >   Differential Revision:	https://reviews.freebsd.org/D6923
> 
> As much as this change is welcome, it unnecesarily introduces a
> mandatory 100 ms delay on each vnet teardown, which I already pointed
> out in a comment to r301601 two weeks ago, which remained unanswered,
> along with the question why a delay of 100 ms was introduced here, when
> before r302099 the delay was only a single clock tick?  And furthermore
> the delay computation expresion here is not style(9) compliant...
> 
> Hence, please rectify the above objections, perhaps by something like:
> 
> ===================================================================
> --- tcp_subr.c  (revision 302126)
> +++ tcp_subr.c  (working copy)
> @@ -739,10 +739,11 @@
>          * Sleep to let all tcpcb timers really disappear and cleanup.
>          */
>         do {
> -               pause("tcpdes", hz/10);
>                 INP_LIST_RLOCK(&V_tcbinfo);
>                 n = V_tcbinfo.ipi_count;
>                 INP_LIST_RUNLOCK(&V_tcbinfo);
> +               if (n != 0)
> +                       pause("tcpdes", hz / 100);
>         } while (n != 0);
>         tcp_hc_destroy();
>         syncache_destroy();

I would suggest avoiding the duplicate test by using a break:

	for (;;) {
		/* fetch 'n' */
		if (n == 0)
			break;
		pause(...);
	}

-- 
John Baldwin