This is yet another attempt at improving the iwm(4) newstate task.

The goal is to simplify things by only queuing one state transition
at a time. The newstate task now always transitions to the most
recently requested state, rather than hopping along with every request.

This allows us get rid of the silly newstate generation counter, and
allows us to simply cancel any outstanding transition when the interface
goes down.

The old code was queuing *additional* work from iwm_stop(). Which meant,
for example, that every time upon resume, a task ran only to discover that
it is no longer relevant.

This probably needs some testing to shake out bugs.
Test reports are very much appreciated!

This change might also fix semi-frequent firmware errors during association.
But not all -- I've found that running with IWM_DEBUG cranked up produces
sufficient printfs to make firmware commands time out more often.
It all seems very sensitive to timing which is hard to get completely
right with tasks involved.

Index: if_iwm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.51
diff -u -p -r1.51 if_iwm.c
--- if_iwm.c    27 Sep 2015 16:53:38 -0000      1.51
+++ if_iwm.c    27 Sep 2015 17:33:58 -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,7 +398,7 @@ 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 *);
@@ -5263,43 +5255,29 @@ iwm_media_change(struct ifnet *ifp)
 }
 
 void
-iwm_newstate_cb(void *wk)
+iwm_newstate_task(void *psc)
 {
-       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 = (struct iwm_softc *)psc;
+       struct ieee80211com *ic = &sc->sc_ic;
+       enum ieee80211_state nstate = sc->ns_nstate;
+       enum ieee80211_state ostate = ic->ic_state;
        struct iwm_node *in;
-       int arg = iwmns->ns_arg;
-       struct ifnet *ifp = IC2IFP(ic);
-       struct iwm_softc *sc = ifp->if_softc;
+       int arg = sc->ns_arg;
        int error;
 
-       free(iwmns, M_DEVBUF, sizeof(*iwmns));
-
-       DPRINTF(("Prepare to switch state %s->%s\n",
-           ieee80211_state_name[ic->ic_state],
-           ieee80211_state_name[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 %s->%s\n",
-           ieee80211_state_name[ic->ic_state],
+           ieee80211_state_name[ostate],
            ieee80211_state_name[nstate]));
 
-       if (ic->ic_state == IEEE80211_S_SCAN && nstate != ic->ic_state)
+       if (ostate == IEEE80211_S_SCAN && nstate != ostate)
                iwm_led_blink_stop(sc);
 
        /* disable beacon filtering if we're hopping out of RUN */
-       if (ic->ic_state == IEEE80211_S_RUN && nstate != ic->ic_state) {
+       if (ostate == IEEE80211_S_RUN && nstate != ostate)
                iwm_mvm_disable_beacon_filter(sc, (void *)ic->ic_bss);
 
+       /* Reset the device if moving out of AUTH, ASSOC, or RUN. */
+       if (ostate > IEEE80211_S_SCAN && nstate < ostate) {
                if (((in = (void *)ic->ic_bss) != NULL))
                        in->in_assoc = 0;
                iwm_release(sc, NULL);
@@ -5393,25 +5371,15 @@ iwm_newstate_cb(void *wk)
 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;
-       }
-
-       iwmns->ns_ic = ic;
-       iwmns->ns_nstate = nstate;
-       iwmns->ns_arg = arg;
-       iwmns->ns_generation = sc->sc_generation;
+       sc->ns_nstate = nstate;
+       sc->ns_arg = arg;
 
-       task_set(&iwmns->ns_wk, iwm_newstate_cb, iwmns);
-       task_add(sc->sc_nswq, &iwmns->ns_wk);
+       task_add(sc->sc_nswq, &sc->newstate_task);
 
        return 0;
 }
@@ -5676,8 +5644,10 @@ iwm_stop(struct ifnet *ifp, int disable)
        ic->ic_scan_lock = IEEE80211_SCAN_UNLOCKED;
        ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
 
-       if (ic->ic_state != IEEE80211_S_INIT)
-               ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
+       task_del(systq, &sc->init_task);
+       task_del(sc->sc_nswq, &sc->newstate_task);
+       task_del(sc->sc_eswq, &sc->sc_eswk);
+       sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
 
        timeout_del(&sc->sc_calib_to);
        iwm_led_blink_stop(sc);
@@ -6614,6 +6584,7 @@ iwm_attach(struct device *parent, struct
        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->newstate_task, iwm_newstate_task, sc);
 
        /*
         * We cannot read the MAC address without loading the
@@ -6687,8 +6658,7 @@ iwm_wakeup(struct iwm_softc *sc)
        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);
-
+       task_add(systq, &sc->init_task);
 }
 
 int
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 27 Sep 2015 17:09:03 -0000
@@ -364,6 +364,9 @@ struct iwm_softc {
        struct timeout sc_led_blink_to;
 
        struct task             init_task;
+       struct task             newstate_task;
+       enum ieee80211_state    ns_nstate;
+       int                     ns_arg;
 
        bus_space_tag_t sc_st;
        bus_space_handle_t sc_sh;

Reply via email to