From owner-freebsd-arch@FreeBSD.ORG Thu Sep 1 13:53:42 2011 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D73B1106564A for ; Thu, 1 Sep 2011 13:53:42 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 289CB8FC0A for ; Thu, 1 Sep 2011 13:53:41 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id QAA03108; Thu, 01 Sep 2011 16:53:37 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4E5F8E61.7090701@FreeBSD.org> Date: Thu, 01 Sep 2011 16:53:37 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:5.0) Gecko/20110705 Thunderbird/5.0 MIME-Version: 1.0 To: Hans Petter Selasky References: <201108312015.54587.hselasky@c2i.net> In-Reply-To: <201108312015.54587.hselasky@c2i.net> X-Enigmail-Version: 1.2pre Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-arch@FreeBSD.org Subject: Re: skipping locks, mutex_owned, usb X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Sep 2011 13:53:42 -0000 on 31/08/2011 21:15 Hans Petter Selasky said the following: > What Warner says is correct. The code was written when SCHEDULER_STOPPED() was > not implemented. Really the usbd_transfer_poll should simply return if > SCHEDULER_STOPPED() is not set. Rather than simply return I would instead KASSERT that either kdb_active or SCHEDULER_STOPPED(). > And then those mtx_unlock()/mtx_lock() cases > can simply be removed. Yep. So for now I simply removed those: [wip] usb_transfer: adapt for SCHEDULER_STOPPED() - remove unneeded dropping of locks - check SCHEDULER_STOPPED along with mutex_owned() which may be unreliable after panic Maybe to do: KASSERT that usbd_transfer_poll is called only when kdb_active or SCHEDULER_STOPPED diff --git a/sys/dev/usb/usb_transfer.c b/sys/dev/usb/usb_transfer.c index d4c2408..ef50dc4 100644 --- a/sys/dev/usb/usb_transfer.c +++ b/sys/dev/usb/usb_transfer.c @@ -2150,7 +2150,7 @@ usbd_callback_wrapper(struct usb_xfer_queue *pq) struct usb_xfer_root *info = xfer->xroot; USB_BUS_LOCK_ASSERT(info->bus, MA_OWNED); - if (!mtx_owned(info->xfer_mtx)) { + if (!SCHEDULER_STOPPED() && !mtx_owned(info->xfer_mtx)) { /* * Cases that end up here: * @@ -3094,8 +3094,6 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max) struct usb_device *udev; struct usb_proc_msg *pm; uint16_t n; - uint16_t drop_bus; - uint16_t drop_xfer; for (n = 0; n != max; n++) { /* Extra checks to avoid panic */ @@ -3115,20 +3113,6 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max) if (udev->bus->methods->xfer_poll == NULL) continue; /* no poll method */ - /* make sure that the BUS mutex is not locked */ - drop_bus = 0; - while (mtx_owned(&xroot->udev->bus->bus_mtx)) { - mtx_unlock(&xroot->udev->bus->bus_mtx); - drop_bus++; - } - - /* make sure that the transfer mutex is not locked */ - drop_xfer = 0; - while (mtx_owned(xroot->xfer_mtx)) { - mtx_unlock(xroot->xfer_mtx); - drop_xfer++; - } - /* Make sure cv_signal() and cv_broadcast() is not called */ udev->bus->control_xfer_proc.up_msleep = 0; udev->bus->explore_proc.up_msleep = 0; @@ -3157,14 +3141,6 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max) (pm->pm_callback) (pm); USB_BUS_UNLOCK(xroot->bus); - - /* restore transfer mutex */ - while (drop_xfer--) - mtx_lock(xroot->xfer_mtx); - - /* restore BUS mutex */ - while (drop_bus--) - mtx_lock(&xroot->udev->bus->bus_mtx); } } -- Andriy Gapon