2015-10-03 22:33 GMT+02:00 Stefan Sperling <[email protected]>: > On Sun, Sep 27, 2015 at 08:00:19PM +0200, Stefan Sperling wrote: >> This is yet another attempt at improving the iwm(4) newstate task. > > This diff has been working nicely for me, with many suspend/resume cycles. > Never had a problem connecting to several wifis. > > Any objections? Any Oks? > >> 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;
No issues here so far, and "ifconfig iwm0 down; ifconfig iwm0 up" now makes it really up - previously it required a second one "up" call. -- WBR, Vadim Zhukov
