Date: Thu, 29 Mar 2007 13:51:19 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 116832 for review Message-ID: <200703291351.l2TDpJpB025569@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=116832 Change 116832 by hselasky@hselasky_mini_itx on 2007/03/29 13:50:38 Add more documentation to the config thread system. Make "usbd_config_td_stop()" lock the mutex it requires. Check if the config thread is gone in "usbd_config_td_queue_command()" before queueing a command. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/usb_subr.c#30 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/usb_subr.c#30 (text+ko) ==== @@ -2412,6 +2412,52 @@ break; } + /* NOTE to reimplementors: dequeueing a command from the + * "used" queue and executing it must be atomic, with + * regard to the "p_mtx" mutex. That means any attempt to + * queue a command by another thread must be blocked until + * either: + * + * 1) the command sleeps + * + * 2) the command returns + * + * Here is a practical example that shows how this + * helps solving a problem: + * + * Assume that you want to set the baud rate on a USB + * serial device. During the programming of the device you + * don't want to receive nor transmit any data, because it + * will be garbage most likely anyway. The programming of + * our USB device takes 20 milliseconds and it needs to + * call functions that sleep. + * + * Non-working solution: Before we queue the programming command, + * we stop transmission and reception of data. Then we + * queue a programming command. At the end of the programming + * command we enable transmission and reception of data. + * + * Problem: If a second programming command is queued + * while the first one is sleeping, we end up enabling + * transmission and reception of data too early. + * + * Working solution: Before we queue the programming + * command, we stop transmission and reception of + * data. Then we queue a programming command. Then we + * queue a second command that only enables transmission + * and reception of data. + * + * Why it works: If a second programming command is queued + * while the first one is sleeping, then the queueing of a + * second command to enable the data transfers, will cause + * the previous one, which is still on the queue, to be + * removed from the queue, and re-inserted after the last + * baud rate programming command, which then gives the + * desired result. + * + * This example assumes that you use a "qcount" of zero. + */ + USBD_IF_DEQUEUE(&(ctd->cmd_used), m); if (m) { @@ -2519,24 +2565,37 @@ void usbd_config_td_stop(struct usbd_config_td *ctd) { - register int error; + uint32_t level; + int error; - while (ctd->config_thread) { + if (ctd->p_mtx) { - mtx_assert(ctd->p_mtx, MA_OWNED); + mtx_lock(ctd->p_mtx); - ctd->flag_config_td_gone = 1; + while (ctd->config_thread) { usbd_config_td_queue_command(ctd, NULL, &usbd_config_td_dummy_cmd, 0, 0); + /* set the gone flag after queueing the + * last command: + */ + ctd->flag_config_td_gone = 1; + if (cold) { panic("%s:%d: cannot stop config thread!\n", __FUNCTION__, __LINE__); } + level = mtx_drop_recurse(ctd->p_mtx); + error = msleep(&(ctd->wakeup_config_td_gone), ctd->p_mtx, 0, "wait config TD", 0); + + mtx_pickup_recurse(ctd->p_mtx, level); + } + + mtx_unlock(ctd->p_mtx); } return; } @@ -2550,11 +2609,8 @@ void usbd_config_td_unsetup(struct usbd_config_td *ctd) { - if (ctd->p_mtx) { - mtx_lock(ctd->p_mtx); - usbd_config_td_stop(ctd); - mtx_unlock(ctd->p_mtx); - } + usbd_config_td_stop(ctd); + if (ctd->p_cmd_queue) { free(ctd->p_cmd_queue, M_DEVBUF); ctd->p_cmd_queue = NULL; @@ -2576,7 +2632,10 @@ struct usbd_mbuf *m; int32_t qlen; - mtx_assert(ctd->p_mtx, MA_OWNED); + if (usbd_config_td_is_gone(ctd)) { + /* nothing more to do */ + return; + } /* * first check if the command was
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200703291351.l2TDpJpB025569>