From owner-svn-src-head@freebsd.org Wed May 18 20:04:08 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AB505B41B7E; Wed, 18 May 2016 20:04:08 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 5413D1F84; Wed, 18 May 2016 20:04:07 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-106-149-109.carlnfd1.nsw.optusnet.com.au (c122-106-149-109.carlnfd1.nsw.optusnet.com.au [122.106.149.109]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 49AC7D6622E; Thu, 19 May 2016 06:03:59 +1000 (AEST) Date: Thu, 19 May 2016 06:03:54 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ian Lepore cc: "Bjoern A. Zeeb" , Nathan Whitehorn , Justin Hibbits , Scott Long , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r300154 - head/sys/net In-Reply-To: <1463594263.1180.298.camel@freebsd.org> Message-ID: <20160519043606.G1061@besplex.bde.org> References: <201605181545.u4IFjCKD030751@repo.freebsd.org> <20160518105033.1eae7432@zhabar.knownspace> <759d085c-a485-c2ed-5d70-26eb4d27cdc2@freebsd.org> <1463592737.1180.294.camel@freebsd.org> <1EED3540-DCCF-40D2-91BA-CE0AA54C5D08@lists.zabbadoz.net> <1463594263.1180.298.camel@freebsd.org> MIME-Version: 1.0 X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=c+ZWOkJl c=1 sm=1 tr=0 a=R/f3m204ZbWUO/0rwPSMPw==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=nlC_4_pT8q9DhB4Ho9EA:9 a=6I5d2MoRAAAA:8 a=OUm4Lhyl49i_GLGZxNkA:9 a=45ClL6m2LaAA:10 a=IjZwj45LgO3ly-622nXo:22 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 May 2016 20:04:08 -0000 On Wed, 18 May 2016, Ian Lepore wrote: > On Wed, 2016-05-18 at 17:35 +0000, Bjoern A. Zeeb wrote: >>> On 18 May 2016, at 17:32 , Ian Lepore wrote: >>> >>> On Wed, 2016-05-18 at 10:14 -0700, Nathan Whitehorn wrote: >>> ... >>> It may be more complicated than that, though. armv6 can do 64-bit >>> atomics even tho it's 32-bit. armv4, also 32-bit, can do 64-bit >>> atomics in the kernel but not in userland. >>> >>> Maybe machine/atomic.h needs a #define that says whether 64-bit ops >>> are >>> available in the current compilation unit. (And likewise for other >>> bit >>> sizes if we have arches that have other limitations.) >> >> Question because I didn=A2t follow the details, but how was this solved >> for the COUNTERS framework? Using special code that pessimizes old machines on 32-bit arches especially= =2E For example, incrementing a 32-bit network counter used to take 1 inlined counter++ statement (as little as 1 instruction on i386, but it is a read-modify-write instruction and thus no faster than separate instructions, and if the counter is shared this is almost as slow as a locked instruction in some cases). This now takes an if_inc_counter() function call which takes 33 instruction= s altogether on i386 with certain nonstandard not very optimal CFLAGS, and 9 instructions on amd64. (COUNTER64() code is inlined, and the function call is a separate pessimization. It costs about half of the 9 instructions on amd64 and its instructions are relatively heavyweight.) This is when i386 has cmpxchgb. The single cmpxchg8b instruction is heavier weight than a 32-bit memory increment and using it takes lots of control logic. > iirc, each platform implements counters its own way, probably the wrong > way on all of them except x86. I think other arches just use compatiblity code which uses critical sections. This is not so bad. It might be faster than using cmpxchg8b depending on how fast critical_enter() and critical_exit() are. Unfortunately, they are not very fast. They are functions too, and on i386 critical_enter() takes 20+ instructions. critical_exit() takes more, and debugging is broken and caused a panic when I tried to trace through critical_exit(). That is so slow that hard-disabling interrupts is probably faster. Network drivers were mostly written under the assumption that they are running on a UP system and incrementing a counter is inline and fast, so they increment counters without worrying about the overhead for this. 33 instructions for 2 if_inc_counter()s per packet is about a 1% pessimization for bge on my slow hardware. > For some crazy reason the docs for COUNTERS say that it does not use > atomics. I have no idea why the docs for an API are dictating > implementation, but I suspect it's because atomics are more expensive > on x86 than other alternatives. So the arm code slavishly avoids using > atomics in COUNTERS even though doing so would be more effecient than > the current copied-from-x86 code. Other places just hard-code use of PCPU although that is also more complicated and uglier than counter++. Counters in PCPU only exist because full atomics are probably slower on all arches and much slower on most arches. Most 64-bit PCPU accesses on 32-bit arches are broken since they are not atomic even for 1 CPU. COUNTER64() is more careful to a fault. arm PCPU_INC() seems to be broken even for 32-bit accesses. In the non-SMP case, it just does pcpu->pc_ ## member++ and in the SMP case it does the same with a register pointer instead of a global. I wrote some alternative x86 implementations that are at least 20% faster than the cmpxchg8b method, but the best method is clearly to use only 32-bit low-level counters and add up the counters in a daemon. The daemon shouldn't run very often. There aren't many counters except i/o byte counters that want to wrap in 32 bits more than once per hour. Even 100 Gbps ethernet can only do 150 Mpps so it takes at least 30 seconds to wrap. Bruce From owner-svn-src-head@freebsd.org Wed May 18 20:06:47 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4A74BB41C2A; Wed, 18 May 2016 20:06:47 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 098041299; Wed, 18 May 2016 20:06:46 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u4IK6kTZ011779; Wed, 18 May 2016 20:06:46 GMT (envelope-from bz@FreeBSD.org) Received: (from bz@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u4IK6jP8011776; Wed, 18 May 2016 20:06:45 GMT (envelope-from bz@FreeBSD.org) Message-Id: <201605182006.u4IK6jP8011776@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: bz set sender to bz@FreeBSD.org using -f From: "Bjoern A. Zeeb" Date: Wed, 18 May 2016 20:06:45 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r300164 - head/sys/net X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 May 2016 20:06:47 -0000 Author: bz Date: Wed May 18 20:06:45 2016 New Revision: 300164 URL: https://svnweb.freebsd.org/changeset/base/300164 Log: Rather than having the if_vmove() code intermixed in the vnet_destroy() function in vnet.c move it to if.c where it logically belongs and put it under a VNET_SYSUNINIT() call. To not change the current behaviour make sure it runs first thing during teardown. In the future this will allow us more flexibility on changing the order on when we want to get rid of interfaces. Stop exporting if_vmove() and make it file static. Reviewed by: gnn Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D6438 Modified: head/sys/net/if.c head/sys/net/if_var.h head/sys/net/vnet.c Modified: head/sys/net/if.c ============================================================================== --- head/sys/net/if.c Wed May 18 19:59:05 2016 (r300163) +++ head/sys/net/if.c Wed May 18 20:06:45 2016 (r300164) @@ -182,6 +182,9 @@ static int if_getgroupmembers(struct ifg static void if_delgroups(struct ifnet *); static void if_attach_internal(struct ifnet *, int, struct if_clone *); static int if_detach_internal(struct ifnet *, int, struct if_clone **); +#ifdef VIMAGE +static void if_vmove(struct ifnet *, struct vnet *); +#endif #ifdef INET6 /* @@ -392,6 +395,20 @@ vnet_if_uninit(const void *unused __unus } VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_ORDER_FIRST, vnet_if_uninit, NULL); + +static void +vnet_if_return(const void *unused __unused) +{ + struct ifnet *ifp, *nifp; + + /* Return all inherited interfaces to their parent vnets. */ + TAILQ_FOREACH_SAFE(ifp, &V_ifnet, if_link, nifp) { + if (ifp->if_home_vnet != ifp->if_vnet) + if_vmove(ifp, ifp->if_home_vnet); + } +} +VNET_SYSUNINIT(vnet_if_return, SI_SUB_VNET_DONE, SI_ORDER_ANY, + vnet_if_return, NULL); #endif static void Modified: head/sys/net/if_var.h ============================================================================== --- head/sys/net/if_var.h Wed May 18 19:59:05 2016 (r300163) +++ head/sys/net/if_var.h Wed May 18 20:06:45 2016 (r300164) @@ -535,7 +535,6 @@ void if_dead(struct ifnet *); int if_delmulti(struct ifnet *, struct sockaddr *); void if_delmulti_ifma(struct ifmultiaddr *); void if_detach(struct ifnet *); -void if_vmove(struct ifnet *, struct vnet *); void if_purgeaddrs(struct ifnet *); void if_delallmulti(struct ifnet *); void if_down(struct ifnet *); Modified: head/sys/net/vnet.c ============================================================================== --- head/sys/net/vnet.c Wed May 18 19:59:05 2016 (r300163) +++ head/sys/net/vnet.c Wed May 18 20:06:45 2016 (r300164) @@ -269,7 +269,6 @@ vnet_alloc(void) void vnet_destroy(struct vnet *vnet) { - struct ifnet *ifp, *nifp; SDT_PROBE2(vnet, functions, vnet_destroy, entry, __LINE__, vnet); KASSERT(vnet->vnet_sockcnt == 0, @@ -280,13 +279,6 @@ vnet_destroy(struct vnet *vnet) VNET_LIST_WUNLOCK(); CURVNET_SET_QUIET(vnet); - - /* Return all inherited interfaces to their parent vnets. */ - TAILQ_FOREACH_SAFE(ifp, &V_ifnet, if_link, nifp) { - if (ifp->if_home_vnet != ifp->if_vnet) - if_vmove(ifp, ifp->if_home_vnet); - } - vnet_sysuninit(); CURVNET_RESTORE();