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

Convert ndis_spinlock to ndis_mtx and start using the sleepable

Browse Source 
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
This commit is contained in:
Coleman Kane 2008-06-11 13:40:15 +00:00
parent ac8b6edd89
commit 21a6592999
2 changed files with 5 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
sys/dev/if_ndis/if_ndis.c
View File

@ -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

8
sys/dev/if_ndis/if_ndisvar.h
View File

@ -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)