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

Various fixups to locking:

Browse Source 
- Remove a lot of superfluous locking during attach.  There is no need
  to lock access to the driver until some other thread has a way of getting
  to it.  For ethernet drivers the other ways include registering an
  interrupt handler via bus_setup_intr(), calling ether_ifattach() to hook
  into the network stack, and kicking off a callout-driven timer via
  callout_reset().
- Use callout_* rather than timeout/untimeout.
- Break out of xl_rxeof() if IFF_DRV_RUNNING is clear after ifp->if_input
  returns to handle the case where the interface was stopped while we were
  passing a packet up the stack.  Don't call xl_rxeof() in xl_rxeof_task()
  unless IFF_DRV_RUNNING is set.  With these fixes in place, any
  outstanding task will gracefully terminate as soon as it gets a chance to
  run after the interface has been stopped via xl_stop().  As a result,
  taskqueue_drain() is no longer required in xl_stop().  The task is still
  drained in detach() however to make sure that detach() can safely destroy
  the driver mutex at the end of the function.
- Lock the driver lock in the ifmedia callouts and don't lock across
  ifmedia_ioctl() in xl_ioctl().

Note: glebius came up with most of (3) as well independently.  I took a
rather roundabout way of arriving at the same conclusion.

MFC after:	3 days
This commit is contained in:
John Baldwin 2005-08-18 19:24:30 +00:00
parent 9aa2457d66
commit 997452064e
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=149251
2 changed files with 33 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
sys/pci/if_xl.c
View File

@ -458,8 +458,6 @@ xl_mii_readreg(struct xl_softc *sc, struct xl_mii_frame *frame)
{
int i, ack;
/*XL_LOCK_ASSERT(sc);*/
/* Set up frame for RX. */
frame->mii_stdelim = XL_MII_STARTDELIM;
frame->mii_opcode = XL_MII_READOP;
@ -528,8 +526,6 @@ static int
xl_mii_writereg(struct xl_softc *sc, struct xl_mii_frame *frame)
{
/*XL_LOCK_ASSERT(sc);*/
/* Set up frame for TX. */
frame->mii_stdelim = XL_MII_STARTDELIM;
frame->mii_opcode = XL_MII_WRITEOP;
@ -617,8 +613,6 @@ xl_miibus_statchg(device_t dev)
sc = device_get_softc(dev);
mii = device_get_softc(sc->xl_miibus);
/*XL_LOCK_ASSERT(sc);*/
xl_setcfg(sc);
/* Set ASIC's duplex mode to match the PHY. */
@ -651,8 +645,6 @@ xl_miibus_mediainit(device_t dev)
mii = device_get_softc(sc->xl_miibus);
ifm = &mii->mii_media;
/*XL_LOCK_ASSERT(sc);*/
if (sc->xl_media & (XL_MEDIAOPT_AUI | XL_MEDIAOPT_10FL)) {
/*
* Check for a 10baseFL board in disguise.
@ -716,8 +708,6 @@ xl_read_eeprom(struct xl_softc *sc, caddr_t dest, int off, int cnt, int swap)
int err = 0, i;
u_int16_t word = 0, *ptr;
XL_LOCK_ASSERT(sc);
#define EEPROM_5BIT_OFFSET(A) ((((A) << 2) & 0x7F00) | ((A) & 0x003F))
#define EEPROM_8BIT_OFFSET(A) ((A) & 0x003F)
/*
@ -904,7 +894,7 @@ xl_setmode(struct xl_softc *sc, int media)
u_int16_t mediastat;
char *pmsg = "", *dmsg = "";
/*XL_LOCK_ASSERT(sc);*/
XL_LOCK_ASSERT(sc);
XL_SEL_WIN(4);
mediastat = CSR_READ_2(sc, XL_W4_MEDIA_STATUS);
@ -1091,8 +1081,6 @@ static void
xl_mediacheck(struct xl_softc *sc)
{
XL_LOCK_ASSERT(sc);
/*
* If some of the media options bits are set, assume they are
* correct. If not, try to figure it out down below.
@ -1364,10 +1352,10 @@ xl_attach(device_t dev)
ifp->if_softc = sc;
if_initname(ifp, device_get_name(dev), device_get_unit(dev));
XL_LOCK(sc);
/* Reset the adapter. */
XL_LOCK(sc);
xl_reset(sc);
XL_UNLOCK(sc);
/*
* Get station address from the EEPROM.
@ -1375,14 +1363,11 @@ xl_attach(device_t dev)
if (xl_read_eeprom(sc, (caddr_t)&eaddr, XL_EE_OEM_ADR0, 3, 1)) {
device_printf(dev, "failed to read station address\n");
error = ENXIO;
XL_UNLOCK(sc);
goto fail;
}
XL_UNLOCK(sc);
sc->xl_unit = unit;
callout_handle_init(&sc->xl_stat_ch);
callout_init_mtx(&sc->xl_stat_callout, &sc->xl_mtx, 0);
TASK_INIT(&sc->xl_task, 0, xl_rxeof_task, sc);
/*
@ -1473,8 +1458,6 @@ xl_attach(device_t dev)
if (error)
goto fail;
XL_LOCK(sc);
/*
* Figure out the card type. 3c905B adapters have the
* 'supportsNoTxLength' bit set in the capabilities
@ -1533,9 +1516,6 @@ xl_attach(device_t dev)
xl_mediacheck(sc);
/* XXX Downcalls to ifmedia, miibus about to happen. */
XL_UNLOCK(sc);
if (sc->xl_media & XL_MEDIAOPT_MII ||
sc->xl_media & XL_MEDIAOPT_BTX ||
sc->xl_media & XL_MEDIAOPT_BT4) {
@ -1556,12 +1536,8 @@ xl_attach(device_t dev)
* a 10/100 card of some kind, we need to force the transceiver
* type to something sane.
*/
if (sc->xl_xcvr == XL_XCVR_AUTO) {
/* XXX Direct hardware access needs lock coverage. */
XL_LOCK(sc);
if (sc->xl_xcvr == XL_XCVR_AUTO)
xl_choose_xcvr(sc, bootverbose);
XL_UNLOCK(sc);
}
/*
* Do ifmedia setup.
@ -1610,7 +1586,6 @@ xl_attach(device_t dev)
ifmedia_add(&sc->ifmedia, IFM_ETHER|IFM_100_FX, 0, NULL);
}
/* XXX: Unlocked, leaf will take lock. */
media = IFM_ETHER|IFM_100_TX|IFM_FDX;
xl_choose_media(sc, &media);
@ -1618,7 +1593,6 @@ xl_attach(device_t dev)
ifmedia_set(&sc->ifmedia, media);
done:
/* XXX: Unlocked hardware access, narrow race. */
if (sc->xl_flags & XL_FLAG_NO_XCVR_PWR) {
XL_SEL_WIN(0);
CSR_WRITE_2(sc, XL_W0_MFG_ID, XL_NO_XCVR_PWR_MAGICBITS);
@ -1648,7 +1622,8 @@ xl_attach(device_t dev)
/*
* Choose a default media.
* XXX This is a leaf function only called by xl_attach() and
* acquires/releases the non-recursible driver mutex.
* acquires/releases the non-recursible driver mutex to
* satisfy lock assertions.
*/
static void
xl_choose_media(struct xl_softc *sc, int *media)
@ -1715,7 +1690,6 @@ xl_detach(device_t dev)
ifp = sc->xl_ifp;
KASSERT(mtx_initialized(&sc->xl_mtx), ("xl mutex not initialized"));
XL_LOCK(sc);
if (sc->xl_flags & XL_FLAG_USE_MMIO) {
rid = XL_PCI_LOMEM;
@ -1727,8 +1701,12 @@ xl_detach(device_t dev)
/* These should only be active if attach succeeded */
if (device_is_attached(dev)) {
XL_LOCK(sc);
xl_reset(sc);
xl_stop(sc);
XL_UNLOCK(sc);
taskqueue_drain(taskqueue_swi, &sc->xl_task);
callout_drain(&sc->xl_stat_callout);
ether_ifdetach(ifp);
if_free(ifp);
}
@ -1766,7 +1744,6 @@ xl_detach(device_t dev)
bus_dma_tag_destroy(sc->xl_ldata.xl_tx_tag);
}
XL_UNLOCK(sc);
mtx_destroy(&sc->xl_mtx);
return (0);
@ -2076,6 +2053,14 @@ xl_rxeof(struct xl_softc *sc)
XL_UNLOCK(sc);
(*ifp->if_input)(ifp, m);
XL_LOCK(sc);
/*
* If we are running from the taskqueue, the interface
* might have been stopped while we were passing the last
* packet up the network stack.
*/
if (!(ifp->if_drv_flags & IFF_DRV_RUNNING))
return;
}
/*
@ -2111,7 +2096,8 @@ xl_rxeof_task(void *arg, int pending)
NET_LOCK_GIANT();
XL_LOCK(sc);
xl_rxeof(sc);
if (sc->xl_ifp->if_drv_flags & IFF_DRV_RUNNING)
xl_rxeof(sc);
XL_UNLOCK(sc);
NET_UNLOCK_GIANT();
}
@ -2441,9 +2427,8 @@ xl_stats_update(void *xsc)
{
struct xl_softc *sc = xsc;
XL_LOCK(sc);
XL_LOCK_ASSERT(sc);
xl_stats_update_locked(sc);
XL_UNLOCK(sc);
}
static void
@ -2490,7 +2475,7 @@ xl_stats_update_locked(struct xl_softc *sc)
XL_SEL_WIN(7);
if (!sc->xl_stats_no_timeout)
sc->xl_stat_ch = timeout(xl_stats_update, sc, hz);
callout_reset(&sc->xl_stat_callout, hz, xl_stats_update, sc);
}
/*
@ -3032,7 +3017,7 @@ xl_init_locked(struct xl_softc *sc)
ifp->if_drv_flags |= IFF_DRV_RUNNING;
ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
sc->xl_stat_ch = timeout(xl_stats_update, sc, hz);
callout_reset(&sc->xl_stat_callout, hz, xl_stats_update, sc);
}
/*
@ -3045,7 +3030,7 @@ xl_ifmedia_upd(struct ifnet *ifp)
struct ifmedia *ifm = NULL;
struct mii_data *mii = NULL;
/*XL_LOCK_ASSERT(sc);*/
XL_LOCK(sc);
if (sc->xl_miibus != NULL)
mii = device_get_softc(sc->xl_miibus);
@ -3069,11 +3054,13 @@ xl_ifmedia_upd(struct ifnet *ifp)
if (sc->xl_media & XL_MEDIAOPT_MII ||
sc->xl_media & XL_MEDIAOPT_BTX ||
sc->xl_media & XL_MEDIAOPT_BT4) {
xl_init(sc); /* XXX */
xl_init_locked(sc);
} else {
xl_setmode(sc, ifm->ifm_media);
}
XL_UNLOCK(sc);
return (0);
}
@ -3088,7 +3075,7 @@ xl_ifmedia_sts(struct ifnet *ifp, struct ifmediareq *ifmr)
u_int16_t status = 0;
struct mii_data *mii = NULL;
/*XL_LOCK_ASSERT(sc);*/
XL_LOCK(sc);
if (sc->xl_miibus != NULL)
mii = device_get_softc(sc->xl_miibus);
@ -3148,6 +3135,8 @@ xl_ifmedia_sts(struct ifnet *ifp, struct ifmediareq *ifmr)
if_printf(ifp, "unknown XCVR type: %d\n", icfg);
break;
}
XL_UNLOCK(sc);
}
static int
@ -3205,8 +3194,6 @@ xl_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
break;
case SIOCGIFMEDIA:
case SIOCSIFMEDIA:
/* XXX Downcall from ifmedia possibly with locks held. */
/*XL_LOCK(sc);*/
if (sc->xl_miibus != NULL)
mii = device_get_softc(sc->xl_miibus);
if (mii == NULL)
@ -3215,7 +3202,6 @@ xl_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
else
error = ifmedia_ioctl(ifp, ifr,
&mii->mii_media, command);
/*XL_UNLOCK(sc);*/
break;
case SIOCSIFCAP:
XL_LOCK(sc);
@ -3286,8 +3272,6 @@ xl_stop(struct xl_softc *sc)
ether_poll_deregister(ifp);
#endif /* DEVICE_POLLING */
taskqueue_drain(taskqueue_swi, &sc->xl_task);
CSR_WRITE_2(sc, XL_COMMAND, XL_CMD_RX_DISABLE);
CSR_WRITE_2(sc, XL_COMMAND, XL_CMD_STATS_DISABLE);
CSR_WRITE_2(sc, XL_COMMAND, XL_CMD_INTR_ENB);
@ -3311,7 +3295,7 @@ xl_stop(struct xl_softc *sc)
bus_space_write_4(sc->xl_ftag, sc->xl_fhandle, 4, 0x8000);
/* Stop the stats updater. */
untimeout(xl_stats_update, sc, sc->xl_stat_ch);
callout_stop(&sc->xl_stat_callout);
/*
* Free data in the RX lists.

2
sys/pci/if_xlreg.h
View File

@ -601,7 +601,7 @@ struct xl_softc {
int xl_if_flags;
struct xl_list_data xl_ldata;
struct xl_chain_data xl_cdata;
struct callout_handle xl_stat_ch;
struct callout xl_stat_callout;
int xl_flags;
struct resource *xl_fres;
bus_space_handle_t xl_fhandle;