Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 4 Aug 2006 11:58:59 GMT
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 103161 for review
Message-ID:  <200608041158.k74BwxRD072494@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=103161

Change 103161 by hselasky@hselasky_mini_itx on 2006/08/04 11:58:01

	Improve the config thread layer by not allowing "kthread_exit()"
	calls from subroutines. When the thread is being torn down, sleep
	calls and USB request calls should just return immediately and
	perform no operation. To check if the config thread is gone, 
	there is a new function, usbd_config_td_is_gone().

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/if_ural.c#8 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_subr.c#13 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_subr.h#18 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/if_ural.c#8 (text+ko) ====

@@ -627,19 +627,24 @@
 ural_cfg_do_request(struct ural_softc *sc, usb_device_request_t *req, 
 		    void *data)
 {
+	u_int16_t length;
 	usbd_status err;
 
+	if (usbd_config_td_is_gone(&(sc->sc_config_td))) {
+	    goto error;
+	}
+
 	err = usbd_do_request_flags_mtx(sc->sc_udev, &(sc->sc_mtx), req, 
 					data, 0, NULL, 1000);
 
-	usbd_config_td_check_gone(&(sc->sc_config_td));
-
 	if (err) {
-	    u_int16_t length = UGETW(req->wLength);
 
 	    printf("%s: device request failed, err=%s "
 		   "(ignored)\n", sc->sc_name, usbd_errstr(err));
 
+	error:
+	    length = UGETW(req->wLength);
+
 	    if ((req->bmRequestType & UT_READ) && length) {
 	        bzero(data, length);
 	    }
@@ -1962,10 +1967,12 @@
 	    while (sc->sc_flags & (URAL_FLAG_SEND_BYTE_FRAME|
 				   URAL_FLAG_SEND_BCN_FRAME)) {
 
+		if (usbd_config_td_is_gone(&(sc->sc_config_td))) {
+		    break;
+		}
+
 	        error = msleep(&(sc->sc_wakeup_bcn), &(sc->sc_mtx), 
 			       0, "ural beacon sleep", 0);
-
-		usbd_config_td_check_gone(&(sc->sc_config_td));
 	    }
 	}
 	return;
@@ -2471,7 +2478,9 @@
 	        if (ural_cfg_bbp_read(sc, RAL_BBP_VERSION) != 0) {
 		    break;
 		}
-		usbd_config_td_sleep(&(sc->sc_config_td), hz/100);
+		if (usbd_config_td_sleep(&(sc->sc_config_td), hz/100)) {
+		    break;
+		}
 	    } else {
 	        printf("%s: timeout waiting for BBP\n",
 		       sc->sc_name);
@@ -2556,7 +2565,9 @@
 		      (RAL_BBP_AWAKE | RAL_RF_AWAKE)) {
 		      break;
 		  }
-		  usbd_config_td_sleep(&(sc->sc_config_td), hz/100);
+		  if (usbd_config_td_sleep(&(sc->sc_config_td), hz/100)) {
+		      break;
+		  }
 	    } else {
 	        printf("%s: timeout waiting for "
 		       "BBP/RF to wakeup\n", sc->sc_name);

==== //depot/projects/usb/src/sys/dev/usb/usb_subr.c#13 (text+ko) ====

@@ -2366,7 +2366,9 @@
 
 	while(1) {
 
-	    usbd_config_td_check_gone(ctd);
+	    if (ctd->flag_config_td_gone) {
+	        break;
+	    }
 
 	    USBD_IF_DEQUEUE(&(ctd->cmd_used), m);
 
@@ -2396,6 +2398,8 @@
 
 	ctd->config_thread = NULL;
 
+	wakeup(&(ctd->wakeup_config_td_gone));
+
 	mtx_unlock(ctd->p_mtx);
 
 	kthread_exit(0);
@@ -2591,41 +2595,39 @@
 }
 
 /*---------------------------------------------------------------------------*
- * usbd_config_td_check_gone
+ * usbd_config_td_is_gone
  *
- * NOTE: this function can only be called from the config thread
+ * Return values:
+ *    0: config thread is running
+ * else: config thread is gone
  *---------------------------------------------------------------------------*/
-void
-usbd_config_td_check_gone(struct usbd_config_td *ctd)
+u_int8_t
+usbd_config_td_is_gone(struct usbd_config_td *ctd)
 {
-	u_int32_t level;
-
 	mtx_assert(ctd->p_mtx, MA_OWNED);
 
-	if (ctd->flag_config_td_gone) {
-
-	    ctd->config_thread = NULL;
-
-	    wakeup(&(ctd->wakeup_config_td_gone));
-
-	    level = mtx_drop_recurse(ctd->p_mtx);
-
-	    mtx_unlock(ctd->p_mtx);
-
-	    kthread_exit(0);
-	}
-	return;
+	return ctd->flag_config_td_gone ? 1 : 0;
 }
 
 /*---------------------------------------------------------------------------*
  * usbd_config_td_sleep
  *
  * NOTE: this function can only be called from the config thread
+ *
+ * Return values:
+ *    0: normal delay
+ * else: config thread is gone
  *---------------------------------------------------------------------------*/
-void
+u_int8_t
 usbd_config_td_sleep(struct usbd_config_td *ctd, u_int32_t timeout)
 {
 	register int error;
+	u_int8_t is_gone = usbd_config_td_is_gone(ctd);
+	u_int32_t level;
+
+	if (is_gone) {
+	    goto done;
+	}
 
 	if (timeout == 0) {
 	    /* zero means no timeout, 
@@ -2635,12 +2637,15 @@
 	    timeout = 1;
 	}
 
+	level = mtx_drop_recurse(ctd->p_mtx);
+
 	error = msleep(ctd, ctd->p_mtx, 0, 
 		       "config td sleep", timeout);
 
-	usbd_config_td_check_gone(ctd);
+	mtx_pickup_recurse(ctd->p_mtx, level);
 
-	return;
+ done:
+	return is_gone;
 }
 
 /*---------------------------------------------------------------------------*

==== //depot/projects/usb/src/sys/dev/usb/usb_subr.h#18 (text+ko) ====

@@ -769,10 +769,10 @@
 usbd_config_td_queue_command(struct usbd_config_td *ctd,
 			     usbd_config_td_command_t *command_func,
 			     u_int16_t command_ref);
-void
-usbd_config_td_check_gone(struct usbd_config_td *ctd);
+u_int8_t
+usbd_config_td_is_gone(struct usbd_config_td *ctd);
 
-void
+u_int8_t
 usbd_config_td_sleep(struct usbd_config_td *ctd, u_int32_t timeout);
 
 struct mbuf *



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200608041158.k74BwxRD072494>