Date: Thu, 18 Sep 2008 20:46:40 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 150048 for review Message-ID: <200809182046.m8IKkeal035399@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=150048 Change 150048 by hselasky@hselasky_laptop001 on 2008/09/18 20:45:48 Fix a synchronisation issue in USB WLAN drivers. Affected files ... .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.c#9 edit .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.h#4 edit .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_process.c#9 edit .. //depot/projects/usb/src/sys/dev/usb2/wlan/if_rum2.c#13 edit .. //depot/projects/usb/src/sys/dev/usb2/wlan/if_ural2.c#13 edit .. //depot/projects/usb/src/sys/dev/usb2/wlan/if_zyd2.c#14 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.c#9 (text+ko) ==== @@ -33,6 +33,8 @@ #include <dev/usb2/core/usb2_config_td.h> #include <dev/usb2/core/usb2_debug.h> +static void usb2_config_td_sync_cb(struct usb2_config_td_softc *sc, struct usb2_config_td_cc *cc, uint16_t ref); + static void usb2_config_td_dispatch(struct usb2_proc_msg *pm) { @@ -146,21 +148,23 @@ * usb2_config_td_queue_command * * This function will enter a command into the config thread queue for - * execution. The "command_qcount" gives the maximum number of - * equivalent commands that will be kept on the queue before queueing - * the next command. "command_ref" is the reference count for the - * current command which is passed on to the "command_post_func" + * execution. The "command_sync" field was previously used to indicate + * the queue count which is now fixed at two elements. If the + * "command_sync" field is equal to "USB2_CONFIG_TD_SYNC" the command + * will be executed synchronously from the config thread. The + * "command_ref" argument is the reference count for the current + * command which is passed on to the "command_post_func" * function. This parameter can be used to make a command * unique. "command_pre_func" is called from this function when we * have the final queue element. "command_post_func" is called from * the USB config thread when the command reaches the beginning of the - * USB config thread queue. + * USB config thread queue. This function must be called locked. *------------------------------------------------------------------------*/ void usb2_config_td_queue_command(struct usb2_config_td *ctd, usb2_config_td_command_t *command_pre_func, usb2_config_td_command_t *command_post_func, - uint16_t command_qcount, + uint16_t command_sync, uint16_t command_ref) { struct usb2_config_td_item *pi; @@ -227,6 +231,10 @@ if (command_pre_func) { (command_pre_func) (ctd->p_softc, (void *)(pi + 1), command_ref); } + + if (command_sync == USB2_CONFIG_TD_SYNC) { + usb2_proc_mwait(&ctd->usb2_proc, pi_0, pi_1); + } return; } @@ -278,3 +286,38 @@ done: return (is_gone); } + +/*------------------------------------------------------------------------* + * usb2_config_td_sync + * + * This function will wait until all commands have been executed on + * the config thread. This function must be called locked and can + * sleep. + * + * Return values: + * 0: success + * Else: config thread is gone + *------------------------------------------------------------------------*/ +uint8_t +usb2_config_td_sync(struct usb2_config_td *ctd) +{ + if (usb2_config_td_is_gone(ctd)) { + return (1); + } + + usb2_config_td_queue_command(ctd, NULL, + &usb2_config_td_sync_cb, USB2_CONFIG_TD_SYNC, 0); + + if (usb2_config_td_is_gone(ctd)) { + return (1); + } + + return (0); +} + +static void +usb2_config_td_sync_cb(struct usb2_config_td_softc *sc, + struct usb2_config_td_cc *cc, uint16_t ref) +{ + return; +} ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_config_td.h#4 (text+ko) ==== @@ -30,6 +30,8 @@ struct usb2_config_td_softc; struct usb2_config_td_cc; +#define USB2_CONFIG_TD_SYNC 0xFFFF /* magic value */ + typedef void (usb2_config_td_command_t)(struct usb2_config_td_softc *sc, struct usb2_config_td_cc *cc, uint16_t reference); typedef void (usb2_config_td_end_of_commands_t)(struct usb2_config_td_softc *sc); @@ -61,8 +63,9 @@ uint8_t usb2_config_td_setup(struct usb2_config_td *ctd, void *priv_sc, struct mtx *priv_mtx, usb2_config_td_end_of_commands_t *p_func_eoc, uint16_t item_size, uint16_t item_count); void usb2_config_td_drain(struct usb2_config_td *ctd); void usb2_config_td_unsetup(struct usb2_config_td *ctd); -void usb2_config_td_queue_command(struct usb2_config_td *ctd, usb2_config_td_command_t *pre_func, usb2_config_td_command_t *post_func, uint16_t command_qcount, uint16_t command_ref); +void usb2_config_td_queue_command(struct usb2_config_td *ctd, usb2_config_td_command_t *pre_func, usb2_config_td_command_t *post_func, uint16_t command_sync, uint16_t command_ref); uint8_t usb2_config_td_is_gone(struct usb2_config_td *ctd); uint8_t usb2_config_td_sleep(struct usb2_config_td *ctd, uint32_t timeout); +uint8_t usb2_config_td_sync(struct usb2_config_td *ctd); #endif /* _USB2_CONFIG_TD_H_ */ ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_process.c#9 (text+ko) ==== @@ -147,6 +147,7 @@ continue; } + /* end if messages - check if anyone is waiting for sync */ if (up->up_dsleep) { up->up_dsleep = 0; usb2_cv_broadcast(&up->up_drain); @@ -328,7 +329,8 @@ * usb2_proc_mwait * * This function will return when the USB process message pointed to - * by "pm" is no longer on a queue. + * by "pm" is no longer on a queue. This function must be called + * having "up->up_mtx" locked. *------------------------------------------------------------------------*/ void usb2_proc_mwait(struct usb2_process *up, void *_pm0, void *_pm1) @@ -336,7 +338,8 @@ struct usb2_proc_msg *pm0 = _pm0; struct usb2_proc_msg *pm1 = _pm1; - mtx_lock(up->up_mtx); + mtx_assert(up->up_mtx, MA_OWNED); + if (up->up_curtd == curthread) { /* Just remove the messages from the queue. */ if (pm0->pm_qentry.tqe_prev) { @@ -347,12 +350,14 @@ TAILQ_REMOVE(&up->up_qhead, pm1, pm_qentry); pm1->pm_qentry.tqe_prev = NULL; } - } else if (pm0->pm_qentry.tqe_prev || + } else while (pm0->pm_qentry.tqe_prev || pm1->pm_qentry.tqe_prev) { + /* check if config thread is gone */ + if (up->up_gone) + break; up->up_dsleep = 1; usb2_cv_wait(&up->up_drain, up->up_mtx); } - mtx_unlock(up->up_mtx); return; } @@ -390,11 +395,6 @@ up->up_csleep = 0; usb2_cv_signal(&up->up_cv); } - /* Check if someone is waiting - should not happen */ - - if (up->up_dsleep) { - printf("WARNING: Someone is waiting for USB process drain!\n"); - } /* Check if we are still cold booted */ if (cold) { @@ -404,6 +404,14 @@ } usb2_cv_wait(&up->up_cv, up->up_mtx); } + /* Check if someone is waiting - should not happen */ + + if (up->up_dsleep) { + up->up_dsleep = 0; + usb2_cv_broadcast(&up->up_drain); + DPRINTF("WARNING: Someone is waiting " + "for USB process drain!\n"); + } mtx_unlock(up->up_mtx); return; } ==== //depot/projects/usb/src/sys/dev/usb2/wlan/if_rum2.c#13 (text+ko) ==== @@ -2559,6 +2559,16 @@ { struct rum_vap *rvp; struct ieee80211vap *vap; + struct rum_softc *sc = ic->ic_ifp->if_softc; + + /* Need to sync with config thread: */ + mtx_lock(&sc->sc_mtx); + if (usb2_config_td_sync(&sc->sc_config_td)) { + mtx_unlock(&sc->sc_mtx); + /* config thread is gone */ + return (NULL); + } + mtx_unlock(&sc->sc_mtx); if (!TAILQ_EMPTY(&ic->ic_vaps)) /* only one at a time */ return NULL; @@ -2592,6 +2602,14 @@ rum_vap_delete(struct ieee80211vap *vap) { struct rum_vap *rvp = RUM_VAP(vap); + struct rum_softc *sc = vap->iv_ic->ic_ifp->if_softc; + + /* Need to sync with config thread: */ + mtx_lock(&sc->sc_mtx); + if (usb2_config_td_sync(&sc->sc_config_td)) { + /* ignore */ + } + mtx_unlock(&sc->sc_mtx); ieee80211_amrr_cleanup(&rvp->amrr); ieee80211_vap_detach(vap); ==== //depot/projects/usb/src/sys/dev/usb2/wlan/if_ural2.c#13 (text+ko) ==== @@ -2351,6 +2351,16 @@ { struct ural_vap *uvp; struct ieee80211vap *vap; + struct ural_softc *sc = ic->ic_ifp->if_softc; + + /* Need to sync with config thread: */ + mtx_lock(&sc->sc_mtx); + if (usb2_config_td_sync(&sc->sc_config_td)) { + mtx_unlock(&sc->sc_mtx); + /* config thread is gone */ + return (NULL); + } + mtx_unlock(&sc->sc_mtx); if (!TAILQ_EMPTY(&ic->ic_vaps)) /* only one at a time */ return NULL; @@ -2385,6 +2395,14 @@ ural_vap_delete(struct ieee80211vap *vap) { struct ural_vap *uvp = URAL_VAP(vap); + struct ural_softc *sc = vap->iv_ic->ic_ifp->if_softc; + + /* Need to sync with config thread: */ + mtx_lock(&sc->sc_mtx); + if (usb2_config_td_sync(&sc->sc_config_td)) { + /* ignore */ + } + mtx_unlock(&sc->sc_mtx); ieee80211_amrr_cleanup(&uvp->amrr); ieee80211_vap_detach(vap); ==== //depot/projects/usb/src/sys/dev/usb2/wlan/if_zyd2.c#14 (text+ko) ==== @@ -3003,6 +3003,16 @@ { struct zyd_vap *zvp; struct ieee80211vap *vap; + struct zyd_softc *sc = ic->ic_ifp->if_softc; + + /* Need to sync with config thread: */ + mtx_lock(&sc->sc_mtx); + if (usb2_config_td_sync(&sc->sc_config_td)) { + mtx_unlock(&sc->sc_mtx); + /* config thread is gone */ + return (NULL); + } + mtx_unlock(&sc->sc_mtx); if (!TAILQ_EMPTY(&ic->ic_vaps)) /* only one at a time */ return NULL; @@ -3034,6 +3044,14 @@ zyd_vap_delete(struct ieee80211vap *vap) { struct zyd_vap *zvp = ZYD_VAP(vap); + struct zyd_softc *sc = vap->iv_ic->ic_ifp->if_softc; + + /* Need to sync with config thread: */ + mtx_lock(&sc->sc_mtx); + if (usb2_config_td_sync(&sc->sc_config_td)) { + /* ignore */ + } + mtx_unlock(&sc->sc_mtx); ieee80211_amrr_cleanup(&zvp->amrr); ieee80211_vap_detach(vap);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200809182046.m8IKkeal035399>