Module Name: src
Committed By: riastradh
Date: Sun Aug 11 12:48:09 UTC 2024
Modified Files:
src/sys/dev/ic: dwc_gmac.c dwc_gmac_var.h
Log Message:
awge(4): Narrow scope of `core' lock and rename to mcast lock.
Sprinkle locking notes.
Proposed at:
https://mail-index.netbsd.org/source-changes-d/2024/08/10/msg014251.html
Comment-only changes from the updated patch at:
https://mail-index.netbsd.org/source-changes-d/2024/08/10/msg014252.html
To generate a diff of this commit:
cvs rdiff -u -r1.93 -r1.94 src/sys/dev/ic/dwc_gmac.c
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/ic/dwc_gmac_var.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/ic/dwc_gmac.c
diff -u src/sys/dev/ic/dwc_gmac.c:1.93 src/sys/dev/ic/dwc_gmac.c:1.94
--- src/sys/dev/ic/dwc_gmac.c:1.93 Sat Aug 10 12:16:47 2024
+++ src/sys/dev/ic/dwc_gmac.c Sun Aug 11 12:48:09 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: dwc_gmac.c,v 1.93 2024/08/10 12:16:47 skrll Exp $ */
+/* $NetBSD: dwc_gmac.c,v 1.94 2024/08/11 12:48:09 riastradh Exp $ */
/*-
* Copyright (c) 2013, 2014 The NetBSD Foundation, Inc.
@@ -39,9 +39,16 @@
* http://www.synopsys.com/dw/ipdir.php?ds=dwc_ether_mac10_100_1000_unive
*/
+/*
+ * Lock order:
+ *
+ * IFNET_LOCK -> sc_mcast_lock
+ * IFNET_LOCK -> sc_intr_lock -> {sc_txq.t_mtx, sc_rxq.r_mtx}
+ */
+
#include <sys/cdefs.h>
-__KERNEL_RCSID(1, "$NetBSD: dwc_gmac.c,v 1.93 2024/08/10 12:16:47 skrll Exp $");
+__KERNEL_RCSID(1, "$NetBSD: dwc_gmac.c,v 1.94 2024/08/11 12:48:09 riastradh Exp $");
/* #define DWC_GMAC_DEBUG 1 */
@@ -87,9 +94,7 @@ static void dwc_gmac_reset_tx_ring(struc
static void dwc_gmac_free_tx_ring(struct dwc_gmac_softc *, struct dwc_gmac_tx_ring *);
static void dwc_gmac_txdesc_sync(struct dwc_gmac_softc *, int, int, int);
static int dwc_gmac_init(struct ifnet *);
-static int dwc_gmac_init_locked(struct ifnet *);
static void dwc_gmac_stop(struct ifnet *, int);
-static void dwc_gmac_stop_locked(struct ifnet *, int);
static void dwc_gmac_start(struct ifnet *);
static void dwc_gmac_start_locked(struct ifnet *);
static int dwc_gmac_queue(struct dwc_gmac_softc *, struct mbuf *);
@@ -297,7 +302,7 @@ dwc_gmac_attach(struct dwc_gmac_softc *s
sc->sc_stopping = false;
sc->sc_txbusy = false;
- sc->sc_core_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
+ sc->sc_mcast_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
sc->sc_intr_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_txq.t_mtx, MUTEX_DEFAULT, IPL_NET);
mutex_init(&sc->sc_rxq.r_mtx, MUTEX_DEFAULT, IPL_NET);
@@ -863,28 +868,13 @@ static int
dwc_gmac_init(struct ifnet *ifp)
{
struct dwc_gmac_softc * const sc = ifp->if_softc;
-
- KASSERT(IFNET_LOCKED(ifp));
-
- mutex_enter(sc->sc_core_lock);
- int ret = dwc_gmac_init_locked(ifp);
- mutex_exit(sc->sc_core_lock);
-
- return ret;
-}
-
-static int
-dwc_gmac_init_locked(struct ifnet *ifp)
-{
- struct dwc_gmac_softc * const sc = ifp->if_softc;
uint32_t ffilt;
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
- KASSERT(mutex_owned(sc->sc_core_lock));
KASSERT(ifp == &sc->sc_ec.ec_if);
- dwc_gmac_stop_locked(ifp, 0);
+ dwc_gmac_stop(ifp, 0);
/*
* Configure DMA burst/transfer mode and RX/TX priorities.
@@ -914,7 +904,9 @@ dwc_gmac_init_locked(struct ifnet *ifp)
/*
* Set up multicast filter
*/
+ mutex_enter(sc->sc_mcast_lock);
dwc_gmac_setmulti(sc);
+ mutex_exit(sc->sc_mcast_lock);
/*
* Set up dma pointer for RX and TX ring
@@ -942,11 +934,11 @@ dwc_gmac_init_locked(struct ifnet *ifp)
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = false;
+ mutex_exit(sc->sc_intr_lock);
mutex_enter(&sc->sc_txq.t_mtx);
sc->sc_txbusy = false;
mutex_exit(&sc->sc_txq.t_mtx);
- mutex_exit(sc->sc_intr_lock);
return 0;
}
@@ -1015,29 +1007,23 @@ dwc_gmac_stop(struct ifnet *ifp, int dis
{
struct dwc_gmac_softc * const sc = ifp->if_softc;
- mutex_enter(sc->sc_core_lock);
- dwc_gmac_stop_locked(ifp, disable);
- mutex_exit(sc->sc_core_lock);
-}
-
-static void
-dwc_gmac_stop_locked(struct ifnet *ifp, int disable)
-{
- struct dwc_gmac_softc * const sc = ifp->if_softc;
-
ASSERT_SLEEPABLE();
KASSERT(IFNET_LOCKED(ifp));
- KASSERT(mutex_owned(sc->sc_core_lock));
+
+ ifp->if_flags &= ~IFF_RUNNING;
+
+ mutex_enter(sc->sc_mcast_lock);
+ sc->sc_if_flags = ifp->if_flags;
+ mutex_exit(sc->sc_mcast_lock);
mutex_enter(sc->sc_intr_lock);
sc->sc_stopping = true;
+ mutex_exit(sc->sc_intr_lock);
mutex_enter(&sc->sc_txq.t_mtx);
sc->sc_txbusy = false;
mutex_exit(&sc->sc_txq.t_mtx);
- mutex_exit(sc->sc_intr_lock);
-
bus_space_write_4(sc->sc_bst, sc->sc_bsh,
AWIN_GMAC_DMA_OPMODE,
bus_space_read_4(sc->sc_bst, sc->sc_bsh,
@@ -1051,9 +1037,6 @@ dwc_gmac_stop_locked(struct ifnet *ifp,
mii_down(&sc->sc_mii);
dwc_gmac_reset_tx_ring(sc, &sc->sc_txq);
dwc_gmac_reset_rx_ring(sc, &sc->sc_rxq);
-
- ifp->if_flags &= ~IFF_RUNNING;
- sc->sc_if_flags = ifp->if_flags;
}
/*
@@ -1150,7 +1133,7 @@ dwc_gmac_ifflags_cb(struct ethercom *ec)
int ret = 0;
KASSERT(IFNET_LOCKED(ifp));
- mutex_enter(sc->sc_core_lock);
+ mutex_enter(sc->sc_mcast_lock);
u_short change = ifp->if_flags ^ sc->sc_if_flags;
sc->sc_if_flags = ifp->if_flags;
@@ -1161,7 +1144,7 @@ dwc_gmac_ifflags_cb(struct ethercom *ec)
dwc_gmac_setmulti(sc);
}
- mutex_exit(sc->sc_core_lock);
+ mutex_exit(sc->sc_mcast_lock);
return ret;
}
@@ -1187,7 +1170,7 @@ dwc_gmac_ioctl(struct ifnet *ifp, u_long
if (error == ENETRESET) {
error = 0;
if (cmd == SIOCADDMULTI || cmd == SIOCDELMULTI) {
- mutex_enter(sc->sc_core_lock);
+ mutex_enter(sc->sc_mcast_lock);
if (sc->sc_if_flags & IFF_RUNNING) {
/*
* Multicast list has changed; set the hardware
@@ -1195,7 +1178,7 @@ dwc_gmac_ioctl(struct ifnet *ifp, u_long
*/
dwc_gmac_setmulti(sc);
}
- mutex_exit(sc->sc_core_lock);
+ mutex_exit(sc->sc_mcast_lock);
}
}
@@ -1393,7 +1376,6 @@ skip:
static void
dwc_gmac_setmulti(struct dwc_gmac_softc *sc)
{
- struct ifnet * const ifp = &sc->sc_ec.ec_if;
struct ether_multi *enm;
struct ether_multistep step;
struct ethercom *ec = &sc->sc_ec;
@@ -1401,7 +1383,7 @@ dwc_gmac_setmulti(struct dwc_gmac_softc
uint32_t ffilt, h;
int mcnt;
- KASSERT(mutex_owned(sc->sc_core_lock));
+ KASSERT(mutex_owned(sc->sc_mcast_lock));
ffilt = bus_space_read_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_FFILT);
@@ -1463,7 +1445,6 @@ special_filter:
0xffffffff);
bus_space_write_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_HTHIGH,
0xffffffff);
- sc->sc_if_flags = ifp->if_flags;
}
int
Index: src/sys/dev/ic/dwc_gmac_var.h
diff -u src/sys/dev/ic/dwc_gmac_var.h:1.21 src/sys/dev/ic/dwc_gmac_var.h:1.22
--- src/sys/dev/ic/dwc_gmac_var.h:1.21 Sat Aug 10 12:16:47 2024
+++ src/sys/dev/ic/dwc_gmac_var.h Sun Aug 11 12:48:09 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: dwc_gmac_var.h,v 1.21 2024/08/10 12:16:47 skrll Exp $ */
+/* $NetBSD: dwc_gmac_var.h,v 1.22 2024/08/11 12:48:09 riastradh Exp $ */
/*-
* Copyright (c) 2013, 2014 The NetBSD Foundation, Inc.
@@ -102,15 +102,15 @@ struct dwc_gmac_softc {
struct dwc_gmac_rx_ring sc_rxq;
struct dwc_gmac_tx_ring sc_txq;
const struct dwc_gmac_desc_methods *sc_descm;
- u_short sc_if_flags; /* shadow of ether flags */
+ u_short sc_if_flags; /* (sc_mcastlock) if_flags cache */
uint16_t sc_mii_clk;
- bool sc_txbusy;
- bool sc_stopping;
+ bool sc_txbusy; /* (sc_txq.t_mtx) no Tx because down or busy */
+ bool sc_stopping; /* (sc_intr_lock) ignore intr because down */
krndsource_t rnd_source;
- kmutex_t *sc_core_lock; /* lock for softc operations */
- kmutex_t *sc_intr_lock; /* lock for interrupt operations */
+ kmutex_t *sc_mcast_lock; /* lock for SIOCADD/DELMULTI */
+ kmutex_t *sc_intr_lock; /* lock for interrupt operations */
- struct if_percpuq *sc_ipq; /* softint-based input queues */
+ struct if_percpuq *sc_ipq; /* softint-based input queues */
void (*sc_set_speed)(struct dwc_gmac_softc *, int);
};
