Fix a deadlock in detach/shutdown.
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
parent
05d1f24e02
commit
15acdf0cd3
@ -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);
|
||||
}
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user