From owner-freebsd-virtualization@FreeBSD.ORG Sun Sep 7 15:36:01 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 CB957106567D for ; Sun, 7 Sep 2008 15:36:01 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outG.internet-mail-service.net (outg.internet-mail-service.net [216.240.47.230]) by mx1.freebsd.org (Postfix) with ESMTP id 99C398FC26 for ; Sun, 7 Sep 2008 15:36:01 +0000 (UTC) (envelope-from julian@elischer.org) Received: from idiom.com (mx0.idiom.com [216.240.32.160]) by out.internet-mail-service.net (Postfix) with ESMTP id E237A2433; Sun, 7 Sep 2008 08:36:03 -0700 (PDT) Received: from julian-mac.elischer.org (localhost [127.0.0.1]) by idiom.com (Postfix) with ESMTP id 8EF4F2D6004; Sun, 7 Sep 2008 08:36:00 -0700 (PDT) Message-ID: <48C3F4E7.6030507@elischer.org> Date: Sun, 07 Sep 2008 08:36:07 -0700 From: Julian Elischer User-Agent: Thunderbird 2.0.0.16 (Macintosh/20080707) MIME-Version: 1.0 To: Marko Zec References: <48C3743D.9090503@elischer.org> <200809071641.11333.zec@freebsd.org> In-Reply-To: <200809071641.11333.zec@freebsd.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 15:36:01 -0000 Marko Zec wrote: > 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() */ also. it it enough to just have a reference? is there a (rw)lock for the vimage list? > > 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 > _______________________________________________ > freebsd-virtualization@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization > To unsubscribe, send any mail to "freebsd-virtualization-unsubscribe@freebsd.org"