From owner-freebsd-virtualization@FreeBSD.ORG Mon Sep 8 14:29:15 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 E10E1106567D; Mon, 8 Sep 2008 14:29:15 +0000 (UTC) (envelope-from brooks@lor.one-eyed-alien.net) Received: from lor.one-eyed-alien.net (cl-162.ewr-01.us.sixxs.net [IPv6:2001:4830:1200:a1::2]) by mx1.freebsd.org (Postfix) with ESMTP id 606268FC17; Mon, 8 Sep 2008 14:29:15 +0000 (UTC) (envelope-from brooks@lor.one-eyed-alien.net) Received: from lor.one-eyed-alien.net (localhost [127.0.0.1]) by lor.one-eyed-alien.net (8.14.3/8.14.2) with ESMTP id m88EU4aQ021138; Mon, 8 Sep 2008 09:30:04 -0500 (CDT) (envelope-from brooks@lor.one-eyed-alien.net) Received: (from brooks@localhost) by lor.one-eyed-alien.net (8.14.3/8.14.3/Submit) id m88EU4im021137; Mon, 8 Sep 2008 09:30:04 -0500 (CDT) (envelope-from brooks) Date: Mon, 8 Sep 2008 09:30:04 -0500 From: Brooks Davis To: Julian Elischer Message-ID: <20080908143004.GC20793@lor.one-eyed-alien.net> References: <48C3743D.9090503@elischer.org> <200809071641.11333.zec@freebsd.org> <48C3F4E7.6030507@elischer.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="s9fJI615cBHmzTOP" Content-Disposition: inline In-Reply-To: <48C3F4E7.6030507@elischer.org> User-Agent: Mutt/1.5.17 (2007-11-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-3.0 (lor.one-eyed-alien.net [127.0.0.1]); Mon, 08 Sep 2008 09:30:04 -0500 (CDT) Cc: Marko Zec , 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: Mon, 08 Sep 2008 14:29:16 -0000 --s9fJI615cBHmzTOP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Sep 07, 2008 at 08:36:07AM -0700, Julian Elischer wrote: > Marko Zec wrote: >> On Sunday 07 September 2008 08:27:09 Julian Elischer wrote: >>> trying to replace VNET_ITERLOOP_{BEGIN,END} >>>=20 >>>=20 >>>=20 >>> looking at an example: >>>=20 >>> static void >>> if_slowtimo(void *arg) >>> { >>> struct ifnet *ifp; >>>=20 >>> IFNET_RLOCK(); >>> VNET_ITERLOOP_BEGIN(); >>> INIT_VNET_NET(curvnet); >>> TAILQ_FOREACH(ifp, &V_ifnet, if_link) { >>> if (ifp->if_timer =3D=3D 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); >>> } >>>=20 >>>=20 >>> If we expand this out we get: (reindented for readability) >>>=20 >>> static void >>> if_slowtimo(void *arg) >>> { >>> struct ifnet *ifp; >>>=20 >>> IFNET_RLOCK(); >>> struct vnet *vnet_iter; >>> VNET_LIST_REF(); >>> LIST_FOREACH(vnet_iter, &vnet_head, vnet_le) { >>> struct vnet *saved_vnet =3D curvnet; >>> curvnet =3D vnet_iter; >>> INIT_VNET_NET(curvnet); >>> TAILQ_FOREACH(ifp, &V_ifnet, if_link) { >>> if (ifp->if_timer =3D=3D 0 || --ifp->if_timer) >>> continue; >>> if (ifp->if_watchdog) >>> (*ifp->if_watchdog)(ifp); >>> } >>> curvnet =3D saved_vnet; >>> } >>> VNET_LIST_UNREF(); >>> IFNET_RUNLOCK(); >>> timeout(if_slowtimo, (void *)0, hz / IFNET_SLOWHZ); >>> } >>>=20 >>> now several things leap out here.. >>> (like, declaring variables in mid block) >>> using different macros to try do this cleanly might lead to: >>>=20 >>>=20 >>>=20 >>> static void >>> if_slowtimo(void *arg) >>> { >>> struct ifnet *ifp; >>> VNET_DECL(vnet_iter); >>> VNET_DECL(saved_vnet); >>>=20 >>> IFNET_RLOCK(); >>> CURVNET_SAVE(saved_vnet); >>> VNET_LIST_REF(); >>> FOREACH_VNET(vnet_iter) { >>>=20 >>> CURVNET_SET_QUIET(vnet_iter); >>> INIT_VNET_NET(vnet_iter); >>> TAILQ_FOREACH(ifp, &V_ifnet, if_link) { >>> if (ifp->if_timer =3D=3D 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); >>> } >>>=20 >>> this adds a lot to the original.. >>>=20 >>> I could see: >>>=20 >>> using bigger macros, getting it back (size wise) to: >>>=20 >>> static void >>> if_slowtimo(void *arg) >>> { >>> struct ifnet *ifp; >>> VNET_ITERATOR_DECL(vnet_iter, saved_vnet); >>>=20 >>> IFNET_RLOCK(); >>> FOREACH_VNET(vnet_iter, saved_vnet) { >>> VNET_SWITCHTO(vnet_iter); >>> TAILQ_FOREACH(ifp, &V_ifnet, if_link) { >>> if (ifp->if_timer =3D=3D 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); >>> } >>>=20 >>> still bigger than the original macros though arguably more "C-like" >>>=20 >>> anyone have better ways to express this? >>>=20 >>> Brook, robert, bz? does this look better? >>=20 >> I don't think we need an explicit CURVNET_SAVE() in most of the places= =20 >> that iterate over the entire vnet list, because in functions where=20 >> iteration takes place there's typically no vnet context set on entry. >>=20 >> So perhaps a replacement for current VNET_ITERLOOP_BEGIN /=20 >> VNET_ITERLOOP_END kludge looking like this could suffice: >>=20 >> 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() */ >=20 > also. it it enough to just have a reference? is there a (rw)lock for > the vimage list? This really needs to be changed to the appropriate real lock (sx, rw, or rm) rather than a hand rolled version. Unless another syncronization mechanism does not provide what we need, we should always error on the side of using the existing primative. -- Brooks >>=20 >> This could provide more insight into what's going on in the loop, while= =20 >> still allowing for all the macros to vanish in nooptions VIMAGE builds. >>=20 >> Marko >> _______________________________________________ >> freebsd-virtualization@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization >> To unsubscribe, send any mail to "freebsd-virtualization-unsubscribe@fre= ebsd.org" >=20 > _______________________________________________ > freebsd-virtualization@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization > To unsubscribe, send any mail to "freebsd-virtualization-unsubscribe@free= bsd.org" --s9fJI615cBHmzTOP Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.8 (FreeBSD) iD8DBQFIxTbsXY6L6fI4GtQRAlJVAJ0SmIc+gJW7AAWwDPpynVUrVumgowCfZAMD RHslUHcfiCUH7kkTzz/uD8M= =DPo3 -----END PGP SIGNATURE----- --s9fJI615cBHmzTOP--