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