Date: Sun, 7 Sep 2008 16:41:11 +0200 From: Marko Zec <zec@freebsd.org> To: Julian Elischer <julian@elischer.org> Cc: FreeBSD virtualization mailing list <freebsd-virtualization@freebsd.org> Subject: Re: FOREACH_VNET... Message-ID: <200809071641.11333.zec@freebsd.org> In-Reply-To: <48C3743D.9090503@elischer.org>
index | next in thread | previous in thread | raw e-mail
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
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809071641.11333.zec>