Module Name: src
Committed By: thorpej
Date: Tue Feb 4 19:42:56 UTC 2020
Modified Files:
src/sys/dev/pci/ixgbe: ixgbe.c ixgbe.h
Log Message:
- Fix locking problem with optics module interrupts: ifmedia_add()
may block on memory allocation, and so it cannot be safely done from
a softint nor can it be done while holding a spin lock. Fix this by
using a workqueue rather than a softint, and hold the IFNET_LOCK
across the entire handler, and the CORE_LOCK only across the code that
needs to serialize access to the hardware state.
- Use ifmedia_fini().
Tested in a variety of devices by msaitoh@. (Thanks!)
To generate a diff of this commit:
cvs rdiff -u -r1.222 -r1.223 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.62 -r1.63 src/sys/dev/pci/ixgbe/ixgbe.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/pci/ixgbe/ixgbe.c
diff -u src/sys/dev/pci/ixgbe/ixgbe.c:1.222 src/sys/dev/pci/ixgbe/ixgbe.c:1.223
--- src/sys/dev/pci/ixgbe/ixgbe.c:1.222 Sat Feb 1 12:55:22 2020
+++ src/sys/dev/pci/ixgbe/ixgbe.c Tue Feb 4 19:42:55 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.c,v 1.222 2020/02/01 12:55:22 thorpej Exp $ */
+/* $NetBSD: ixgbe.c,v 1.223 2020/02/04 19:42:55 thorpej Exp $ */
/******************************************************************************
@@ -267,10 +267,11 @@ static int ixgbe_msix_link(void *);
/* Software interrupts for deferred work */
static void ixgbe_handle_que(void *);
static void ixgbe_handle_link(void *);
-static void ixgbe_handle_msf(void *);
static void ixgbe_handle_mod(void *);
static void ixgbe_handle_phy(void *);
+static void ixgbe_handle_msf(struct work *, void *);
+
/* Workqueue handler for deferred work */
static void ixgbe_handle_que_work(struct work *, void *);
@@ -410,10 +411,12 @@ static int (*ixgbe_ring_empty)(struct if
#define IXGBE_CALLOUT_FLAGS CALLOUT_MPSAFE
#define IXGBE_SOFTINFT_FLAGS SOFTINT_MPSAFE
#define IXGBE_WORKQUEUE_FLAGS WQ_PERCPU | WQ_MPSAFE
+#define IXGBE_TASKLET_WQ_FLAGS WQ_MPSAFE
#else
#define IXGBE_CALLOUT_FLAGS 0
#define IXGBE_SOFTINFT_FLAGS 0
#define IXGBE_WORKQUEUE_FLAGS WQ_PERCPU
+#define IXGBE_TASKLET_WQ_FLAGS 0
#endif
#define IXGBE_WORKQUEUE_PRI PRI_SOFTNET
@@ -1099,8 +1102,6 @@ ixgbe_attach(device_t parent, device_t d
ixgbe_handle_link, adapter);
adapter->mod_si = softint_establish(SOFTINT_NET | IXGBE_SOFTINFT_FLAGS,
ixgbe_handle_mod, adapter);
- adapter->msf_si = softint_establish(SOFTINT_NET | IXGBE_SOFTINFT_FLAGS,
- ixgbe_handle_msf, adapter);
adapter->phy_si = softint_establish(SOFTINT_NET | IXGBE_SOFTINFT_FLAGS,
ixgbe_handle_phy, adapter);
if (adapter->feat_en & IXGBE_FEATURE_FDIR)
@@ -1108,13 +1109,23 @@ ixgbe_attach(device_t parent, device_t d
softint_establish(SOFTINT_NET | IXGBE_SOFTINFT_FLAGS,
ixgbe_reinit_fdir, adapter);
if ((adapter->link_si == NULL) || (adapter->mod_si == NULL)
- || (adapter->msf_si == NULL) || (adapter->phy_si == NULL)
+ || (adapter->phy_si == NULL)
|| ((adapter->feat_en & IXGBE_FEATURE_FDIR)
&& (adapter->fdir_si == NULL))) {
aprint_error_dev(dev,
"could not establish software interrupts ()\n");
goto err_out;
}
+ char wqname[MAXCOMLEN];
+ snprintf(wqname, sizeof(wqname), "%s-msf", device_xname(dev));
+ error = workqueue_create(&adapter->msf_wq, wqname,
+ ixgbe_handle_msf, adapter, IXGBE_WORKQUEUE_PRI, IPL_NET,
+ IXGBE_TASKLET_WQ_FLAGS);
+ if (error) {
+ aprint_error_dev(dev,
+ "could not create MSF workqueue (%d)\n", error);
+ goto err_out;
+ }
error = ixgbe_start_hw(hw);
switch (error) {
@@ -1510,7 +1521,9 @@ ixgbe_config_link(struct adapter *adapte
if (hw->phy.multispeed_fiber) {
ixgbe_enable_tx_laser(hw);
kpreempt_disable();
- softint_schedule(adapter->msf_si);
+ if (adapter->schedule_wqs_ok)
+ workqueue_enqueue(adapter->msf_wq,
+ &adapter->msf_wc, NULL);
kpreempt_enable();
}
kpreempt_disable();
@@ -3088,7 +3101,9 @@ ixgbe_msix_link(void *arg)
(eicr & IXGBE_EICR_GPI_SDP1_BY_MAC(hw))) {
IXGBE_WRITE_REG(hw, IXGBE_EICR,
IXGBE_EICR_GPI_SDP1_BY_MAC(hw));
- softint_schedule(adapter->msf_si);
+ if (adapter->schedule_wqs_ok)
+ workqueue_enqueue(adapter->msf_wq,
+ &adapter->msf_wc, NULL);
}
}
@@ -3500,9 +3515,9 @@ ixgbe_free_softint(struct adapter *adapt
softint_disestablish(adapter->mod_si);
adapter->mod_si = NULL;
}
- if (adapter->msf_si != NULL) {
- softint_disestablish(adapter->msf_si);
- adapter->msf_si = NULL;
+ if (adapter->msf_wq != NULL) {
+ workqueue_destroy(adapter->msf_wq);
+ adapter->msf_wq = NULL;
}
if (adapter->phy_si != NULL) {
softint_disestablish(adapter->phy_si);
@@ -3593,6 +3608,7 @@ ixgbe_detach(device_t dev, int flags)
bus_generic_detach(dev);
#endif
if_detach(adapter->ifp);
+ ifmedia_fini(&adapter->media);
if_percpuq_destroy(adapter->ipq);
sysctl_teardown(&adapter->sysctllog);
@@ -4129,6 +4145,9 @@ ixgbe_init_locked(struct adapter *adapte
/* Now inform the stack we're ready */
ifp->if_flags |= IFF_RUNNING;
+ /* OK to schedule workqueues. */
+ adapter->schedule_wqs_ok = true;
+
return;
} /* ixgbe_init_locked */
@@ -4657,7 +4676,8 @@ ixgbe_handle_mod(void *context)
goto out;
}
}
- softint_schedule(adapter->msf_si);
+ if (adapter->schedule_wqs_ok)
+ workqueue_enqueue(adapter->msf_wq, &adapter->msf_wc, NULL);
out:
IXGBE_CORE_UNLOCK(adapter);
} /* ixgbe_handle_mod */
@@ -4667,13 +4687,23 @@ out:
* ixgbe_handle_msf - Tasklet for MSF (multispeed fiber) interrupts
************************************************************************/
static void
-ixgbe_handle_msf(void *context)
+ixgbe_handle_msf(struct work *wk, void *context)
{
struct adapter *adapter = context;
+ struct ifnet *ifp = adapter->ifp;
struct ixgbe_hw *hw = &adapter->hw;
u32 autoneg;
bool negotiate;
+ /*
+ * Hold the IFNET_LOCK across this entire call. This will
+ * prevent additional changes to adapter->phy_layer and
+ * and serialize calls to this tasklet. We cannot hold the
+ * CORE_LOCK while calling into the ifmedia functions as
+ * they may block while allocating memory.
+ */
+ IFNET_LOCK(ifp);
+
IXGBE_CORE_LOCK(adapter);
++adapter->msf_sicount.ev_count;
/* get_supported_phy_layer will call hw->phy.ops.identify_sfp() */
@@ -4686,12 +4716,13 @@ ixgbe_handle_msf(void *context)
negotiate = 0;
if (hw->mac.ops.setup_link)
hw->mac.ops.setup_link(hw, autoneg, TRUE);
+ IXGBE_CORE_UNLOCK(adapter);
/* Adjust media types shown in ifconfig */
ifmedia_removeall(&adapter->media);
ixgbe_add_media_types(adapter);
ifmedia_set(&adapter->media, IFM_ETHER | IFM_AUTO);
- IXGBE_CORE_UNLOCK(adapter);
+ IFNET_UNLOCK(ifp);
} /* ixgbe_handle_msf */
/************************************************************************
@@ -4723,6 +4754,8 @@ ixgbe_ifstop(struct ifnet *ifp, int disa
IXGBE_CORE_LOCK(adapter);
ixgbe_stop(adapter);
IXGBE_CORE_UNLOCK(adapter);
+
+ workqueue_wait(adapter->msf_wq, &adapter->msf_wc);
}
/************************************************************************
@@ -4746,6 +4779,9 @@ ixgbe_stop(void *arg)
ixgbe_disable_intr(adapter);
callout_stop(&adapter->timer);
+ /* Don't schedule workqueues. */
+ adapter->schedule_wqs_ok = false;
+
/* Let the stack know...*/
ifp->if_flags &= ~IFF_RUNNING;
@@ -5088,7 +5124,9 @@ ixgbe_legacy_irq(void *arg)
(eicr & IXGBE_EICR_GPI_SDP1_BY_MAC(hw))) {
IXGBE_WRITE_REG(hw, IXGBE_EICR,
IXGBE_EICR_GPI_SDP1_BY_MAC(hw));
- softint_schedule(adapter->msf_si);
+ if (adapter->schedule_wqs_ok)
+ workqueue_enqueue(adapter->msf_wq,
+ &adapter->msf_wc, NULL);
}
}
Index: src/sys/dev/pci/ixgbe/ixgbe.h
diff -u src/sys/dev/pci/ixgbe/ixgbe.h:1.62 src/sys/dev/pci/ixgbe/ixgbe.h:1.63
--- src/sys/dev/pci/ixgbe/ixgbe.h:1.62 Tue Jan 21 14:55:55 2020
+++ src/sys/dev/pci/ixgbe/ixgbe.h Tue Feb 4 19:42:55 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.h,v 1.62 2020/01/21 14:55:55 msaitoh Exp $ */
+/* $NetBSD: ixgbe.h,v 1.63 2020/02/04 19:42:55 thorpej Exp $ */
/******************************************************************************
SPDX-License-Identifier: BSD-3-Clause
@@ -507,11 +507,14 @@ struct adapter {
/* Mbuf cluster size */
u32 rx_mbuf_sz;
+ bool schedule_wqs_ok;
+
/* Support for pluggable optics */
bool sfp_probe;
void *link_si; /* Link tasklet */
void *mod_si; /* SFP tasklet */
- void *msf_si; /* Multispeed Fiber */
+ struct workqueue *msf_wq; /* Multispeed Fiber */
+ struct work msf_wc;
void *mbx_si; /* VF -> PF mailbox interrupt */
/* Flow Director */
