Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/usb more smp cleanup for ure(4)/axen(4)/cdce(4):
details: https://anonhg.NetBSD.org/src/rev/d3fdd9abbfd0
branches: trunk
changeset: 452331:d3fdd9abbfd0
user: mrg <mrg%NetBSD.org@localhost>
date: Fri Jun 28 01:57:43 2019 +0000
description:
more smp cleanup for ure(4)/axen(4)/cdce(4):
- convert IFF_ALLMULTI to ETHER_F_ALLMULTI, using ETHER_LOCK()
- remove IFF_OACTIVE use, and simply check the ring count in start
- assert/take more locks
- XXX: IFF_RUNNING is not properly protected (all driver problem)
- fix axen_timer setting so it actually runs
- document a locking issue in stop callback:
stop is called with the softc lock held, but the lock order
in all other places is ifnet -> softc -> rx -> tx, so taking
ifnet lock when softc lock is held would be problematic
- in rxeof check for stopping/dying more often. i managed to
trigger a pagefault in cdce_rxeof() when yanking an active
device as it attempted to usbd_setup_xfer() on closed pipes.
- add missing USBD_MPSAFE and cdce_stopping resetting for cdce(4)
between this and other recent clean ups increase performance of
these drivers mostly. some numbers (in mbit/sec):
old: new:
driver in out in+out in out in+out
---- -- --- ------ -- --- ------
cdce 39 32 44 38 33 54
axen 44 34 45 48 37 42
ure 36 34 35 36 38 38
i'm not sure why axen drops a little with in+out. cdce is
helped quite a lot, and ure a little. it is disappointing that
ure does not outperform cdce -- it's the same actual hardware,
and the device-specific (ure) should outperform the generic
cdce driver...
diffstat:
sys/dev/usb/if_axen.c | 47 +++++++++++++++++++++------------
sys/dev/usb/if_cdce.c | 70 +++++++++++++++++++++++++++++++++++++-------------
sys/dev/usb/if_ure.c | 45 ++++++++++++++++++++++----------
3 files changed, 112 insertions(+), 50 deletions(-)
diffs (truncated from 519 to 300 lines):
diff -r 2a8910d352ae -r d3fdd9abbfd0 sys/dev/usb/if_axen.c
--- a/sys/dev/usb/if_axen.c Thu Jun 27 19:56:10 2019 +0000
+++ b/sys/dev/usb/if_axen.c Fri Jun 28 01:57:43 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_axen.c,v 1.48 2019/06/22 10:58:39 mrg Exp $ */
+/* $NetBSD: if_axen.c,v 1.49 2019/06/28 01:57:43 mrg Exp $ */
/* $OpenBSD: if_axen.c,v 1.3 2013/10/21 10:10:22 yuo Exp $ */
/*
@@ -23,7 +23,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_axen.c,v 1.48 2019/06/22 10:58:39 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_axen.c,v 1.49 2019/06/28 01:57:43 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -455,12 +455,14 @@
rxmode = le16toh(wval);
rxmode &= ~(AXEN_RXCTL_ACPT_ALL_MCAST | AXEN_RXCTL_PROMISC |
AXEN_RXCTL_ACPT_MCAST);
- ifp->if_flags &= ~IFF_ALLMULTI;
if (ifp->if_flags & IFF_PROMISC) {
DPRINTF(("%s: promisc\n", device_xname(sc->axen_dev)));
rxmode |= AXEN_RXCTL_PROMISC;
-allmulti: ifp->if_flags |= IFF_ALLMULTI;
+allmulti:
+ ETHER_LOCK(ec);
+ ec->ec_flags |= ETHER_F_ALLMULTI;
+ ETHER_UNLOCK(ec);
rxmode |= AXEN_RXCTL_ACPT_ALL_MCAST
/* | AXEN_RXCTL_ACPT_PHY_MCAST */;
} else {
@@ -468,6 +470,8 @@
DPRINTF(("%s: initializing hash table\n",
device_xname(sc->axen_dev)));
ETHER_LOCK(ec);
+ ec->ec_flags &= ~ETHER_F_ALLMULTI;
+
ETHER_FIRST_MULTI(step, ec, enm);
while (enm != NULL) {
if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
@@ -979,8 +983,11 @@
usb_rem_task_wait(sc->axen_udev, &sc->axen_tick_task,
USB_TASKQ_DRIVER, NULL);
- if (ifp->if_flags & IFF_RUNNING)
+ if (ifp->if_flags & IFF_RUNNING) {
+ IFNET_LOCK(ifp);
axen_stop(ifp, 1);
+ IFNET_UNLOCK(ifp);
+ }
mutex_enter(&sc->axen_lock);
sc->axen_refcnt--;
@@ -1256,7 +1263,7 @@
if_percpuq_enqueue((ifp)->if_percpuq, (m));
mutex_enter(&sc->axen_rxlock);
- if (sc->axen_stopping) {
+ if (sc->axen_dying || sc->axen_stopping) {
mutex_exit(&sc->axen_rxlock);
return;
}
@@ -1274,6 +1281,11 @@
} while (pkt_count > 0);
done:
+ if (sc->axen_dying || sc->axen_stopping) {
+ mutex_exit(&sc->axen_rxlock);
+ return;
+ }
+
mutex_exit(&sc->axen_rxlock);
/* Setup new transfer. */
@@ -1351,7 +1363,6 @@
cd->axen_tx_cnt--;
sc->axen_timer = 0;
- ifp->if_flags &= ~IFF_OACTIVE;
switch (status) {
case USBD_NOT_STARTED:
@@ -1489,6 +1500,7 @@
/* Transmit */
err = usbd_transfer(c->axen_xfer);
if (err != USBD_IN_PROGRESS) {
+ /* XXXSMP IFNET_LOCK */
axen_stop(ifp, 0);
return EIO;
}
@@ -1505,11 +1517,9 @@
int idx;
KASSERT(mutex_owned(&sc->axen_txlock));
+ KASSERT(cd->axen_tx_cnt <= AXEN_TX_LIST_CNT);
- if (sc->axen_link == 0)
- return;
-
- if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) != IFF_RUNNING)
+ if (sc->axen_link == 0 || (ifp->if_flags & IFF_RUNNING) == 0)
return;
idx = cd->axen_tx_prod;
@@ -1519,7 +1529,6 @@
break;
if (axen_encap(sc, m, idx)) {
- ifp->if_flags |= IFF_OACTIVE; /* XXX */
ifp->if_oerrors++;
break;
}
@@ -1537,9 +1546,6 @@
}
cd->axen_tx_prod = idx;
- if (cd->axen_tx_cnt >= AXEN_TX_LIST_CNT)
- ifp->if_flags |= IFF_OACTIVE;
-
/*
* Set a timeout in case the chip goes out to lunch.
*/
@@ -1646,8 +1652,8 @@
mutex_exit(&sc->axen_rxlock);
/* Indicate we are up and running. */
+ KASSERT(IFNET_LOCKED(ifp));
ifp->if_flags |= IFF_RUNNING;
- ifp->if_flags &= ~IFF_OACTIVE;
callout_schedule(&sc->axen_stat_ch, hz);
return 0;
@@ -1771,8 +1777,15 @@
axen_cmd(sc, AXEN_CMD_MAC_WRITE2, 2, AXEN_MAC_RXCTL, &wval);
axen_unlock_mii_sc_locked(sc);
+ /*
+ * XXXSMP Would like to
+ * KASSERT(IFNET_LOCKED(ifp))
+ * here but the locking order is:
+ * ifnet -> sc lock -> rxlock -> txlock
+ * and sc lock is already held.
+ */
+ ifp->if_flags &= ~IFF_RUNNING;
sc->axen_timer = 0;
- ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
callout_stop(&sc->axen_stat_ch);
sc->axen_link = 0;
diff -r 2a8910d352ae -r d3fdd9abbfd0 sys/dev/usb/if_cdce.c
--- a/sys/dev/usb/if_cdce.c Thu Jun 27 19:56:10 2019 +0000
+++ b/sys/dev/usb/if_cdce.c Fri Jun 28 01:57:43 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_cdce.c,v 1.50 2019/06/23 02:14:14 mrg Exp $ */
+/* $NetBSD: if_cdce.c,v 1.51 2019/06/28 01:57:43 mrg Exp $ */
/*
* Copyright (c) 1997, 1998, 1999, 2000-2003 Bill Paul <wpaul%windriver.com@localhost>
@@ -41,7 +41,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_cdce.c,v 1.50 2019/06/23 02:14:14 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_cdce.c,v 1.51 2019/06/28 01:57:43 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -146,7 +146,6 @@
static void cdce_start(struct ifnet *);
static int cdce_ioctl(struct ifnet *, u_long, void *);
static void cdce_init(void *);
-static void cdce_watchdog(struct ifnet *);
static void cdce_stop(struct cdce_softc *);
static void cdce_tick(void *);
static void cdce_tick_task(void *);
@@ -384,8 +383,11 @@
usb_rem_task_wait(sc->cdce_udev, &sc->cdce_tick_task,
USB_TASKQ_DRIVER, NULL);
- if (ifp->if_flags & IFF_RUNNING)
+ if (ifp->if_flags & IFF_RUNNING) {
+ IFNET_LOCK(ifp);
cdce_stop(sc);
+ IFNET_UNLOCK(ifp);
+ }
callout_destroy(&sc->cdce_stat_ch);
ether_ifdetach(ifp);
@@ -408,8 +410,8 @@
struct mbuf *m_head = NULL;
KASSERT(mutex_owned(&sc->cdce_txlock));
- if (sc->cdce_dying || sc->cdce_stopping ||
- (ifp->if_flags & IFF_OACTIVE))
+
+ if (sc->cdce_dying || sc->cdce_cdata.cdce_tx_cnt == CDCE_TX_LIST_CNT)
return;
IFQ_POLL(&ifp->if_snd, m_head);
@@ -425,12 +427,10 @@
bpf_mtap(ifp, m_head, BPF_D_OUT);
- ifp->if_flags |= IFF_OACTIVE;
-
/*
* Set a timeout in case the chip goes out to lunch.
*/
- ifp->if_timer = 6;
+ sc->cdce_timer = 6;
}
static void
@@ -469,30 +469,42 @@
USBD_FORCE_SHORT_XFER, 10000, cdce_txeof);
err = usbd_transfer(c->cdce_xfer);
if (err != USBD_IN_PROGRESS) {
+ /* XXXSMP IFNET_LOCK */
cdce_stop(sc);
return EIO;
}
sc->cdce_cdata.cdce_tx_cnt++;
+ KASSERT(sc->cdce_cdata.cdce_tx_cnt <= CDCE_TX_LIST_CNT);
return 0;
}
static void
-cdce_stop(struct cdce_softc *sc)
+cdce_stop_locked(struct cdce_softc *sc)
{
usbd_status err;
struct ifnet *ifp = GET_IFP(sc);
int i;
+ /* XXXSMP can't KASSERT(IFNET_LOCKED(ifp)); */
+ KASSERT(mutex_owned(&sc->cdce_lock));
+
mutex_enter(&sc->cdce_rxlock);
mutex_enter(&sc->cdce_txlock);
sc->cdce_stopping = true;
mutex_exit(&sc->cdce_txlock);
mutex_exit(&sc->cdce_rxlock);
- ifp->if_timer = 0;
- ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
+ /*
+ * XXXSMP Would like to
+ * KASSERT(IFNET_LOCKED(ifp))
+ * here but the locking order is:
+ * ifnet -> sc lock -> rxlock -> txlock
+ * and sc lock is already held.
+ */
+ ifp->if_flags &= ~IFF_RUNNING;
+ sc->cdce_timer = 0;
callout_stop(&sc->cdce_stat_ch);
@@ -549,7 +561,15 @@
device_xname(sc->cdce_dev), usbd_errstr(err));
sc->cdce_bulkout_pipe = NULL;
}
- mutex_exit(&sc->cdce_txlock);
+}
+
+static void
+cdce_stop(struct cdce_softc *sc)
+{
+
+ mutex_enter(&sc->cdce_lock);
+ cdce_stop_locked(sc);
+ mutex_exit(&sc->cdce_lock);
}
static int
@@ -613,7 +633,7 @@
static void
cdce_watchdog(struct ifnet *ifp)
{
- struct cdce_softc *sc = ifp->if_softc;
+ struct cdce_softc *sc = ifp->if_softc;
struct cdce_chain *c;
usbd_status stat;
@@ -625,12 +645,16 @@
ifp->if_oerrors++;
aprint_error_dev(sc->cdce_dev, "watchdog timeout\n");
+ mutex_enter(&sc->cdce_txlock);
+
c = &sc->cdce_cdata.cdce_rx_chain[0];
usbd_get_xfer_status(c->cdce_xfer, NULL, NULL, NULL, &stat);
cdce_txeof(c->cdce_xfer, c, stat);
if (!IFQ_IS_EMPTY(&ifp->if_snd))
Home |
Main Index |
Thread Index |
Old Index