Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/usb fix issues seen with transfers being reused befo...
details: https://anonhg.NetBSD.org/src/rev/cb536503f358
branches: trunk
changeset: 452192:cb536503f358
user: mrg <mrg%NetBSD.org@localhost>
date: Fri Jun 21 14:19:46 2019 +0000
description:
fix issues seen with transfers being reused before they are finished
being used.
adapt locking to the modern world. some what inspired by if_smsc.c:
- add locks for softc, rx and tx
- add safe detach support
- safe detach vs mii lock requires 2 methods to lock the MII lock,
- check axen_dying and new axen_stopping more often
- consolidate checks to reduce the number of error paths that need
to release a resource
- move axen_watchdog() out of if_timer into the tick task to
prepare for MPSAFEification
TODO:
- remove spl usage
- enable mpsafe
special thanks to skrll and mlelstv for clearing up various
confusion and providing examples.
diffstat:
sys/dev/usb/if_axen.c | 329 ++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 251 insertions(+), 78 deletions(-)
diffs (truncated from 753 to 300 lines):
diff -r 1a67a9465d3c -r cb536503f358 sys/dev/usb/if_axen.c
--- a/sys/dev/usb/if_axen.c Fri Jun 21 10:59:50 2019 +0000
+++ b/sys/dev/usb/if_axen.c Fri Jun 21 14:19:46 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_axen.c,v 1.43 2019/06/20 10:46:24 mrg Exp $ */
+/* $NetBSD: if_axen.c,v 1.44 2019/06/21 14:19:46 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.43 2019/06/20 10:46:24 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_axen.c,v 1.44 2019/06/21 14:19:46 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -36,7 +36,6 @@
#include <sys/kernel.h>
#include <sys/mbuf.h>
#include <sys/module.h>
-#include <sys/rwlock.h>
#include <sys/socket.h>
#include <sys/sockio.h>
#include <sys/systm.h>
@@ -100,6 +99,8 @@
uint16_t axen_product;
uint16_t axen_flags;
+ uint16_t axen_timer;
+
int axen_ed[AXEN_ENDPT_MAX];
struct usbd_pipe *axen_ep[AXEN_ENDPT_MAX];
int axen_if_flags;
@@ -108,11 +109,16 @@
int axen_refcnt;
bool axen_dying;
+ bool axen_stopping;
bool axen_attached;
struct usb_task axen_tick_task;
+ kmutex_t axen_lock;
kmutex_t axen_mii_lock;
+ kmutex_t axen_rxlock;
+ kmutex_t axen_txlock;
+ kcondvar_t axen_detachcv;
int axen_link;
@@ -167,9 +173,11 @@
static void axen_tick(void *);
static void axen_tick_task(void *);
static void axen_start(struct ifnet *);
+static void axen_start_locked(struct ifnet *);
static int axen_ioctl(struct ifnet *, u_long, void *);
static int axen_init(struct ifnet *);
static void axen_stop(struct ifnet *, int);
+static void axen_stop_locked(struct ifnet *, int);
static void axen_watchdog(struct ifnet *);
static int axen_miibus_readreg(device_t, int, int, uint16_t *);
static int axen_miibus_writereg(device_t, int, int, uint16_t);
@@ -183,11 +191,26 @@
static void axen_ax88179_init(struct axen_softc *);
static void axen_setcoe(struct axen_softc *);
-/* Get exclusive access to the MII registers */
+/*
+ * Access functions for MII. Take the MII lock to call axen_cmd().
+ * Two forms: softc lock currently held or not.
+ */
static void
axen_lock_mii(struct axen_softc *sc)
{
+ mutex_enter(&sc->axen_lock);
+ sc->axen_refcnt++;
+ mutex_exit(&sc->axen_lock);
+
+ mutex_enter(&sc->axen_mii_lock);
+}
+
+static void
+axen_lock_mii_sc_locked(struct axen_softc *sc)
+{
+ KASSERT(mutex_owned(&sc->axen_lock));
+
sc->axen_refcnt++;
mutex_enter(&sc->axen_mii_lock);
}
@@ -197,8 +220,20 @@
{
mutex_exit(&sc->axen_mii_lock);
+ mutex_enter(&sc->axen_lock);
if (--sc->axen_refcnt < 0)
- usb_detach_wakeupold(sc->axen_dev);
+ cv_broadcast(&sc->axen_detachcv);
+ mutex_exit(&sc->axen_lock);
+}
+
+static void
+axen_unlock_mii_sc_locked(struct axen_softc *sc)
+{
+ KASSERT(mutex_owned(&sc->axen_lock));
+
+ mutex_exit(&sc->axen_mii_lock);
+ if (--sc->axen_refcnt < 0)
+ cv_broadcast(&sc->axen_detachcv);
}
static int
@@ -240,13 +275,12 @@
usbd_status err;
uint16_t data;
- if (sc->axen_dying) {
- DPRINTF(("axen: dying\n"));
+ mutex_enter(&sc->axen_lock);
+ if (sc->axen_dying || sc->axen_phyno != phy) {
+ mutex_exit(&sc->axen_lock);
return -1;
}
-
- if (sc->axen_phyno != phy)
- return -1;
+ mutex_exit(&sc->axen_lock);
axen_lock_mii(sc);
err = axen_cmd(sc, AXEN_CMD_MII_READ_REG, reg, phy, &data);
@@ -275,13 +309,15 @@
usbd_status err;
uint16_t uval;
- if (sc->axen_dying)
+ mutex_enter(&sc->axen_lock);
+ if (sc->axen_dying || sc->axen_phyno != phy) {
+ mutex_exit(&sc->axen_lock);
return -1;
-
- if (sc->axen_phyno != phy)
- return -1;
+ }
+ mutex_exit(&sc->axen_lock);
uval = htole16(val);
+
axen_lock_mii(sc);
err = axen_cmd(sc, AXEN_CMD_MII_WRITE_REG, reg, phy, &uval);
axen_unlock_mii(sc);
@@ -411,10 +447,11 @@
if (sc->axen_dying)
return;
+ KASSERT(mutex_owned(&sc->axen_mii_lock));
+
rxmode = 0;
/* Enable receiver, set RX mode */
- axen_lock_mii(sc);
axen_cmd(sc, AXEN_CMD_MAC_READ2, 2, AXEN_MAC_RXCTL, &wval);
rxmode = le16toh(wval);
rxmode &= ~(AXEN_RXCTL_ACPT_ALL_MCAST | AXEN_RXCTL_PROMISC |
@@ -457,13 +494,13 @@
axen_cmd(sc, AXEN_CMD_MAC_WRITE_FILTER, 8, AXEN_FILTER_MULTI, hashtbl);
wval = htole16(rxmode);
axen_cmd(sc, AXEN_CMD_MAC_WRITE2, 2, AXEN_MAC_RXCTL, &wval);
- axen_unlock_mii(sc);
}
static void
axen_reset(struct axen_softc *sc)
{
+ KASSERT(mutex_owned(&sc->axen_lock));
if (sc->axen_dying)
return;
/* XXX What to reset? */
@@ -696,7 +733,7 @@
uint64_t enabled = ifp->if_capenable;
uint8_t val;
- axen_lock_mii(sc);
+ KASSERT(mutex_owned(&sc->axen_mii_lock));
val = AXEN_RXCOE_OFF;
if (enabled & IFCAP_CSUM_IPv4_Rx)
@@ -723,8 +760,6 @@
if (enabled & IFCAP_CSUM_UDPv6_Tx)
val |= AXEN_TXCOE_UDPv6;
axen_cmd(sc, AXEN_CMD_MAC_WRITE, 1, AXEN_TX_COE, &val);
-
- axen_unlock_mii(sc);
}
static int
@@ -822,6 +857,10 @@
/* Set these up now for axen_cmd(). */
mutex_init(&sc->axen_mii_lock, MUTEX_DEFAULT, IPL_NONE);
+ mutex_init(&sc->axen_txlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+ mutex_init(&sc->axen_rxlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+ mutex_init(&sc->axen_lock, MUTEX_DEFAULT, IPL_NONE);
+ cv_init(&sc->axen_detachcv, "axendet");
s = splnet();
@@ -833,6 +872,10 @@
if (axen_get_eaddr(sc, &eaddr)) {
axen_unlock_mii(sc);
printf("EEPROM checksum error\n");
+ cv_destroy(&sc->axen_detachcv);
+ mutex_destroy(&sc->axen_lock);
+ mutex_destroy(&sc->axen_rxlock);
+ mutex_destroy(&sc->axen_txlock);
mutex_destroy(&sc->axen_mii_lock);
return;
}
@@ -859,7 +902,6 @@
ifp->if_start = axen_start;
ifp->if_init = axen_init;
ifp->if_stop = axen_stop;
- ifp->if_watchdog = axen_watchdog;
IFQ_SET_READY(&ifp->if_snd);
@@ -916,6 +958,10 @@
struct ifnet *ifp = GET_IFP(sc);
int s;
+ mutex_enter(&sc->axen_lock);
+ sc->axen_dying = true;
+ mutex_exit(&sc->axen_lock);
+
DPRINTFN(2,("%s: %s: enter\n", device_xname(sc->axen_dev), __func__));
/* Detached before attached finished, so just bail out. */
@@ -924,8 +970,6 @@
pmf_device_deregister(self);
- sc->axen_dying = true;
-
callout_halt(&sc->axen_stat_ch, NULL);
usb_rem_task_wait(sc->axen_udev, &sc->axen_tick_task,
USB_TASKQ_DRIVER, NULL);
@@ -935,12 +979,12 @@
if (ifp->if_flags & IFF_RUNNING)
axen_stop(ifp, 1);
- callout_destroy(&sc->axen_stat_ch);
- rnd_detach_source(&sc->rnd_source);
- mii_detach(&sc->axen_mii, MII_PHY_ANY, MII_OFFSET_ANY);
- ifmedia_delete_instance(&sc->axen_mii.mii_media, IFM_INST_ANY);
- ether_ifdetach(ifp);
- if_detach(ifp);
+ mutex_enter(&sc->axen_lock);
+ sc->axen_refcnt--;
+ while (sc->axen_refcnt > 0) {
+ /* Wait for processes to go away */
+ cv_wait(&sc->axen_detachcv, &sc->axen_lock);
+ }
#ifdef DIAGNOSTIC
if (sc->axen_ep[AXEN_ENDPT_TX] != NULL ||
@@ -949,16 +993,25 @@
aprint_debug_dev(self, "detach has active endpoints\n");
#endif
+ mutex_exit(&sc->axen_lock);
+
+ callout_destroy(&sc->axen_stat_ch);
+ rnd_detach_source(&sc->rnd_source);
+ mii_detach(&sc->axen_mii, MII_PHY_ANY, MII_OFFSET_ANY);
+ ifmedia_delete_instance(&sc->axen_mii.mii_media, IFM_INST_ANY);
+ ether_ifdetach(ifp);
+ if_detach(ifp);
+
sc->axen_attached = false;
- if (--sc->axen_refcnt >= 0) {
- /* Wait for processes to go away. */
- usb_detach_waitold(sc->axen_dev);
- }
splx(s);
usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->axen_udev,sc->axen_dev);
+ cv_destroy(&sc->axen_detachcv);
+ mutex_destroy(&sc->axen_lock);
+ mutex_destroy(&sc->axen_rxlock);
+ mutex_destroy(&sc->axen_txlock);
mutex_destroy(&sc->axen_mii_lock);
return 0;
@@ -975,7 +1028,17 @@
switch (act) {
case DVACT_DEACTIVATE:
Home |
Main Index |
Thread Index |
Old Index