On Thu, Dec 23, 2010 at 12:36:26AM +0000, Jacob Meuser wrote: > no feedback yet. anyone care to comment on this?
Sounds good to me. I can get odd things to happen when I unplug my ural AP, I just haven't had a chance to test this to see if it helps. I will try. It's Christmas after all. :-). .... Ken > > On Thu, Dec 16, 2010 at 12:28:56AM +0000, Jacob Meuser wrote: > > the following diff tries to make sure that no other processes/threads > > are or will be using the drivers software context when the driver is > > detached. > > > > this diff covers rum(4), run(4), ural(4) and urtw(4). without the > > diff, I can get the kernel to crash by starting a scan with > > the deice, then ejecting it while the scan is running. with this > > diff, I cannot get a crash. > > > > normally, usb device detach happens in the usb_task thread. the sole > > exception is when the usb bus itself is being detached. this happens > > with cardbus devices, and in that case, the usb device detach happens > > in the generic workq thread. > > > > this adds a simple reference counting/waiting mechanism. this is > > definitely needed for ioctl(2), which can sleep then start using the > > driver again. it may not be needed for timeout(9)s now, but maybe > > someday it will. another possible process using the device is the > > usb_task thread. in the normal case, this is the same process that > > is detaching, so there is no concurrency issue. there is already > > usb_rem_wait_task() to deal with detaches from other processes, > > because the bus driver needs it. > > > > this also adds more uses of usbd_is_dying() to check if the device > > has been detached: > > * before adding timeouts > > * when entering timeouts, usb_tasks, and ioctl > > > > also of note is the order things are done in the detach routines: > > 1) remove pending timeouts > > 2) remove (and possibly wait for) pending usb_tasks > > 3) wait for other processes > > 4) detach from the network stack > > 5) cleanup/free usb resources > > > > thoughts? ok? > > > > -- > > [email protected] > > SDF Public Access UNIX System - http://sdf.lonestar.org > > > > > > Index: if_ral.c > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/if_ral.c,v > > retrieving revision 1.117 > > diff -u -p -r1.117 if_ral.c > > --- if_ral.c 6 Dec 2010 04:41:39 -0000 1.117 > > +++ if_ral.c 15 Dec 2010 02:58:35 -0000 > > @@ -363,17 +363,20 @@ ural_detach(struct device *self, int fla > > > > s = splusb(); > > > > - if (ifp->if_softc != NULL) { > > - ieee80211_ifdetach(ifp); /* free all nodes */ > > - if_detach(ifp); > > - } > > - > > - usb_rem_task(sc->sc_udev, &sc->sc_task); > > if (timeout_initialized(&sc->scan_to)) > > timeout_del(&sc->scan_to); > > if (timeout_initialized(&sc->amrr_to)) > > timeout_del(&sc->amrr_to); > > > > + usb_rem_wait_task(sc->sc_udev, &sc->sc_task); > > + > > + usbd_ref_wait(sc->sc_udev); > > + > > + if (ifp->if_softc != NULL) { > > + ieee80211_ifdetach(ifp); /* free all nodes */ > > + if_detach(ifp); > > + } > > + > > if (sc->amrr_xfer != NULL) { > > usbd_free_xfer(sc->amrr_xfer); > > sc->amrr_xfer = NULL; > > @@ -547,8 +550,15 @@ ural_next_scan(void *arg) > > struct ieee80211com *ic = &sc->sc_ic; > > struct ifnet *ifp = &ic->ic_if; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > if (ic->ic_state == IEEE80211_S_SCAN) > > ieee80211_next_scan(ifp); > > + > > + usbd_ref_decr(sc->sc_udev); > > } > > > > void > > @@ -559,6 +569,9 @@ ural_task(void *arg) > > enum ieee80211_state ostate; > > struct ieee80211_node *ni; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > ostate = ic->ic_state; > > > > switch (sc->sc_state) { > > @@ -574,7 +587,8 @@ ural_task(void *arg) > > > > case IEEE80211_S_SCAN: > > ural_set_chan(sc, ic->ic_bss->ni_chan); > > - timeout_add_msec(&sc->scan_to, 200); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_msec(&sc->scan_to, 200); > > break; > > > > case IEEE80211_S_AUTH: > > @@ -1324,6 +1338,11 @@ ural_ioctl(struct ifnet *ifp, u_long cmd > > struct ifreq *ifr; > > int s, error = 0; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return ENXIO; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > s = splnet(); > > > > switch (cmd) { > > @@ -1387,6 +1406,8 @@ ural_ioctl(struct ifnet *ifp, u_long cmd > > > > splx(s); > > > > + usbd_ref_decr(sc->sc_udev); > > + > > return error; > > } > > > > @@ -2147,7 +2168,8 @@ ural_amrr_start(struct ural_softc *sc, s > > i--); > > ni->ni_txrate = i; > > > > - timeout_add_sec(&sc->amrr_to, 1); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_sec(&sc->amrr_to, 1); > > } > > > > void > > @@ -2157,6 +2179,11 @@ ural_amrr_timeout(void *arg) > > usb_device_request_t req; > > int s; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > s = splusb(); > > > > /* > > @@ -2174,6 +2201,8 @@ ural_amrr_timeout(void *arg) > > (void)usbd_transfer(sc->amrr_xfer); > > > > splx(s); > > + > > + usbd_ref_decr(sc->sc_udev); > > } > > > > void > > @@ -2203,7 +2232,8 @@ ural_amrr_update(usbd_xfer_handle xfer, > > > > ieee80211_amrr_choose(&sc->amrr, sc->sc_ic.ic_bss, &sc->amn); > > > > - timeout_add_sec(&sc->amrr_to, 1); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_sec(&sc->amrr_to, 1); > > } > > > > int > > Index: if_rum.c > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/if_rum.c,v > > retrieving revision 1.94 > > diff -u -p -r1.94 if_rum.c > > --- if_rum.c 6 Dec 2010 04:41:39 -0000 1.94 > > +++ if_rum.c 15 Dec 2010 02:58:35 -0000 > > @@ -460,17 +460,20 @@ rum_detach(struct device *self, int flag > > > > s = splusb(); > > > > - if (ifp->if_softc != NULL) { > > - ieee80211_ifdetach(ifp); /* free all nodes */ > > - if_detach(ifp); > > - } > > - > > - usb_rem_task(sc->sc_udev, &sc->sc_task); > > if (timeout_initialized(&sc->scan_to)) > > timeout_del(&sc->scan_to); > > if (timeout_initialized(&sc->amrr_to)) > > timeout_del(&sc->amrr_to); > > > > + usb_rem_wait_task(sc->sc_udev, &sc->sc_task); > > + > > + usbd_ref_wait(sc->sc_udev); > > + > > + if (ifp->if_softc != NULL) { > > + ieee80211_ifdetach(ifp); /* free all nodes */ > > + if_detach(ifp); > > + } > > + > > if (sc->amrr_xfer != NULL) { > > usbd_free_xfer(sc->amrr_xfer); > > sc->amrr_xfer = NULL; > > @@ -647,8 +650,12 @@ rum_next_scan(void *arg) > > if (usbd_is_dying(sc->sc_udev)) > > return; > > > > + usbd_ref_incr(sc->sc_udev); > > + > > if (ic->ic_state == IEEE80211_S_SCAN) > > ieee80211_next_scan(ifp); > > + > > + usbd_ref_decr(sc->sc_udev); > > } > > > > void > > @@ -676,7 +683,8 @@ rum_task(void *arg) > > > > case IEEE80211_S_SCAN: > > rum_set_chan(sc, ic->ic_bss->ni_chan); > > - timeout_add_msec(&sc->scan_to, 200); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_msec(&sc->scan_to, 200); > > break; > > > > case IEEE80211_S_AUTH: > > @@ -1350,6 +1358,11 @@ rum_ioctl(struct ifnet *ifp, u_long cmd, > > struct ifreq *ifr; > > int s, error = 0; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return ENXIO; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > s = splnet(); > > > > switch (cmd) { > > @@ -1413,6 +1426,8 @@ rum_ioctl(struct ifnet *ifp, u_long cmd, > > > > splx(s); > > > > + usbd_ref_decr(sc->sc_udev); > > + > > return error; > > } > > > > @@ -2236,7 +2251,8 @@ rum_amrr_start(struct rum_softc *sc, str > > i--); > > ni->ni_txrate = i; > > > > - timeout_add_sec(&sc->amrr_to, 1); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_sec(&sc->amrr_to, 1); > > } > > > > void > > @@ -2290,7 +2306,8 @@ rum_amrr_update(usbd_xfer_handle xfer, u > > > > ieee80211_amrr_choose(&sc->amrr, sc->sc_ic.ic_bss, &sc->amn); > > > > - timeout_add_sec(&sc->amrr_to, 1); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_sec(&sc->amrr_to, 1); > > } > > > > int > > Index: if_run.c > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/if_run.c,v > > retrieving revision 1.81 > > diff -u -p -r1.81 if_run.c > > --- if_run.c 6 Nov 2010 00:09:30 -0000 1.81 > > +++ if_run.c 15 Dec 2010 02:58:36 -0000 > > @@ -283,6 +283,7 @@ static const struct usb_devno run_devs[] > > int run_match(struct device *, void *, void *); > > void run_attach(struct device *, struct device *, void *); > > int run_detach(struct device *, int); > > +int run_activate(struct device *, int); > > int run_alloc_rx_ring(struct run_softc *); > > void run_free_rx_ring(struct run_softc *); > > int run_alloc_tx_ring(struct run_softc *, int); > > @@ -368,7 +369,8 @@ struct cfdriver run_cd = { > > }; > > > > const struct cfattach run_ca = { > > - sizeof (struct run_softc), run_match, run_attach, run_detach > > + sizeof (struct run_softc), run_match, run_attach, run_detach, > > + run_activate > > }; > > > > static const struct { > > @@ -590,17 +592,32 @@ run_detach(struct device *self, int flag > > struct ifnet *ifp = &sc->sc_ic.ic_if; > > int qid, s; > > > > - s = splnet(); > > - > > - /* wait for all queued asynchronous commands to complete */ > > - while (sc->cmdq.queued > 0) > > - tsleep(&sc->cmdq, 0, "cmdq", 0); > > + 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 */ > > +#if 0 > > + while (sc->cmdq.queued > 0) > > + tsleep(&sc->cmdq, 0, "cmdq", 0); > > +#endif > > + /* the async commands are run in a task */ > > + usb_rem_wait_task(sc->sc_udev, &sc->sc_task); > > + > > + /* but the task might not have run if it did not start before > > + * usbd_deactivate() was called, so wakeup now. we're > > + * detaching, no need to try to run more commands. > > + */ > > + if (sc->cmdq.queued > 0) { > > + sc->cmdq.queued = 0; > > + wakeup(&sc->cmdq); > > + } > > + > > + usbd_ref_wait(sc->sc_udev); > > + > > if (ifp->if_softc != NULL) { > > ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE); > > ieee80211_ifdetach(ifp); > > @@ -1437,8 +1454,15 @@ run_next_scan(void *arg) > > { > > struct run_softc *sc = arg; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > if (sc->sc_ic.ic_state == IEEE80211_S_SCAN) > > ieee80211_next_scan(&sc->sc_ic.ic_if); > > + > > + usbd_ref_decr(sc->sc_udev); > > } > > > > void > > @@ -1449,6 +1473,9 @@ run_task(void *arg) > > struct run_host_cmd *cmd; > > int s; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > /* process host commands */ > > s = splusb(); > > while (ring->next != ring->cur) { > > @@ -1472,6 +1499,9 @@ run_do_async(struct run_softc *sc, void > > struct run_host_cmd *cmd; > > int s; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > s = splusb(); > > cmd = &ring->cmd[ring->cur]; > > cmd->cb = cb; > > @@ -1530,7 +1560,8 @@ run_newstate_cb(struct run_softc *sc, vo > > > > case IEEE80211_S_SCAN: > > run_set_chan(sc, ic->ic_bss->ni_chan); > > - timeout_add_msec(&sc->scan_to, 200); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_msec(&sc->scan_to, 200); > > break; > > > > case IEEE80211_S_AUTH: > > @@ -1566,7 +1597,8 @@ run_newstate_cb(struct run_softc *sc, vo > > run_read_region_1(sc, RT2860_TX_STA_CNT0, > > (uint8_t *)sta, sizeof sta); > > /* start calibration timer */ > > - timeout_add_sec(&sc->calib_to, 1); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_sec(&sc->calib_to, 1); > > } > > > > /* turn link LED on */ > > @@ -1812,7 +1844,9 @@ run_calibrate_cb(struct run_softc *sc, v > > ieee80211_amrr_choose(&sc->amrr, sc->sc_ic.ic_bss, &sc->amn); > > splx(s); > > > > -skip: timeout_add_sec(&sc->calib_to, 1); > > +skip: > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_sec(&sc->calib_to, 1); > > } > > > > void > > @@ -2290,6 +2324,11 @@ run_ioctl(struct ifnet *ifp, u_long cmd, > > struct ifreq *ifr; > > int s, error = 0; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return ENXIO; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > s = splnet(); > > > > switch (cmd) { > > @@ -2352,6 +2391,8 @@ run_ioctl(struct ifnet *ifp, u_long cmd, > > > > splx(s); > > > > + usbd_ref_decr(sc->sc_udev); > > + > > return error; > > } > > > > @@ -3263,6 +3304,9 @@ run_init(struct ifnet *ifp) > > uint8_t bbp1, bbp3; > > int i, error, qid, ridx, ntries; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return ENXIO; > > + > > for (ntries = 0; ntries < 100; ntries++) { > > if ((error = run_read(sc, RT2860_ASIC_VER_ID, &tmp)) != 0) > > goto fail; > > @@ -3520,4 +3564,21 @@ run_stop(struct ifnet *ifp, int disable) > > for (qid = 0; qid < 4; qid++) > > run_free_tx_ring(sc, qid); > > run_free_rx_ring(sc); > > +} > > + > > +int > > +run_activate(struct device *self, int act) > > +{ > > + struct run_softc *sc = (struct run_softc *)self; > > + > > + switch (act) { > > + case DVACT_ACTIVATE: > > + break; > > + > > + case DVACT_DEACTIVATE: > > + usbd_deactivate(sc->sc_udev); > > + break; > > + } > > + > > + return 0; > > } > > Index: if_urtw.c > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/if_urtw.c,v > > retrieving revision 1.35 > > diff -u -p -r1.35 if_urtw.c > > --- if_urtw.c 6 Dec 2010 04:41:39 -0000 1.35 > > +++ if_urtw.c 15 Dec 2010 02:58:37 -0000 > > @@ -775,18 +775,21 @@ urtw_detach(struct device *self, int fla > > > > s = splusb(); > > > > - if (ifp->if_softc != NULL) { > > - ieee80211_ifdetach(ifp); /* free all nodes */ > > - if_detach(ifp); > > - } > > - > > - usb_rem_task(sc->sc_udev, &sc->sc_task); > > - usb_rem_task(sc->sc_udev, &sc->sc_ledtask); > > if (timeout_initialized(&sc->scan_to)) > > timeout_del(&sc->scan_to); > > if (timeout_initialized(&sc->sc_led_ch)) > > timeout_del(&sc->sc_led_ch); > > > > + usb_rem_wait_task(sc->sc_udev, &sc->sc_task); > > + usb_rem_wait_task(sc->sc_udev, &sc->sc_ledtask); > > + > > + usbd_ref_wait(sc->sc_udev); > > + > > + if (ifp->if_softc != NULL) { > > + ieee80211_ifdetach(ifp); /* free all nodes */ > > + if_detach(ifp); > > + } > > + > > /* abort and free xfers */ > > urtw_free_tx_data_list(sc); > > urtw_free_rx_data_list(sc); > > @@ -1913,7 +1916,8 @@ urtw_led_mode0(struct urtw_softc *sc, in > > URTW_LED_OFF : URTW_LED_ON; > > t.tv_sec = 0; > > t.tv_usec = 100 * 1000L; > > - timeout_add(&sc->sc_led_ch, tvtohz(&t)); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add(&sc->sc_led_ch, tvtohz(&t)); > > break; > > case URTW_LED_POWER_ON_BLINK: > > urtw_led_on(sc, URTW_LED_GPIO); > > @@ -2034,7 +2038,8 @@ urtw_led_blink(struct urtw_softc *sc) > > case URTW_LED_BLINK_NORMAL: > > t.tv_sec = 0; > > t.tv_usec = 100 * 1000L; > > - timeout_add(&sc->sc_led_ch, tvtohz(&t)); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add(&sc->sc_led_ch, tvtohz(&t)); > > break; > > default: > > panic("unknown LED status 0x%x", sc->sc_gpio_ledstate); > > @@ -2397,6 +2402,11 @@ urtw_ioctl(struct ifnet *ifp, u_long cmd > > struct ifreq *ifr; > > int s, error = 0; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return (ENXIO); > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > s = splnet(); > > > > switch (cmd) { > > @@ -2470,6 +2480,8 @@ urtw_ioctl(struct ifnet *ifp, u_long cmd > > > > splx(s); > > > > + usbd_ref_decr(sc->sc_udev); > > + > > return (error); > > } > > > > @@ -3513,8 +3525,15 @@ urtw_next_scan(void *arg) > > struct ieee80211com *ic = &sc->sc_ic; > > struct ifnet *ifp = &ic->ic_if; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > + usbd_ref_incr(sc->sc_udev); > > + > > if (ic->ic_state == IEEE80211_S_SCAN) > > ieee80211_next_scan(ifp); > > + > > + usbd_ref_decr(sc->sc_udev); > > } > > > > void > > @@ -3526,6 +3545,9 @@ urtw_task(void *arg) > > enum ieee80211_state ostate; > > usbd_status error = 0; > > > > + if (usbd_is_dying(sc->sc_udev)) > > + return; > > + > > ostate = ic->ic_state; > > > > switch (sc->sc_state) { > > @@ -3538,7 +3560,8 @@ urtw_task(void *arg) > > > > case IEEE80211_S_SCAN: > > urtw_set_chan(sc, ic->ic_bss->ni_chan); > > - timeout_add_msec(&sc->scan_to, 200); > > + if (!usbd_is_dying(sc->sc_udev)) > > + timeout_add_msec(&sc->scan_to, 200); > > break; > > > > case IEEE80211_S_AUTH: > > Index: usbdi.c > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/usbdi.c,v > > retrieving revision 1.41 > > diff -u -p -r1.41 usbdi.c > > --- usbdi.c 6 Dec 2010 04:25:27 -0000 1.41 > > +++ usbdi.c 15 Dec 2010 02:58:37 -0000 > > @@ -97,6 +97,26 @@ usbd_deactivate(usbd_device_handle dev) > > dev->dying = 1; > > } > > > > +void > > +usbd_ref_incr(usbd_device_handle dev) > > +{ > > + dev->ref_cnt++; > > +} > > + > > +void > > +usbd_ref_decr(usbd_device_handle dev) > > +{ > > + if (--dev->ref_cnt == 0 && dev->dying) > > + wakeup(&dev->ref_cnt); > > +} > > + > > +void > > +usbd_ref_wait(usbd_device_handle dev) > > +{ > > + while (dev->ref_cnt > 0) > > + tsleep(&dev->ref_cnt, PWAIT, "usbref", hz * 60); > > +} > > + > > static __inline int > > usbd_xfer_isread(usbd_xfer_handle xfer) > > { > > Index: usbdi.h > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/usbdi.h,v > > retrieving revision 1.36 > > diff -u -p -r1.36 usbdi.h > > --- usbdi.h 6 Dec 2010 04:25:27 -0000 1.36 > > +++ usbdi.h 15 Dec 2010 02:58:37 -0000 > > @@ -168,6 +168,10 @@ int usbd_ratecheck(struct timeval *last) > > int usbd_is_dying(usbd_device_handle); > > void usbd_deactivate(usbd_device_handle); > > > > +void usbd_ref_incr(usbd_device_handle); > > +void usbd_ref_decr(usbd_device_handle); > > +void usbd_ref_wait(usbd_device_handle); > > + > > /* An iterator for descriptors. */ > > typedef struct { > > const uByte *cur; > > Index: usbdivar.h > > =================================================================== > > RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v > > retrieving revision 1.40 > > diff -u -p -r1.40 usbdivar.h > > --- usbdivar.h 6 Dec 2010 04:25:27 -0000 1.40 > > +++ usbdivar.h 15 Dec 2010 02:58:37 -0000 > > @@ -125,7 +125,8 @@ struct usbd_bus { > > struct usbd_device { > > struct usbd_bus *bus; /* our controller */ > > struct usbd_pipe *default_pipe; /* pipe 0 */ > > - u_int8_t dying; /* removed */ > > + u_int8_t dying; /* hardware removed */ > > + u_int8_t ref_cnt; /* # of procs using device */ > > u_int8_t address; /* device address */ > > u_int8_t config; /* current configuration # */ > > u_int8_t depth; /* distance from root hub */ > > -- > [email protected] > SDF Public Access UNIX System - http://sdf.lonestar.org
