On Sun, Nov 23, 2014 at 13:39 +0100, Claudio Jeker wrote: > On Sun, Nov 23, 2014 at 02:10:24AM +0100, Mike Belopuhov wrote: > > Hi, > > > > This removes the system wide if_slowtimo timeout and lets every > > interface with a valid if_watchdog method register it's own. > > The rational is to get rid of the ifnet loop in the softclock > > context to avoid further complications with concurrent access > > to the ifnet list. This might also save some cpu cycles in the > > runtime if number of interfaces is large while only a fraction > > all of them have a watchdog routine. > > > > This has originally come up on NetBSD mailing lists with some > > rather complicated locking strategy. Masao Uebayashi has later > > proposed a similar solution. > > > > The diff was looked at and corrected by mpi@. I've tested the > > trunk and a system build as well. OK? > > > > Please do not use M_TEMP for this (this is not a temporary buffer).
There's nothing wrong with M_TEMP usage here. In fact all the other allocations are done with M_TEMP in if.c. > Maybe even remove the need to allocate the struct timeout (which requires > if_var.h to include sys/timeout.h and adds 24 / 40 bytes to ifnet even > when there is no watchdog used (like on vlan)). > That would be nice, however, I don't think right now it matters too much. Some userland apps are still using kvm to get their hands on the ifnet structure and I'm not 100% positive we should expose sys/timeout.h via if_var.h or teach them to include it on it's own. Historically it seems that if.h was overused in userland hence there are plenty of other things (list heads for example) that get allocated while they clearly should be part of the ifnet. A thorough clean up is long due. > In any case I think M_DEVBUF is the right type. > > We could also rename if_slowtimo to something more concrete like > if_watchdog_timeout but that's just a bikeshed. > We can, but as you said it's bikeshedding. "I'm not saving the world here" (c) Theo.
