d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a deadlock in detach/shutdown.

Browse Source 
The problem was that I was acquiring the driver sx lock and then waiting
for a taskqueue to drain, however the taskqueue itself would try to
acquire the lock as well leading to a deadlock.

To fix the problem roll my own exclusive lock that allows for lock
cancellation.  This is a normal exclusive lock, however if someone
marks it as "dead" then all waiters who request an error return will
get back an error instead of continuing to wait for the lock.

In this particular case, the shutdown and detach functions kill the
lock while the async task thread tries to acquire the lock but will
abort if the lock returns an error.

The other option was to drop the driver lock mid-detach and mid-shutdown,
mid-detach was a ok, however mid-shutdown was not.

While I'm here, fix a bug in what appears to be the mii link status
word in the softc going out to lunch.  Explicitly set the status
word to 1 after initializing the mii.  This would result in an interface
that would never respond to "if_start" requests as the mii interface
would always look down.
This commit is contained in:
Alfred Perlstein 2006-12-23 17:18:18 +00:00
parent edea595c91
commit cc8317f2b0
2 changed files with 164 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

170
sys/dev/usb/if_aue.c
View File

@ -74,9 +74,10 @@ __FBSDID("$FreeBSD$");
#include <sys/mbuf.h>
#include <sys/malloc.h>
#include <sys/kernel.h>
#include <sys/kdb.h>
#include <sys/module.h>
#include <sys/socket.h>
#include <sys/sx.h>
#include <sys/condvar.h>
#include <sys/taskqueue.h>
#include <net/if.h>
@ -205,6 +206,7 @@ static void aue_start(struct ifnet *);
static void aue_start_thread(struct aue_softc *);
static int aue_ioctl(struct ifnet *, u_long, caddr_t);
static void aue_init(void *);
static void aue_init_body(struct aue_softc *);
static void aue_stop(struct aue_softc *);
static void aue_watchdog(struct ifnet *);
static void aue_shutdown(device_t);
@ -225,6 +227,14 @@ static int aue_csr_write_1(struct aue_softc *, int, int);
static int aue_csr_read_2(struct aue_softc *, int);
static int aue_csr_write_2(struct aue_softc *, int, int);
static int aue_xlock(struct aue_softc *, int);
static void aue_xunlock(struct aue_softc *);
static void aue_xlockkill(struct aue_softc *);
static void aue_xlockrevive(struct aue_softc *);
static void aue_xlockassert(struct aue_softc *, int,
const char *, const char *, int);
static void aue_stopasynctasks(struct aue_softc *);
static device_method_t aue_methods[] = {
/* Device interface */
DEVMETHOD(device_probe, aue_match),
@ -530,6 +540,7 @@ aue_setmulti(struct aue_softc *sc)
u_int32_t h = 0, i;
u_int8_t hashtbl[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
AUE_SXASSERTLOCKED(sc);
ifp = sc->aue_ifp;
if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
@ -702,7 +713,7 @@ USB_ATTACH(aue)
mtx_init(&sc->aue_mtx, device_get_nameunit(self), MTX_NETWORK_LOCK,
MTX_DEF | MTX_RECURSE);
sx_init(&sc->aue_sx, device_get_nameunit(self));
cv_init(&sc->aue_cv, device_get_nameunit(self));
AUE_SXLOCK(sc);
/* Reset the adapter. */
@ -718,7 +729,7 @@ USB_ATTACH(aue)
printf("aue%d: can not if_alloc()\n", sc->aue_unit);
AUE_SXUNLOCK(sc);
mtx_destroy(&sc->aue_mtx);
sx_destroy(&sc->aue_sx);
cv_destroy(&sc->aue_cv);
USB_ATTACH_ERROR_RETURN;
}
ifp->if_softc = sc;
@ -750,7 +761,7 @@ USB_ATTACH(aue)
if_free(ifp);
AUE_SXUNLOCK(sc);
mtx_destroy(&sc->aue_mtx);
sx_destroy(&sc->aue_sx);
cv_destroy(&sc->aue_cv);
USB_ATTACH_ERROR_RETURN;
}
@ -763,6 +774,7 @@ USB_ATTACH(aue)
ether_ifattach(ifp, eaddr);
usb_register_netisr();
sc->aue_dying = 0;
sc->aue_link = 1;
AUE_SXUNLOCK(sc);
USB_ATTACH_SUCCESS_RETURN;
@ -779,8 +791,7 @@ aue_detach(device_t dev)
ifp = sc->aue_ifp;
sc->aue_dying = 1;
callout_drain(&sc->aue_tick_callout);
taskqueue_drain(taskqueue_thread, &sc->aue_task);
aue_stopasynctasks(sc);
ether_ifdetach(ifp);
if_free(ifp);
@ -795,7 +806,7 @@ aue_detach(device_t dev)
AUE_SXUNLOCK(sc);
mtx_destroy(&sc->aue_mtx);
sx_destroy(&sc->aue_sx);
cv_destroy(&sc->aue_cv);
return (0);
}
@ -860,6 +871,7 @@ aue_rxeof_thread(struct aue_softc *sc)
usbd_status status = c->ue_status;
AUE_SXASSERTLOCKED(sc);
if (sc->aue_dying)
return;
ifp = sc->aue_ifp;
@ -1109,6 +1121,15 @@ static void
aue_init(void *xsc)
{
struct aue_softc *sc = xsc;
AUE_SXLOCK(sc);
aue_init_body(sc);
AUE_SXUNLOCK(sc);
}
static void
aue_init_body(struct aue_softc *sc)
{
struct ifnet *ifp = sc->aue_ifp;
struct mii_data *mii = GET_MII(sc);
struct ue_chain *c;
@ -1212,6 +1233,7 @@ aue_ifmedia_upd(struct ifnet *ifp)
mii_phy_reset(miisc);
}
mii_mediachg(mii);
sc->aue_link = 1;
return (0);
}
@ -1240,11 +1262,14 @@ aue_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
struct mii_data *mii;
int error = 0;
if (sc->aue_dying)
return(0);
AUE_GIANTLOCK();
AUE_SXLOCK(sc);
switch(command) {
case SIOCSIFFLAGS:
AUE_SXLOCK(sc);
if (ifp->if_flags & IFF_UP) {
if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
ifp->if_flags & IFF_PROMISC &&
@ -1255,30 +1280,32 @@ aue_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
sc->aue_if_flags & IFF_PROMISC) {
AUE_CLRBIT(sc, AUE_CTL2, AUE_CTL2_RX_PROMISC);
} else if (!(ifp->if_drv_flags & IFF_DRV_RUNNING))
aue_init(sc);
aue_init_body(sc);
} else {
if (ifp->if_drv_flags & IFF_DRV_RUNNING)
aue_stop(sc);
}
sc->aue_if_flags = ifp->if_flags;
error = 0;
AUE_SXUNLOCK(sc);
break;
case SIOCADDMULTI:
case SIOCDELMULTI:
AUE_SXLOCK(sc);
aue_setmulti(sc);
error = 0;
AUE_SXUNLOCK(sc);
break;
case SIOCGIFMEDIA:
case SIOCSIFMEDIA:
AUE_SXLOCK(sc);
mii = GET_MII(sc);
error = ifmedia_ioctl(ifp, ifr, &mii->mii_media, command);
AUE_SXUNLOCK(sc);
break;
default:
error = ether_ioctl(ifp, command, data);
break;
}
AUE_SXUNLOCK(sc);
AUE_GIANTUNLOCK();
return (error);
@ -1329,8 +1356,7 @@ aue_stop(struct aue_softc *sc)
aue_csr_write_1(sc, AUE_CTL0, 0);
aue_csr_write_1(sc, AUE_CTL1, 0);
aue_reset(sc);
callout_drain(&sc->aue_tick_callout);
taskqueue_drain(taskqueue_thread, &sc->aue_task);
aue_stopasynctasks(sc);
/* Stop transfers. */
if (sc->aue_ep[AUE_ENDPT_RX] != NULL) {
@ -1423,6 +1449,111 @@ aue_task_sched(struct aue_softc *sc, int task)
AUE_UNLOCK(sc);
}
static int
aue_xlock(struct aue_softc *sc, int interruptable)
{
AUE_LOCK(sc);
while ((sc->aue_lockflags & AUE_LOCKED) != 0) {
if (sc->aue_locker == curthread) {
#if 1
panic("aue: locking against myself");
#else
kdb_backtrace();
printf("aue: locking against myself\n");
#endif
}
if (interruptable) {
if ((sc->aue_lockflags & AUE_LOCKDEAD) != 0) {
AUE_UNLOCK(sc);
return (ENOLCK);
}
}
cv_wait(&sc->aue_cv, &sc->aue_mtx);
}
sc->aue_lockflags |= AUE_LOCKED;
sc->aue_locker = curthread;
AUE_UNLOCK(sc);
return (0);
}
static void
aue_xunlock(struct aue_softc *sc)
{
AUE_LOCK(sc);
if (sc->aue_locker != curthread) {
#if 1
panic("unlocking lock not owned by me");
#else
kdb_backtrace();
printf("unlocking lock not owned by me\n");
#endif
}
sc->aue_lockflags &= ~AUE_LOCKED;
sc->aue_locker = NULL;
cv_signal(&sc->aue_cv);
AUE_UNLOCK(sc);
}
static void
aue_xlockkill(struct aue_softc *sc)
{
AUE_LOCK(sc);
sc->aue_lockflags |= AUE_LOCKDEAD;
cv_broadcast(&sc->aue_cv);
AUE_UNLOCK(sc);
}
static void
aue_xlockrevive(struct aue_softc *sc)
{
AUE_LOCK(sc);
sc->aue_lockflags &= ~AUE_LOCKDEAD;
cv_broadcast(&sc->aue_cv);
AUE_UNLOCK(sc);
}
static void
aue_xlockassert(
struct aue_softc *sc,
int locked,
const char *file,
const char *fun,
int line)
{
struct thread *td;
int flags;
AUE_LOCK(sc);
flags = sc->aue_lockflags;
td = sc->aue_locker;
AUE_UNLOCK(sc);
if (locked == 1) {
if ((flags & AUE_LOCKED) == 0 || td != curthread) {
panic("aue assert: lock not owned @%s:%s:%d",
file, fun, line);
}
} else {
if ((flags & AUE_LOCKED) != 0 && td == curthread) {
panic("aue assert: lock owned @%s:%s:%d",
file, fun, line);
}
}
}
static void
aue_stopasynctasks(struct aue_softc *sc)
{
AUE_SXASSERTLOCKED(sc);
callout_drain(&sc->aue_tick_callout);
aue_xlockkill(sc);
taskqueue_drain(taskqueue_thread, &sc->aue_task);
aue_xlockrevive(sc);
}
/*
* We defer all interrupt operations to this function.
*
@ -1440,7 +1571,16 @@ aue_task(void *arg, int pending)
sc->aue_deferedtasks = 0;
AUE_UNLOCK(sc);
AUE_GIANTLOCK(); // XXX: usb not giant safe
AUE_SXLOCK(sc);
/*
* Try to lock the exclusive lock, if we fail
* then we are probably draining the taskqueue.
* If we did an unconditional lock here we would
* deadlock.
*/
if (aue_xlock(sc, 1)) {
AUE_GIANTUNLOCK(); // XXX: usb not giant safe
break;
}
if ((tasks & AUE_TASK_WATCHDOG) != 0) {
aue_watchdog_thread(sc);
}

14
sys/dev/usb/if_auereg.h
View File

@ -228,7 +228,11 @@ struct aue_softc {
struct callout aue_tick_callout;
struct task aue_task;
struct mtx aue_mtx;
struct sx aue_sx;
struct cv aue_cv;
struct thread * aue_locker; /* lock owner */
int aue_lockflags;
#define AUE_LOCKED 0x01 /* locked */
#define AUE_LOCKDEAD 0x02 /* lock draining */
u_int16_t aue_flags;
char aue_dying;
struct timeval aue_rx_notice;
@ -265,10 +269,10 @@ aue_dumpstate(const char *func, const char *tag)
#define AUE_LOCK(_sc) mtx_lock(&(_sc)->aue_mtx)
#define AUE_UNLOCK(_sc) mtx_unlock(&(_sc)->aue_mtx)
#define AUE_SXLOCK(_sc) \
do { AUE_DUMPSTATE("sxlock"); sx_xlock(&(_sc)->aue_sx); } while(0)
#define AUE_SXUNLOCK(_sc) sx_xunlock(&(_sc)->aue_sx)
#define AUE_SXASSERTLOCKED(_sc) sx_assert(&(_sc)->aue_sx, SX_XLOCKED)
#define AUE_SXASSERTUNLOCKED(_sc) sx_assert(&(_sc)->aue_sx, SX_UNLOCKED)
do { AUE_DUMPSTATE("sxlock"); aue_xlock((_sc), 0); } while(0)
#define AUE_SXUNLOCK(_sc) aue_xunlock(_sc)
#define AUE_SXASSERTLOCKED(_sc) aue_xlockassert((_sc), 1, __FILE__, __func__, __LINE__)
#define AUE_SXASSERTUNLOCKED(_sc) aue_xlockassert((_sc), 0, __FILE__, __func__, __LINE__)
#define AUE_TIMEOUT 1000
#define AUE_MIN_FRAMELEN 60