Date: Fri, 02 Dec 2016 12:42:24 -0800 From: Chris Torek <torek@torek.net> To: freebsd-net@freebsd.org Subject: mutex usage in if_bridge vs other drivers Message-ID: <201612022042.uB2KgOkO055419@elf.torek.net>
next in thread | raw e-mail | index | archive | help
I'm not sure if this is really a freebsd-net topic or
more generic kernel, but since I observed this by looking
for possible causes of a bizarre network driver crash I'll
send it here (I'm not on the freebsd-net mailing list but
I'll check the archives for a bit too).
When doing if_bridge SIOCSDRVSPEC commands we go through
this code path:
static int
bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
{
[snip]
BRIDGE_LOCK(sc);
error = (*bc->bc_func)(sc, &args);
BRIDGE_UNLOCK(sc);
Note that the BRIDGE_LOCK is a plain (non-sleepable) mutex.
The bc_func may be one of many functions, but two interesting
ones are bridge_ioctl_add() and bridge_ioctl_del(). These
are somewhat complicated, but both eventually call:
bridge_mutecaps(sc);
which iterates over all (newly augmented, or remaining)
bridge members calling bridge_set_ifcap(sc, bif, enabled)
to enable or disable capabilities like IFCAP_TSO4 and so
on. This in turn sets up an SIOCSIFCAP ioctl and does:
struct ifnet *ifp = bif->bif_ifp;
if (ifp->if_capenable != set) {
error = (*ifp->if_ioctl)(ifp, SIOCSIFCAP, (caddr_t)&ifr);
...
}
In the case of dev/bxe/bxe.c, this now (as of FreeBSD 10)
has its own sleep (sx) lock, and it may lock that sx lock.
If it does, we panic because we're holding the bridge lock.
THE QUESTION:
- Who is wrong, the bxe driver or the bridge code? I.e.,
does the bridge driver need to release its lock here,
and if so, is that actually safe to do? (We might need
to restart the loop over all the members if we drop the
lock.)
- Or if the bridge driver should retain its lock, can it
use an sx lock here, to permit members to also use sx
locks?
Chris
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201612022042.uB2KgOkO055419>