On Sun, Jul 19, 2015 at 04:32:39AM +0200, Stefan Sperling wrote:
> Please test this if you use iwm(4). It should make the driver more
> reliable, e.g. when bringing the interface up which sometimes fails
> because of... reasons.
Please test this updated diff instead. The previous one had races between
the ioctl handler and the newstate function, pointed out by kettenis@.
I believe claudio@ has already hit them. I got help from mpi@, too.
Index: if_iwm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.45
diff -u -p -r1.45 if_iwm.c
--- if_iwm.c 15 Jun 2015 08:06:11 -0000 1.45
+++ if_iwm.c 20 Jul 2015 22:33:16 -0000
@@ -195,14 +195,6 @@ const struct iwm_rate {
#define IWM_RIDX_IS_CCK(_i_) ((_i_) < IWM_RIDX_OFDM)
#define IWM_RIDX_IS_OFDM(_i_) ((_i_) >= IWM_RIDX_OFDM)
-struct iwm_newstate_state {
- struct task ns_wk;
- struct ieee80211com *ns_ic;
- enum ieee80211_state ns_nstate;
- int ns_arg;
- int ns_generation;
-};
-
int iwm_store_cscheme(struct iwm_softc *, uint8_t *, size_t);
int iwm_firmware_store_section(struct iwm_softc *, enum iwm_ucode_type,
uint8_t *, size_t);
@@ -406,13 +398,13 @@ struct ieee80211_node *iwm_node_alloc(st
void iwm_calib_timeout(void *);
void iwm_setrates(struct iwm_node *);
int iwm_media_change(struct ifnet *);
-void iwm_newstate_cb(void *);
+void iwm_newstate_task(void *);
int iwm_newstate(struct ieee80211com *, enum ieee80211_state, int);
void iwm_endscan_cb(void *);
int iwm_init_hw(struct iwm_softc *);
int iwm_init(struct ifnet *);
void iwm_start(struct ifnet *);
-void iwm_stop(struct ifnet *, int);
+void iwm_stop(struct ifnet *);
void iwm_watchdog(struct ifnet *);
int iwm_ioctl(struct ifnet *, u_long, iwm_caddr_t);
const char *iwm_desc_lookup(uint32_t);
@@ -425,9 +417,9 @@ int iwm_match(struct device *, void *, v
int iwm_preinit(struct iwm_softc *);
void iwm_attach_hook(iwm_hookarg_t);
void iwm_attach(struct device *, struct device *, void *);
-void iwm_init_task(void *);
int iwm_activate(struct device *, int);
-void iwm_wakeup(struct iwm_softc *);
+void iwm_suspend(struct iwm_softc *);
+void iwm_resume(struct iwm_softc *);
#if NBPFILTER > 0
void iwm_radiotap_attach(struct iwm_softc *);
@@ -5250,40 +5242,27 @@ iwm_media_change(struct ifnet *ifp)
sc->sc_fixed_ridx = ridx;
}
- if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) ==
- (IFF_UP | IFF_RUNNING)) {
- iwm_stop(ifp, 0);
- error = iwm_init(ifp);
- }
- return error;
+ /*
+ * No need to do anything on the hardware side.
+ * Channel changes will be applied during the
+ * association sequence in iwm_auth().
+ */
+
+ return (0);
}
void
-iwm_newstate_cb(void *wk)
+iwm_newstate_task(void *arg)
{
- struct iwm_newstate_state *iwmns = (void *)wk;
- struct ieee80211com *ic = iwmns->ns_ic;
- enum ieee80211_state nstate = iwmns->ns_nstate;
- int generation = iwmns->ns_generation;
+ struct iwm_softc *sc = arg;
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct ifnet *ifp = IC2IFP(&sc->sc_ic);
+ struct iwm_newstate_task_arg *task_arg = &sc->sc_newstate_task_arg;
+ enum ieee80211_state nstate = task_arg->state;
struct iwm_node *in;
- int arg = iwmns->ns_arg;
- struct ifnet *ifp = IC2IFP(ic);
- struct iwm_softc *sc = ifp->if_softc;
int error;
- free(iwmns, M_DEVBUF, sizeof(*iwmns));
-
- DPRINTF(("Prepare to switch state %d->%d\n", ic->ic_state, nstate));
- if (sc->sc_generation != generation) {
- DPRINTF(("newstate_cb: someone pulled the plug meanwhile\n"));
- if (nstate == IEEE80211_S_INIT) {
- DPRINTF(("newstate_cb: nstate == IEEE80211_S_INIT:
calling sc_newstate()\n"));
- sc->sc_newstate(ic, nstate, arg);
- }
- return;
- }
-
- DPRINTF(("switching state %d->%d\n", ic->ic_state, nstate));
+ sc->sc_newstate_errno = 0;
if (ic->ic_state == IEEE80211_S_SCAN && nstate != ic->ic_state)
iwm_led_blink_stop(sc);
@@ -5292,6 +5271,8 @@ iwm_newstate_cb(void *wk)
if (ic->ic_state == IEEE80211_S_RUN && nstate != ic->ic_state) {
iwm_mvm_disable_beacon_filter(sc, (void *)ic->ic_bss);
+ timeout_del(&sc->sc_calib_to);
+
if (((in = (void *)ic->ic_bss) != NULL))
in->in_assoc = 0;
iwm_release(sc, NULL);
@@ -5310,8 +5291,9 @@ iwm_newstate_cb(void *wk)
if (nstate == IEEE80211_S_SCAN ||
nstate == IEEE80211_S_AUTH ||
nstate == IEEE80211_S_ASSOC) {
- DPRINTF(("Force transition to INIT; MGT=%d\n", arg));
- sc->sc_newstate(ic, IEEE80211_S_INIT, arg);
+ DPRINTF(("Force transition to INIT; MGT=%d\n",
+ task_arg->arg));
+ sc->sc_newstate(ic, IEEE80211_S_INIT, task_arg->arg);
DPRINTF(("Going INIT->SCAN\n"));
nstate = IEEE80211_S_SCAN;
}
@@ -5319,10 +5301,25 @@ iwm_newstate_cb(void *wk)
switch (nstate) {
case IEEE80211_S_INIT:
- sc->sc_scanband = 0;
+ if (ifp->if_flags & IFF_RUNNING) {
+ /* Stop the device. */
+ sc->sc_scanband = 0;
+ sc->sc_auth_prot = 0;
+ iwm_stop(ifp);
+ }
+
+ if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP)
+ iwm_init(ifp);
break;
case IEEE80211_S_SCAN:
+ /* Allow scan only if the interface is up */
+ if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) !=
+ (IFF_UP | IFF_RUNNING)) {
+ sc->sc_newstate_errno = ENXIO;
+ return;
+ }
+
if (sc->sc_scanband)
break;
@@ -5330,11 +5327,13 @@ iwm_newstate_cb(void *wk)
ic->ic_des_esslen != 0,
ic->ic_des_essid, ic->ic_des_esslen)) != 0) {
printf("%s: could not initiate scan\n", DEVNAME(sc));
- return;
+ sc->sc_newstate_errno = EIO;
+ ieee80211_end_scan(&ic->ic_if);
+ break;
}
- ic->ic_state = nstate;
+
iwm_led_blink_start(sc);
- return;
+ break;
case IEEE80211_S_AUTH:
if ((error = iwm_auth(sc)) != 0) {
@@ -5374,36 +5373,30 @@ iwm_newstate_cb(void *wk)
timeout_add_msec(&sc->sc_calib_to, 500);
iwm_mvm_led_enable(sc);
break; }
-
- default:
- break;
}
- sc->sc_newstate(ic, nstate, arg);
+ sc->sc_newstate(ic, nstate, task_arg->arg);
+
+ /* Wake up sleeping process waiting for state transition completion. */
+ wakeup(&sc->sc_newstate);
+
+ if (nstate == IEEE80211_S_INIT &&
+ (ifp->if_flags & (IFF_UP | IFF_RUNNING)) == (IFF_UP | IFF_RUNNING))
+ ieee80211_begin_scan(ifp);
}
int
iwm_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg)
{
- struct iwm_newstate_state *iwmns;
- struct ifnet *ifp = IC2IFP(ic);
- struct iwm_softc *sc = ifp->if_softc;
-
- timeout_del(&sc->sc_calib_to);
-
- iwmns = malloc(sizeof(*iwmns), M_DEVBUF, M_NOWAIT);
- if (!iwmns) {
- DPRINTF(("%s: allocating state cb mem failed\n", DEVNAME(sc)));
- return ENOMEM;
- }
+ struct iwm_softc *sc = IC2IFP(ic)->if_softc;
+ struct iwm_newstate_task_arg *task_arg = &sc->sc_newstate_task_arg;
- iwmns->ns_ic = ic;
- iwmns->ns_nstate = nstate;
- iwmns->ns_arg = arg;
- iwmns->ns_generation = sc->sc_generation;
+ /* Remove any already scheduled transition. */
+ task_del(systq, &sc->sc_newstate_task);
- task_set(&iwmns->ns_wk, iwm_newstate_cb, iwmns);
- task_add(sc->sc_nswq, &iwmns->ns_wk);
+ task_arg->state = nstate;
+ task_arg->arg = arg;
+ task_add(systq, &sc->sc_newstate_task);
return 0;
}
@@ -5432,11 +5425,8 @@ iwm_endscan_cb(void *arg)
}
if (done) {
- if (!sc->sc_scanband) {
- ic->ic_scan_lock = IEEE80211_SCAN_UNLOCKED;
- } else {
+ if (sc->sc_scanband)
ieee80211_end_scan(&ic->ic_if);
- }
sc->sc_scanband = 0;
}
}
@@ -5541,24 +5531,18 @@ iwm_allow_mcast(struct iwm_softc *sc)
return error;
}
-/*
- * ifnet interfaces
- */
-
+/* Initialize the hardware and mark the interface as RUNNING. */
int
iwm_init(struct ifnet *ifp)
{
struct iwm_softc *sc = ifp->if_softc;
int error;
- if (sc->sc_flags & IWM_FLAG_HW_INITED) {
- return 0;
- }
sc->sc_generation++;
sc->sc_flags &= ~IWM_FLAG_STOPPED;
if ((error = iwm_init_hw(sc)) != 0) {
- iwm_stop(ifp, 1);
+ iwm_stop(ifp);
return error;
}
@@ -5569,9 +5553,6 @@ iwm_init(struct ifnet *ifp)
ifp->if_flags &= ~IFF_OACTIVE;
ifp->if_flags |= IFF_RUNNING;
- ieee80211_begin_scan(ifp);
- sc->sc_flags |= IWM_FLAG_HW_INITED;
-
return 0;
}
@@ -5648,25 +5629,15 @@ iwm_start(struct ifnet *ifp)
}
void
-iwm_stop(struct ifnet *ifp, int disable)
+iwm_stop(struct ifnet *ifp)
{
struct iwm_softc *sc = ifp->if_softc;
- struct ieee80211com *ic = &sc->sc_ic;
- sc->sc_flags &= ~IWM_FLAG_HW_INITED;
- sc->sc_flags |= IWM_FLAG_STOPPED;
- sc->sc_generation++;
- sc->sc_scanband = 0;
- sc->sc_auth_prot = 0;
- ic->ic_scan_lock = IEEE80211_SCAN_UNLOCKED;
ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
+ sc->sc_flags |= IWM_FLAG_STOPPED;
+ ifp->if_timer = 0;
+ sc->sc_tx_timer = 0; /* cancel watchdog */
- if (ic->ic_state != IEEE80211_S_INIT)
- ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
-
- timeout_del(&sc->sc_calib_to);
- iwm_led_blink_stop(sc);
- ifp->if_timer = sc->sc_tx_timer = 0;
iwm_stop_device(sc);
}
@@ -5674,6 +5645,7 @@ void
iwm_watchdog(struct ifnet *ifp)
{
struct iwm_softc *sc = ifp->if_softc;
+ struct ieee80211com *ic = &sc->sc_ic;
ifp->if_timer = 0;
if (sc->sc_tx_timer > 0) {
@@ -5682,7 +5654,7 @@ iwm_watchdog(struct ifnet *ifp)
#ifdef IWM_DEBUG
iwm_nic_error(sc);
#endif
- task_add(systq, &sc->init_task);
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
ifp->if_oerrors++;
return;
}
@@ -5703,18 +5675,6 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd,
s = splnet();
- /*
- * Prevent processes from entering this function while another
- * process is tsleep'ing in it.
- */
- while ((sc->sc_flags & IWM_FLAG_BUSY) && error == 0)
- error = tsleep(&sc->sc_flags, PCATCH, "iwmioc", 0);
- if (error != 0) {
- splx(s);
- return error;
- }
- sc->sc_flags |= IWM_FLAG_BUSY;
-
switch (cmd) {
case SIOCSIFADDR:
ifp->if_flags |= IFF_UP;
@@ -5725,12 +5685,16 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd,
case SIOCSIFFLAGS:
if (ifp->if_flags & IFF_UP) {
if (!(ifp->if_flags & IFF_RUNNING)) {
- if ((error = iwm_init(ifp)) != 0)
- ifp->if_flags &= ~IFF_UP;
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
+ tsleep(&sc->sc_newstate, 0, "iwmstart", 0);
+ error = sc->sc_newstate_errno;
}
} else {
- if (ifp->if_flags & IFF_RUNNING)
- iwm_stop(ifp, 1);
+ if (ifp->if_flags & IFF_RUNNING) {
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
+ tsleep(&sc->sc_newstate, 0, "iwmstop", 0);
+ error = sc->sc_newstate_errno;
+ }
}
break;
@@ -5748,17 +5712,9 @@ iwm_ioctl(struct ifnet *ifp, u_long cmd,
error = ieee80211_ioctl(ifp, cmd, data);
}
- if (error == ENETRESET) {
+ if (error == ENETRESET)
error = 0;
- if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) ==
- (IFF_UP | IFF_RUNNING)) {
- iwm_stop(ifp, 0);
- error = iwm_init(ifp);
- }
- }
- sc->sc_flags &= ~IWM_FLAG_BUSY;
- wakeup(&sc->sc_flags);
splx(s);
return error;
}
@@ -6158,6 +6114,7 @@ iwm_intr(void *arg)
{
struct iwm_softc *sc = arg;
struct ifnet *ifp = IC2IFP(&sc->sc_ic);
+ struct ieee80211com *ic = &sc->sc_ic;
int handled = 0;
int r1, r2, rv = 0;
int isperiodic = 0;
@@ -6227,7 +6184,7 @@ iwm_intr(void *arg)
printf("%s: fatal firmware error\n", DEVNAME(sc));
ifp->if_flags &= ~IFF_UP;
- iwm_stop(ifp, 1);
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
rv = 1;
goto out;
@@ -6237,7 +6194,7 @@ iwm_intr(void *arg)
handled |= IWM_CSR_INT_BIT_HW_ERR;
printf("%s: hardware error, stopping device \n", DEVNAME(sc));
ifp->if_flags &= ~IFF_UP;
- iwm_stop(ifp, 1);
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
rv = 1;
goto out;
}
@@ -6257,7 +6214,7 @@ iwm_intr(void *arg)
DPRINTF(("%s: rfkill switch, disabling interface\n",
DEVNAME(sc)));
ifp->if_flags &= ~IFF_UP;
- iwm_stop(ifp, 1);
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
}
}
@@ -6533,9 +6490,6 @@ iwm_attach(struct device *parent, struct
sc->sc_eswq = taskq_create("iwmes", 1, IPL_NET, 0);
if (sc->sc_eswq == NULL)
goto fail4;
- sc->sc_nswq = taskq_create("iwmns", 1, IPL_NET, 0);
- if (sc->sc_nswq == NULL)
- goto fail4;
/* Clear pending interrupts. */
IWM_WRITE(sc, IWM_CSR_INT, 0xffffffff);
@@ -6586,7 +6540,7 @@ iwm_attach(struct device *parent, struct
#endif
timeout_set(&sc->sc_calib_to, iwm_calib_timeout, sc);
timeout_set(&sc->sc_led_blink_to, iwm_led_blink_timeout, sc);
- task_set(&sc->init_task, iwm_init_task, sc);
+ task_set(&sc->sc_newstate_task, iwm_newstate_task, sc);
/*
* We cannot read the MAC address without loading the
@@ -6631,37 +6585,37 @@ iwm_radiotap_attach(struct iwm_softc *sc
#endif
void
-iwm_init_task(void *arg1)
+iwm_suspend(struct iwm_softc *sc)
{
- struct iwm_softc *sc = arg1;
struct ifnet *ifp = &sc->sc_ic.ic_if;
- int s;
+ struct ieee80211com *ic = &sc->sc_ic;
- s = splnet();
- while (sc->sc_flags & IWM_FLAG_BUSY)
- tsleep(&sc->sc_flags, 0, "iwmpwr", 0);
- sc->sc_flags |= IWM_FLAG_BUSY;
-
- iwm_stop(ifp, 0);
- if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP)
- iwm_init(ifp);
+ /* Remove any already scheduled transition. */
+ task_del(systq, &sc->sc_newstate_task);
- sc->sc_flags &= ~IWM_FLAG_BUSY;
- wakeup(&sc->sc_flags);
- splx(s);
+ /* Cancel pending timeouts. */
+ if (ic->ic_state == IEEE80211_S_SCAN)
+ iwm_led_blink_stop(sc);
+ if (ic->ic_state == IEEE80211_S_RUN)
+ timeout_del(&sc->sc_calib_to);
+
+ iwm_stop(ifp);
+
+ /* Reset net80211 state machine. */
+ sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
}
void
-iwm_wakeup(struct iwm_softc *sc)
+iwm_resume(struct iwm_softc *sc)
{
pcireg_t reg;
+ struct ieee80211com *ic = &sc->sc_ic;
/* Clear device-specific "PCI retry timeout" register (41h). */
reg = pci_conf_read(sc->sc_pct, sc->sc_pcitag, 0x40);
pci_conf_write(sc->sc_pct, sc->sc_pcitag, 0x40, reg & ~0xff00);
- iwm_init_task(sc);
-
+ ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
}
int
@@ -6673,10 +6627,10 @@ iwm_activate(struct device *self, int ac
switch (act) {
case DVACT_SUSPEND:
if (ifp->if_flags & IFF_RUNNING)
- iwm_stop(ifp, 0);
+ iwm_suspend(sc);
break;
- case DVACT_WAKEUP:
- iwm_wakeup(sc);
+ case DVACT_RESUME:
+ iwm_resume(sc);
break;
}
Index: if_iwmvar.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwmvar.h,v
retrieving revision 1.9
diff -u -p -r1.9 if_iwmvar.h
--- if_iwmvar.h 15 Jun 2015 08:06:12 -0000 1.9
+++ if_iwmvar.h 20 Jul 2015 22:11:16 -0000
@@ -288,10 +288,8 @@ struct iwm_rx_ring {
};
#define IWM_FLAG_USE_ICT 0x01
-#define IWM_FLAG_HW_INITED 0x02
-#define IWM_FLAG_STOPPED 0x04
-#define IWM_FLAG_RFKILL 0x08
-#define IWM_FLAG_BUSY 0x10
+#define IWM_FLAG_STOPPED 0x02
+#define IWM_FLAG_RFKILL 0x04
struct iwm_ucode_status {
uint32_t uc_error_event_table;
@@ -353,18 +351,23 @@ struct iwm_bf_data {
int last_cqm_event;
};
+struct iwm_newstate_task_arg {
+ enum ieee80211_state state;
+ int arg;
+};
+
struct iwm_softc {
struct device sc_dev;
struct ieee80211com sc_ic;
int (*sc_newstate)(struct ieee80211com *, enum ieee80211_state, int);
- int sc_newstate_pending;
+ struct task sc_newstate_task;
+ struct iwm_newstate_task_arg sc_newstate_task_arg;
+ int sc_newstate_errno;
struct ieee80211_amrr sc_amrr;
struct timeout sc_calib_to;
struct timeout sc_led_blink_to;
- struct task init_task;
-
bus_space_tag_t sc_st;
bus_space_handle_t sc_sh;
bus_size_t sc_sz;
@@ -448,7 +451,7 @@ struct iwm_softc {
uint8_t sc_cmd_resp[IWM_CMD_RESP_MAX];
int sc_wantresp;
- struct taskq *sc_nswq, *sc_eswq;
+ struct taskq *sc_eswq;
struct task sc_eswk;
struct iwm_rx_phy_info sc_last_phy_info;
