Date: Fri, 14 Nov 2003 16:33:18 -0500 (EST) From: Robert Watson <rwatson@FreeBSD.org> To: Luigi Rizzo <rizzo@icir.org> Cc: src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/an if_an.c if_anreg.h src/sys/dev/bfe if_bfe.c src/sys/dev/my if_my.c src/sys/dev/owi if_owi.c if_wivar.h src/sys/dev/re if_re.c src/sys/dev/wl if_wl.c src/sys/pci if_dc.c if_dcreg.h if_pcn.c if_pcnreg.h if_rl.c ... Message-ID: <Pine.NEB.3.96L.1031114162807.18149F-100000@fledge.watson.org> In-Reply-To: <20031114132714.A88606@xorpc.icir.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 14 Nov 2003, Luigi Rizzo wrote:
> On Fri, Nov 14, 2003 at 11:00:33AM -0800, Sam Leffler wrote:
> > sam 2003/11/14 11:00:33 PST
> ...
> > Log:
> > Drop the driver lock around calls to if_input to avoid a LOR when
> > the packets are immediately returned for sending (e.g. when bridging
> > or packet forwarding). There are more efficient ways to do this
> > but for now use the least intrusive approach.
>
> the number of places that this commit had to touch makes me wonder
> whether it wouldn't be better to include somehow these calls into the
> if_input routine somehow...
I can think of two concerns with that: first, that interface locks may be
handled differently for different interfaces, and second, that in the
future we may want to avoid an unlock/lock pair for each packet input.
I've been running with local patches intended to reduce the number of
unlock/lock pairs for coalesced interrupt handling by only unlocking to
deliver chains of mbufs to if_input rather than individual mbufs. I.e.,
an approximation of the attached (can't remember if this is the exact
patch since those boxes are at home). It could also be argued that the
number of places that had to be touched in Sam's recent commit was a
property of the locking strategy evolving from "ad hoc" to something more
like a strategy.
Robert N M Watson FreeBSD Core Team, TrustedBSD Projects
robert@fledge.watson.org Network Associates Laboratories
Index: if_fxp.c
===================================================================
RCS file: /data/ncvs/src/sys/dev/fxp/if_fxp.c,v
retrieving revision 1.194
diff -u -r1.194 if_fxp.c
--- if_fxp.c 5 Sep 2003 22:37:31 -0000 1.194
+++ if_fxp.c 3 Oct 2003 16:43:11 -0000
@@ -1623,6 +1623,7 @@
fxp_intr_body(struct fxp_softc *sc, struct ifnet *ifp, u_int8_t statack,
int count)
{
+ struct mbuf *mqueue, *mqueue_tail;
struct mbuf *m;
struct fxp_rx *rxp;
struct fxp_rfa *rfa;
@@ -1685,6 +1686,7 @@
* record the pending RNR in the FXP_FLAG_DEFERRED_RNR flag so
* that the info will be used in the subsequent polling cycle.
*/
+ mqueue = mqueue_tail = NULL;
for (;;) {
rxp = sc->fxp_desc.rx_head;
m = rxp->rx_mbuf;
@@ -1766,10 +1768,21 @@
* packets received, dropping the lock, and then
* calling if_input() on each one.
*/
- FXP_UNLOCK(sc);
+ if (mqueue != NULL) {
+ mqueue_tail->m_nextpkt = m;
+ mqueue_tail = m;
+ } else
+ mqueue = mqueue_tail = m;
+ }
+ }
+ if (mqueue != NULL) {
+ FXP_UNLOCK(sc);
+ while (mqueue != NULL) {
+ m = mqueue;
+ mqueue = mqueue->m_nextpkt;
(*ifp->if_input)(ifp, m);
- FXP_LOCK(sc);
}
+ FXP_LOCK(sc);
}
if (rnr) {
fxp_scb_wait(sc);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1031114162807.18149F-100000>