Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/pci/ixgbe - Fix locking problem with optics module i...
details: https://anonhg.NetBSD.org/src/rev/78830794f5a2
branches: trunk
changeset: 1007046:78830794f5a2
user: thorpej <thorpej%NetBSD.org@localhost>
date: Tue Feb 04 19:42:55 2020 +0000
description:
- 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!)
diffstat:
sys/dev/pci/ixgbe/ixgbe.c | 66 +++++++++++++++++++++++++++++++++++++---------
sys/dev/pci/ixgbe/ixgbe.h | 7 +++-
2 files changed, 57 insertions(+), 16 deletions(-)
diffs (227 lines):
diff -r 2369b22f1c40 -r 78830794f5a2 sys/dev/pci/ixgbe/ixgbe.c
--- a/sys/dev/pci/ixgbe/ixgbe.c Tue Feb 04 18:36:16 2020 +0000
+++ b/sys/dev/pci/ixgbe/ixgbe.c Tue Feb 04 19:42:55 2020 +0000
@@ -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 @@
/* 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 @@
#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_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 @@
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 @@
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 @@
(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 @@
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 @@
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 @@
/* 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 @@
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 @@
* 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 @@
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_CORE_LOCK(adapter);
ixgbe_stop(adapter);
IXGBE_CORE_UNLOCK(adapter);
+
+ workqueue_wait(adapter->msf_wq, &adapter->msf_wc);
}
/************************************************************************
@@ -4746,6 +4779,9 @@
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 @@
(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);
}
}
diff -r 2369b22f1c40 -r 78830794f5a2 sys/dev/pci/ixgbe/ixgbe.h
--- a/sys/dev/pci/ixgbe/ixgbe.h Tue Feb 04 18:36:16 2020 +0000
+++ b/sys/dev/pci/ixgbe/ixgbe.h Tue Feb 04 19:42:55 2020 +0000
@@ -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 @@
/* 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 */
Home |
Main Index |
Thread Index |
Old Index