Date: Wed, 11 Jun 2008 13:40:15 +0000 (UTC) From: Coleman Kane <cokane@FreeBSD.org> To: src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org Subject: cvs commit: src/sys/dev/if_ndis if_ndis.c if_ndisvar.h Message-ID: <200806111340.m5BDejje027411@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
cokane 2008-06-11 13:40:15 UTC
FreeBSD src repository
Modified files:
sys/dev/if_ndis if_ndis.c if_ndisvar.h
Log:
SVN rev 179723 on 2008-06-11 13:40:15Z by cokane
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
Revision Changes Path
1.142 +2 -3 src/sys/dev/if_ndis/if_ndis.c
1.33 +3 -5 src/sys/dev/if_ndis/if_ndisvar.h
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200806111340.m5BDejje027411>