I've received reports from various folks over the last year where iwm
would print messages like these while various ifconfig commands are used:
iwm0: could not initiate scan
iwm0: device timeout
I found this also happens on one of my machines during bsd.rd upgrades.
While the upgrade script processes /etc/hostname.iwm0 it ends up running
something like this:
ifconfig iwm0 nwid foo wpakey foofoofoo; ifconfig iwm0 down; dhclient iwm0
This command triggers the 'could not initiate scan' message for me even with
a GENERIC.MP kernel.
The problem seems to be that iwm's various tasks will sleep while waiting
for a device command to finish. If an ioctl process now sneaks in it will
trigger new commands to be sent in parallel, which the hardware does not like.
I see one way of fixing this: grab the ioctl lock in all of iwm's tasks.
With this patch I cannot reproduce the problem anymore.
It's a bit tricky since the newstate task may be scheduled from interrupt
context as well as ioctl process context. So it only grabs the lock if it
is not already held by another process.
I don't really like this but it seems this is needed for correctness.
Does somebody have a better idea?
Index: if_iwm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.155
diff -u -p -r1.155 if_iwm.c
--- if_iwm.c 18 Dec 2016 10:37:42 -0000 1.155
+++ if_iwm.c 5 Jan 2017 13:40:49 -0000
@@ -2461,13 +2461,20 @@ iwm_htprot_task(void *arg)
struct iwm_softc *sc = arg;
struct ieee80211com *ic = &sc->sc_ic;
struct iwm_node *in = (void *)ic->ic_bss;
- int err;
+ int s, err;
+
+ /* Prevent ioctls from interfering while we are sleeping. */
+ rw_enter_write(&sc->ioctl_rwl);
+ s = splnet();
/* This call updates HT protection based on in->in_ni.ni_htop1. */
err = iwm_mac_ctxt_cmd(sc, in, IWM_FW_CTXT_ACTION_MODIFY, 1);
if (err)
printf("%s: could not change HT protection: error %d\n",
DEVNAME(sc), err);
+
+ splx(s);
+ rw_exit(&sc->ioctl_rwl);
}
/*
@@ -2489,11 +2496,19 @@ iwm_ba_task(void *arg)
struct iwm_softc *sc = arg;
struct ieee80211com *ic = &sc->sc_ic;
struct ieee80211_node *ni = ic->ic_bss;
+ int s;
+
+ /* Prevent ioctls from interfering while we are sleeping. */
+ rw_enter_write(&sc->ioctl_rwl);
+ s = splnet();
if (sc->ba_start)
iwm_sta_rx_agg(sc, ni, sc->ba_tid, sc->ba_ssn, 1);
else
iwm_sta_rx_agg(sc, ni, sc->ba_tid, 0, 0);
+
+ splx(s);
+ rw_exit(&sc->ioctl_rwl);
}
/*
@@ -5336,9 +5351,17 @@ iwm_setrates_task(void *arg)
struct iwm_softc *sc = arg;
struct ieee80211com *ic = &sc->sc_ic;
struct iwm_node *in = (struct iwm_node *)ic->ic_bss;
+ int s;
+
+ /* Prevent ioctls from interfering while we are sleeping. */
+ rw_enter_write(&sc->ioctl_rwl);
+ s = splnet();
/* Update rates table based on new TX rate determined by AMRR. */
iwm_setrates(in);
+
+ splx(s);
+ rw_exit(&sc->ioctl_rwl);
}
void
@@ -5487,7 +5510,18 @@ iwm_newstate_task(void *psc)
enum ieee80211_state ostate = ic->ic_state;
struct iwm_node *in = (struct iwm_node *)ic->ic_bss;
int arg = sc->ns_arg;
- int err;
+ int err, s, locked;
+
+ /*
+ * Prevent ioctls from interfering while we are sleeping.
+ * Since this task can be scheduled from ioctl context another
+ * process might already be holding the lock and waiting for us
+ * to finish up. Don't grab the lock in that case.
+ */
+ locked = (rw_status(&sc->ioctl_rwl) != 0);
+ if (!locked)
+ rw_enter_write(&sc->ioctl_rwl);
+ s = splnet();
DPRINTF(("switching state %s->%s\n",
ieee80211_state_name[ostate],
@@ -5528,31 +5562,30 @@ iwm_newstate_task(void *psc)
case IEEE80211_S_SCAN:
if (ic->ic_state == nstate &&
(sc->sc_flags & IWM_FLAG_SCANNING))
- return;
+ goto out;
if (isset(sc->sc_enabled_capa, IWM_UCODE_TLV_CAPA_UMAC_SCAN))
err = iwm_umac_scan(sc);
else
err = iwm_lmac_scan(sc);
if (err) {
printf("%s: could not initiate scan\n", DEVNAME(sc));
- return;
+ goto out;
}
sc->sc_flags |= IWM_FLAG_SCANNING;
ic->ic_state = nstate;
iwm_led_blink_start(sc);
- return;
+ goto out;
case IEEE80211_S_AUTH:
err = iwm_auth(sc);
if (err)
- return;
-
+ goto out;
break;
case IEEE80211_S_ASSOC:
err = iwm_assoc(sc);
if (err)
- return;
+ goto out;
break;
case IEEE80211_S_RUN:
@@ -5564,7 +5597,7 @@ iwm_newstate_task(void *psc)
if (err) {
printf("%s: failed to update PHY\n",
DEVNAME(sc));
- return;
+ goto out;
}
}
@@ -5572,14 +5605,14 @@ iwm_newstate_task(void *psc)
err = iwm_mac_ctxt_cmd(sc, in, IWM_FW_CTXT_ACTION_MODIFY, 1);
if (err) {
printf("%s: failed to update MAC\n", DEVNAME(sc));
- return;
+ goto out;
}
err = iwm_power_update_device(sc);
if (err) {
printf("%s: could send power command (error %d)\n",
DEVNAME(sc), err);
- return;
+ goto out;
}
#ifdef notyet
/*
@@ -5591,21 +5624,21 @@ iwm_newstate_task(void *psc)
if (err) {
printf("%s: could not enable beacon filter\n",
DEVNAME(sc));
- return;
+ goto out;
}
#endif
err = iwm_power_mac_update_mode(sc, in);
if (err) {
printf("%s: could not update MAC power (error %d)\n",
DEVNAME(sc), err);
- return;
+ goto out;
}
err = iwm_update_quotas(sc, in);
if (err) {
printf("%s: could not update quotas (error %d)\n",
DEVNAME(sc), err);
- return;
+ goto out;
}
ieee80211_amrr_node_init(&sc->sc_amrr, &in->in_amn);
@@ -5625,6 +5658,10 @@ iwm_newstate_task(void *psc)
}
sc->sc_newstate(ic, nstate, arg);
+out:
+ splx(s);
+ if (!locked)
+ rw_exit(&sc->ioctl_rwl);
}
int
@@ -6162,7 +6199,6 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd,
struct ifreq *ifr;
int s, err = 0;
-
/*
* Prevent processes from entering this function while another
* process is tsleep'ing in it.
@@ -7317,6 +7353,7 @@ iwm_init_task(void *arg1)
struct ifnet *ifp = &sc->sc_ic.ic_if;
int s;
+ /* Prevent ioctls from interfering while we are sleeping. */
rw_enter_write(&sc->ioctl_rwl);
s = splnet();
