[PATCH] Remove if_watchdog use
John Baldwin
jhb at freebsd.org
Fri Nov 6 20:08:40 UTC 2009
I have a patchset that converts all the remaining users of if_watchdog to
using a private callout instead. In some cases the the driver already used a
private timer to drive a stats timer and I merely hooked into that timer. In
other cases a new callout needed to be added to the driver. Some drivers
even abused the if_watchdog interface to provide a stats timer that fired
every second. :) For a few drivers I also fixed other things such as busted
locking, order-of-operations issues in detach, or just completely busted
drivers (fea(4) and fpa(4) which share the pdq backend). Please test.
Barring any major screaming and shouting I plan to commit this in a week or
so and after that to work on removing the if_watchdog/if_timer stuff from the
network stack.
The patch is at http://www.FreeBSD.org/~jhb/patches/cleanup.patch
Driver details:
- an(4)
- Locking fixes to not do silly things like drop the lock only to call a
function that immediately reacquires the lock. Also removes recursive
locking.
- Hooks into the stat timer to drive the watchdog timer.
- bwi(4), cm(4), ep(4), fatm(4), malo(4), mwl(4), vx(4)
- Adds a new private timer to drive the watchdog timer.
- ce(4), cp(4), ctau(4), cx(4), lmc(4)
- These drivers have two modes, a netgraph mode and an ifnet mode. In the
netgraph mode they used a private timer to drive the watchdog. In the
ifnet mode they used if_watchdog. Now they just always use the private
timer.
- de(4)
- This driver abused the watchdog interface to manage its once-a-second
stat timer. It uses a callout to manage this now instead.
- ed(4)
- This driver used to provide a hook to allow individual attachments to
provide a 'tick' event to be called from an optional stats timer.
The stats timer only ran if the tick function pointer was non-NULL
and the attachment's tick routine had to call callout_reset(), etc.
Now the driver always schedules a stat timer and manages the
callout_reset() internally. This timer is used to drive the watchdog
and will also call the attachment's 'tick' handler if one is provided.
- ixgb(4)
- Uses callout_init_mtx() instead of callout_init(..., CALLOUT_MPSAFE).
- Remove silly callout handling in a few places (cancelling the callout
only to rescheduled it again immediately afterwards).
- Hooks into the stat timer to drive the watchdog timer.
- lge(4), nve(4), pcn(4)
- Hooks into the stat timer to drive the watchdog timer.
- my(4)
- This driver used the watchdog timer both as a watchdog on transmit and
auto-negotiation. To make this simpler and easier to understand I have
split this out into two separate timers. One just manages the auto-neg
side of things and one is a transmit watchdog.
- fea(4), fpa(4)
- These two drivers share a common backend, pdq, which was plagued with
several issues. I'm quite confident no one has used these drivers in
years since the are guaranteed to panic during attach.
- Add real locking. Previously these drivers only acquired their lock
in their interrupt handler or in the ioctl routine (but too broadly in
the latter). No locking was used for the stack calling down into the
driver via if_init() or if_start(), for device shutdown or detach. Also,
the interrupt handler held the driver lock while calling if_input(). All
this stuff should be fixed in the locking changes.
- Really fix these drivers to handle if_alloc(). The front-end attachments
were using if_initname() before the ifnet was allocated. Fix this by
moving some of the duplicated logic from each driver into pdq_ifattach().
While here, make pdq_ifattach() return an error so that the driver just
fails to attach if if_alloc() fails rather than panic'ing.
- Adds a new private timer to drive the watchdog timer.
- sn(4)
- Use bus_*() rather than bus_space_*().
- Adds a new private timer to drive the watchdog timer.
- Fixup detach.
- ste(4), ti(4)
- Adds a new private timer to drive the watchdog timer.
- Fixup detach.
- tl(4), wb(4)
- Use bus_*() rather than bus_space_*().
- Hooks into the stat timer to drive the watchdog timer.
- Fixup detach.
- vge(4)
- Overhaul the locking to avoid recursion and add missing locking in a few
places.
- Don't schedule a task to call vge_start() from contexts that are safe to
call vge_start() directly. Just invoke the routine directly instead
(this is what all of the other NIC drivers I am familiar with do). Note
that vge(4) does not use an interrupt filter handler which is the primary
reason some other drivers use tasks.
- Adds a new private timer to drive the watchdog timer.
- Use bus_*() rather than bus_space_*().
- Fixup detach.
- netfront(4)
- This doesn't actually implement a watchdog, it just sets if_watchdog to a
non-existent function under '#ifdef notyet'.
- admsw(4)
- This driver is a bit special in that it has no locking at all, not even
a poor attempt. :) It also appears to be for a specific MIPS board of
some sort.
- It has multiple ifnet's for multiple ports, but it only used if_timer and
if_watchdog from the first ifnet. For this driver I added a single
private timer to replace the if_timer use on the first ifnet. I marked
the callout MPSAFE, but the driver really needs to have locking added at
which point it could use callout_init_mtx().
--
John Baldwin
More information about the freebsd-net
mailing list