From owner-p4-projects@FreeBSD.ORG Mon Apr 23 19:19:22 2007 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 0EE2916A401; Mon, 23 Apr 2007 19:19:22 +0000 (UTC) X-Original-To: perforce@FreeBSD.org Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D8E9616A403 for ; Mon, 23 Apr 2007 19:19:21 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [69.147.83.41]) by mx1.freebsd.org (Postfix) with ESMTP id CA9AF13C45B for ; Mon, 23 Apr 2007 19:19:21 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.8/8.13.8) with ESMTP id l3NJJLew021424 for ; Mon, 23 Apr 2007 19:19:21 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.8/8.13.8/Submit) id l3NJJLg6021421 for perforce@freebsd.org; Mon, 23 Apr 2007 19:19:21 GMT (envelope-from hselasky@FreeBSD.org) Date: Mon, 23 Apr 2007 19:19:21 GMT Message-Id: <200704231919.l3NJJLg6021421@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 118675 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Apr 2007 19:19:22 -0000 http://perforce.freebsd.org/chv.cgi?CH=118675 Change 118675 by hselasky@hselasky_mini_itx on 2007/04/23 19:18:51 Make the USB stack smarter by allowing NULL pointer arguments to be passed to usbd_transfer_start() and usbd_transfer_stop(). At the same time add some more comments describing why. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#21 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#21 (text+ko) ==== @@ -383,7 +383,6 @@ while(n_setup--) { xfer = pxfer[n_setup]; - pxfer[n_setup] = NULL; if(xfer) { @@ -391,6 +390,27 @@ { mtx_lock(xfer->priv_mtx); + /* HINT: when you start/stop a transfer, + * it might be a good idea to directly + * use the "pxfer[]" structure: + * + * usbd_transfer_start(sc->pxfer[0]); + * usbd_transfer_stop(sc->pxfer[0]); + * + * That way, if your code has many parts + * that will not stop running under the + * same lock, in other words "priv_mtx", + * the usbd_transfer_start and + * usbd_transfer_stop functions will + * simply return when they detect a NULL + * pointer argument. + * + * To avoid any races we clear the "pxfer[]" + * pointer while holding the private mutex + * of the driver: + */ + pxfer[n_setup] = NULL; + usbd_transfer_stop(xfer); /* NOTE: default pipe does not @@ -400,6 +420,9 @@ xfer->pipe->refcount--; mtx_unlock(xfer->priv_mtx); + } else { + /* clear the transfer pointer */ + pxfer[n_setup] = NULL; } __callout_drain(&(xfer->timeout_handle)); @@ -423,6 +446,8 @@ if (info->setup_refcount == 0) { + /* wait for any USB callbacks to return */ + while (info->memory_refcount > 0) { error = msleep(info, info->usb_mtx, 0, "usb_mem_wait", 0); @@ -708,9 +733,11 @@ } /*---------------------------------------------------------------------------* - * usbd_transfer_start - start a USB transfer + * usbd_transfer_start - start an USB transfer * - * NOTE: this function can be called any number of times + * NOTE: Calling this function more than one time will only + * result in a single transfer start, until the USB transfer + * completes. * NOTE: if USBD_SYNCHRONOUS is set in "xfer->flags", then this * function will sleep for transfer completion * NOTE: if USBD_USE_POLLING is set in "xfer->flags", then this @@ -719,6 +746,11 @@ void usbd_transfer_start(struct usbd_xfer *xfer) { + if (xfer == NULL) { + /* transfer is gone */ + return; + } + mtx_assert(xfer->priv_mtx, MA_OWNED); if(!(xfer->flags & USBD_DEV_OPEN)) @@ -793,13 +825,19 @@ } /*---------------------------------------------------------------------------* - * usbd_transfer_stop - stop a USB transfer + * usbd_transfer_stop - stop an USB transfer * - * NOTE: this function can be called any number of times + * NOTE: Calling this function more than one time will only + * result in a single transfer stop. *---------------------------------------------------------------------------*/ void usbd_transfer_stop(struct usbd_xfer *xfer) { + if (xfer == NULL) { + /* transfer is gone */ + return; + } + mtx_assert(xfer->priv_mtx, MA_OWNED); if(xfer->flags & USBD_DEV_OPEN)