USB/coredump hangs in 8 and 9

[Andriy Gapon]

on 19/08/2011 00:24 Hans Petter Selasky said the following:
> On Thursday 18 August 2011 19:04:10 Andriy Gapon wrote:
>> If you can help Hans to figure out what you is wrong with USB subsystem in
>> this respect that would help us all.
> 
> Hi,
> 
> usb_busdma.c:   /* we use "mtx_owned()" instead of this function */
> usb_busdma.c:   owned = mtx_owned(uptag->mtx);
> usb_compat_linux.c:     do_unlock = mtx_owned(&Giant) ? 0 : 1;
> usb_compat_linux.c:     do_unlock = mtx_owned(&Giant) ? 0 : 1;
> usb_compat_linux.c:     do_unlock = mtx_owned(&Giant) ? 0 : 1;
> usb_hub.c:      if (mtx_owned(&bus->bus_mtx)) {
> usb_transfer.c: if (!mtx_owned(info->xfer_mtx)) {
> usb_transfer.c: if (mtx_owned(xfer->xroot->xfer_mtx)) {
> usb_transfer.c:         while (mtx_owned(&xroot->udev->bus->bus_mtx)) {
> usb_transfer.c:         while (mtx_owned(xroot->xfer_mtx)) {
> 
> One fix you will need to do, if mtx_owned is not giving correct value is:

First, could you please clarify what is the correct, or rather - expected, value
in this case.  It's not immediately clear to me if we should consider all locks as
owned or un-owned in a situation where all locks are actually skipped behind the
scenes.
Maybe USB code should explicitly check for that condition as to not make any
unsafe assumptions.

Second, it's not clear to me what the above list actually represents in the
context of this discussion.

> static void
> usbd_callback_wrapper(struct usb_xfer_queue *pq)
> {
>         struct usb_xfer *xfer = pq->curr;
>         struct usb_xfer_root *info = xfer->xroot;
> 
>         USB_BUS_LOCK_ASSERT(info->bus, MA_OWNED);
>         if (!mtx_owned(info->xfer_mtx)) {
> 
> The above "if" should be anded with && !paniced && !dumping ... or maybe the 
> new not scheduling variable is good for this purpose?
> 
>                 /*
>                  * Cases that end up here:
>                  *
> 
> #if USB_HAVE_BUSDMA
>         if (mtx_owned(xfer->xroot->xfer_mtx)) {
>                 struct usb_xfer_queue *pq;
> 
> 
> This case is more like a BUS-DMA error case, and is not so important to 
> execute.
> 
> --HPS


-- 
Andriy Gapon