diff options
author | cokane <cokane@FreeBSD.org> | 2008-06-11 13:40:15 +0000 |
---|---|---|
committer | cokane <cokane@FreeBSD.org> | 2008-06-11 13:40:15 +0000 |
commit | 185ba1fe2a3aef7f25ba0480b937389b6f36ed91 (patch) | |
tree | 064be9eb7eaa6665497d6c1bb45149e26afcf649 /sys/dev | |
parent | be4c9e4b3b6be5549c8326f90be430c7db363910 (diff) | |
download | FreeBSD-src-185ba1fe2a3aef7f25ba0480b937389b6f36ed91.zip FreeBSD-src-185ba1fe2a3aef7f25ba0480b937389b6f36ed91.tar.gz |
Convert ndis_spinlock to ndis_mtx and start using the sleepable
mtx interface for NDIS_LOCK/UNLOCK. This should result in less
CPU utilization on behalf of the ndis driver. Additionally, this
commit also fixes a potential LOR in the ndis_tick code, by
not locking inside the ndis_tick function, but instead delegating
that work to the helpers called through IoQueueWorkItem. The
way that this is currently set up for NDIS prevents us from
simply implementing a callout_init_mtx mechanism.
However, the helper functions that handle the various timeout
cases implement fine-grained locking using the spinlocks provided
by the NDIS-compat layer, and using the mtx that is added with
this commit. This leaves the following ndis_softc members operated
on in ndis_tick in an unlocked context:
* ndis_hang_timer - Only modified outside of ndis_tick once, before
the first callout_reset to schedule ndis_tick
* ifp->if_oerrors - Only incremented in two places, which should be
an atomic op
* ndis_tx_timer - Assigned to 5 (when guaranteed to be 0) or 0
(in txeof), to indicate to ndis_tick what to
do. This is the only member of which I was
suspicious for needing the NDIS_LOCK here. My
testing (and another's) have been fine so far.
* ndis_stat_callout - Only uses a simple set of callout routines,
callout_reset only called by ndis_tick after
the initial reset, and then callout_drain is
used exactly once in shutdown code.
The benefit is that ndis_tick doesn't acquire NDIS_LOCK unless one of
the timeout conditions is flagged, and it still obeys the locking
order semantics that are dictated by the NDIS layer at the moment. I
have been investigating a more thorough s/spinlock/mtx/ of the NDIS
layer, but the simplest naive approach (replace KeAcquireSpinLock
with an mtx implementation) has anti-succeeded for me so far. This
is a good first step though.
Tested by: onemda@gmail.com
Reviewed by: current@, jhb, thompsa
Proposed by: jhb
Diffstat (limited to 'sys/dev')
-rw-r--r-- | sys/dev/if_ndis/if_ndis.c | 5 | ||||
-rw-r--r-- | sys/dev/if_ndis/if_ndisvar.h | 8 |
2 files changed, 5 insertions, 8 deletions
diff --git a/sys/dev/if_ndis/if_ndis.c b/sys/dev/if_ndis/if_ndis.c index c4b7128..30600f4 100644 --- a/sys/dev/if_ndis/if_ndis.c +++ b/sys/dev/if_ndis/if_ndis.c @@ -533,7 +533,8 @@ ndis_attach(dev) sc = device_get_softc(dev); - KeInitializeSpinLock(&sc->ndis_spinlock); + mtx_init(&sc->ndis_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, + MTX_DEF); KeInitializeSpinLock(&sc->ndis_rxlock); InitializeListHead(&sc->ndis_shlist); callout_init(&sc->ndis_stat_callout, CALLOUT_MPSAFE); @@ -1644,7 +1645,6 @@ ndis_tick(xsc) struct ndis_softc *sc; sc = xsc; - NDIS_LOCK(sc); if (sc->ndis_hang_timer && --sc->ndis_hang_timer == 0) { IoQueueWorkItem(sc->ndis_tickitem, @@ -1666,7 +1666,6 @@ ndis_tick(xsc) } callout_reset(&sc->ndis_stat_callout, hz, ndis_tick, sc); - NDIS_UNLOCK(sc); } static void diff --git a/sys/dev/if_ndis/if_ndisvar.h b/sys/dev/if_ndis/if_ndisvar.h index 61db134..868b5de 100644 --- a/sys/dev/if_ndis/if_ndisvar.h +++ b/sys/dev/if_ndis/if_ndisvar.h @@ -129,7 +129,7 @@ struct ndis_softc { struct resource *ndis_res_cm; /* common mem (pccard) */ struct resource_list ndis_rl; int ndis_rescnt; - kspin_lock ndis_spinlock; + struct mtx ndis_mtx; uint8_t ndis_irql; device_t ndis_dev; int ndis_unit; @@ -185,7 +185,5 @@ struct ndis_softc { int ndis_hang_timer; }; -#define NDIS_LOCK(_sc) KeAcquireSpinLock(&(_sc)->ndis_spinlock, \ - &(_sc)->ndis_irql); -#define NDIS_UNLOCK(_sc) KeReleaseSpinLock(&(_sc)->ndis_spinlock, \ - (_sc)->ndis_irql); +#define NDIS_LOCK(_sc) mtx_lock(&(_sc)->ndis_mtx) +#define NDIS_UNLOCK(_sc) mtx_unlock(&(_sc)->ndis_mtx) |