From owner-freebsd-virtualization@FreeBSD.ORG Sun Sep 7 14:41:38 2008 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 244AA10656C2 for ; Sun, 7 Sep 2008 14:41:38 +0000 (UTC) (envelope-from zec@freebsd.org) Received: from mail2.srv.carnet.hr (unknown [IPv6:2001:b68:e160:0:216:3eff:fe0f:f088]) by mx1.freebsd.org (Postfix) with ESMTP id 43DF78FC1D for ; Sun, 7 Sep 2008 14:41:37 +0000 (UTC) (envelope-from zec@freebsd.org) Received: from vipnet115-83.mobile.carnet.hr ([193.198.83.115]:60455) by mail2.srv.carnet.hr with esmtp (Exim 4.63) (envelope-from ) id 1KcLS7-0001dV-MQ; Sun, 07 Sep 2008 16:41:30 +0200 From: Marko Zec To: Julian Elischer Date: Sun, 7 Sep 2008 16:41:11 +0200 User-Agent: KMail/1.9.7 References: <48C3743D.9090503@elischer.org> In-Reply-To: <48C3743D.9090503@elischer.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200809071641.11333.zec@freebsd.org> X-SA-Exim-Connect-IP: 193.198.83.115 X-Spam-Checker-Version: SpamAssassin 3.2.3 (2007-08-08) on arneb.carnet.hr X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=ALL_TRUSTED,BAYES_50 autolearn=no version=3.2.3 X-SA-Exim-Version: 4.2.1 (built Tue, 09 Jan 2007 17:23:22 +0000) Cc: FreeBSD virtualization mailing list Subject: Re: FOREACH_VNET... X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Sep 2008 14:41:38 -0000 On Sunday 07 September 2008 08:27:09 Julian Elischer wrote: > trying to replace VNET_ITERLOOP_{BEGIN,END} > > > > looking at an example: > > static void > if_slowtimo(void *arg) > { > struct ifnet *ifp; > > IFNET_RLOCK(); > VNET_ITERLOOP_BEGIN(); > INIT_VNET_NET(curvnet); > TAILQ_FOREACH(ifp, &V_ifnet, if_link) { > if (ifp->if_timer == 0 || --ifp->if_timer) > continue; > if (ifp->if_watchdog) > (*ifp->if_watchdog)(ifp); > } > VNET_ITERLOOP_END(); > IFNET_RUNLOCK(); > timeout(if_slowtimo, (void *)0, hz / IFNET_SLOWHZ); > } > > > If we expand this out we get: (reindented for readability) > > static void > if_slowtimo(void *arg) > { > struct ifnet *ifp; > > IFNET_RLOCK(); > struct vnet *vnet_iter; > VNET_LIST_REF(); > LIST_FOREACH(vnet_iter, &vnet_head, vnet_le) { > struct vnet *saved_vnet = curvnet; > curvnet = vnet_iter; > INIT_VNET_NET(curvnet); > TAILQ_FOREACH(ifp, &V_ifnet, if_link) { > if (ifp->if_timer == 0 || --ifp->if_timer) > continue; > if (ifp->if_watchdog) > (*ifp->if_watchdog)(ifp); > } > curvnet = saved_vnet; > } > VNET_LIST_UNREF(); > IFNET_RUNLOCK(); > timeout(if_slowtimo, (void *)0, hz / IFNET_SLOWHZ); > } > > now several things leap out here.. > (like, declaring variables in mid block) > using different macros to try do this cleanly might lead to: > > > > static void > if_slowtimo(void *arg) > { > struct ifnet *ifp; > VNET_DECL(vnet_iter); > VNET_DECL(saved_vnet); > > IFNET_RLOCK(); > CURVNET_SAVE(saved_vnet); > VNET_LIST_REF(); > FOREACH_VNET(vnet_iter) { > > CURVNET_SET_QUIET(vnet_iter); > INIT_VNET_NET(vnet_iter); > TAILQ_FOREACH(ifp, &V_ifnet, if_link) { > if (ifp->if_timer == 0 || --ifp->if_timer) > continue; > if (ifp->if_watchdog) > (*ifp->if_watchdog)(ifp); > } > } > CURVNET_SET(vnet_hold); > VNET_LIST_UNREF(); > IFNET_RUNLOCK(); > timeout(if_slowtimo, (void *)0, hz / IFNET_SLOWHZ); > } > > this adds a lot to the original.. > > I could see: > > using bigger macros, getting it back (size wise) to: > > static void > if_slowtimo(void *arg) > { > struct ifnet *ifp; > VNET_ITERATOR_DECL(vnet_iter, saved_vnet); > > IFNET_RLOCK(); > FOREACH_VNET(vnet_iter, saved_vnet) { > VNET_SWITCHTO(vnet_iter); > TAILQ_FOREACH(ifp, &V_ifnet, if_link) { > if (ifp->if_timer == 0 || --ifp->if_timer) > continue; > if (ifp->if_watchdog) > (*ifp->if_watchdog)(ifp); > } > } > FOREACH_VNET_COMPLETE(saved_vnet); > IFNET_RUNLOCK(); > timeout(if_slowtimo, (void *)0, hz / IFNET_SLOWHZ); > } > > still bigger than the original macros though arguably more "C-like" > > anyone have better ways to express this? > > Brook, robert, bz? does this look better? I don't think we need an explicit CURVNET_SAVE() in most of the places that iterate over the entire vnet list, because in functions where iteration takes place there's typically no vnet context set on entry. So perhaps a replacement for current VNET_ITERLOOP_BEGIN / VNET_ITERLOOP_END kludge looking like this could suffice: VNET_LIST_REF(); /* essentialy a RLOCK() */ VNET_FOREACH(vnet_iter) { CURVNET_SET(vnet_iter); /* existing code chunk body, 1-tab indent prepended */ CURVNET_RESTORE(); } VNET_LIST_UNREF(); /* essentially a RUNLOCK() */ This could provide more insight into what's going on in the loop, while still allowing for all the macros to vanish in nooptions VIMAGE builds. Marko