Date: Thu, 01 Sep 2011 16:53:37 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Hans Petter Selasky <hselasky@c2i.net> Cc: freebsd-arch@FreeBSD.org Subject: Re: skipping locks, mutex_owned, usb Message-ID: <4E5F8E61.7090701@FreeBSD.org> In-Reply-To: <201108312015.54587.hselasky@c2i.net> References: <201108312015.54587.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E5F8E61.7090701>