Re: better run(4) fix (was: run(4): fix crash in run_task())

[Stefan Sperling]

On Sun, Jul 12, 2015 at 09:11:44PM +0200, Stefan Sperling wrote:
> On Sun, Jul 12, 2015 at 05:57:14PM +0200, Martin Pieuchot wrote:
> > > > run_newstate_cb SCAN -> INIT
> > > > run_newstate_cb SCAN -> SCAN
> > 
> > How is this possible?  Why it isn't INIT -> SCAN?
> 
> I'm not entirely sure.

This implements your suggestion to properly abort async tasks when
bringing the interface down.

Fixes the problem for me just as well.

ok?

Index: if_run.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_run.c,v
retrieving revision 1.109
diff -u -p -r1.109 if_run.c
--- if_run.c    12 Jun 2015 15:47:31 -0000      1.109
+++ if_run.c    15 Jul 2015 03:50:08 -0000
@@ -358,6 +358,7 @@ struct              ieee80211_node *run_node_alloc(s
 int            run_media_change(struct ifnet *);
 void           run_next_scan(void *);
 void           run_task(void *);
+void           run_cancel_async(struct run_softc *);
 void           run_do_async(struct run_softc *, void (*)(struct run_softc *,
                    void *), void *, int);
 int            run_newstate(struct ieee80211com *, enum ieee80211_state, int);
@@ -685,13 +686,10 @@ run_detach(struct device *self, int flag
 
        s = splusb();
 
-       if (timeout_initialized(&sc->scan_to))
-               timeout_del(&sc->scan_to);
        if (timeout_initialized(&sc->calib_to))
                timeout_del(&sc->calib_to);
 
-       /* wait for all queued asynchronous commands to complete */
-       usb_rem_wait_task(sc->sc_udev, &sc->sc_task);
+       run_cancel_async(sc);
 
        usbd_ref_wait(sc->sc_udev);
 
@@ -1740,6 +1738,28 @@ run_task(void *arg)
 }
 
 void
+run_cancel_async(struct run_softc *sc)
+{
+       struct run_host_cmd_ring *ring = &sc->cmdq;
+       struct run_host_cmd *cmd;
+       int s, i;
+
+       s = splusb();
+
+       if (timeout_initialized(&sc->scan_to))
+               timeout_del(&sc->scan_to);
+       usb_rem_task(sc->sc_udev, &sc->sc_task);
+
+       for (i = 0; i < RUN_HOST_CMD_RING_COUNT; i++) {
+               cmd = &ring->cmd[i];
+               cmd->cb = NULL;
+       }
+       ring->next = ring->cur = ring->queued = 0;
+
+       splx(s);
+}
+
+void
 run_do_async(struct run_softc *sc, void (*cb)(struct run_softc *, void *),
     void *arg, int len)
 {
@@ -1751,6 +1771,11 @@ run_do_async(struct run_softc *sc, void 
                return;
 
        s = splusb();
+       if (ring->queued == RUN_HOST_CMD_RING_COUNT) {
+               splx(s);
+               printf("%s: host cmd queue overrun\n", sc->sc_dev.dv_xname);
+               return; /* XXX */
+       }
        cmd = &ring->cmd[ring->cur];
        cmd->cb = cb;
        KASSERT(len <= sizeof (cmd->data));
@@ -4504,7 +4529,9 @@ run_init(struct ifnet *ifp)
        }
 
        /* init host command ring */
-       sc->cmdq.cur = sc->cmdq.next = sc->cmdq.queued = 0;
+       if (sc->cmdq.queued != 0)
+               panic("outstanding host commands queued");
+       sc->cmdq.cur = sc->cmdq.next = 0;
 
        /* init Tx rings (4 EDCAs) */
        for (qid = 0; qid < 4; qid++) {
@@ -4735,13 +4762,16 @@ run_stop(struct ifnet *ifp, int disable)
        ifp->if_timer = 0;
        ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
 
-       timeout_del(&sc->scan_to);
        timeout_del(&sc->calib_to);
 
        s = splusb();
+
+       /* Cancel asynchronous state transitions etc. */
+       run_cancel_async(sc);
        ieee80211_new_state(ic, IEEE80211_S_INIT, -1);
-       /* wait for all queued asynchronous commands to complete */
+       /* Wait for asynchronous state transition to INIT to complete. */
        usb_wait_task(sc->sc_udev, &sc->sc_task);
+
        splx(s);
 
        /* Disable Tx/Rx DMA. */