svn commit: r302099 - head/sys/netinet
Marko Zec
zec at fer.hr
Thu Jun 23 08:18:07 UTC 2016
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();
Thanks,
Marko
>
> Modified:
> head/sys/netinet/tcp_subr.c
>
> Modified: head/sys/netinet/tcp_subr.c
> ==============================================================================
> --- head/sys/netinet/tcp_subr.c Thu Jun 23 00:32:58
> 2016 (r302098) +++ head/sys/netinet/tcp_subr.c Thu Jun
> 23 00:34:03 2016 (r302099) @@ -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();
>
More information about the svn-src-all
mailing list