On Monday 11 May 2015 13:14:52 Alan Stern wrote:
> On Sun, 10 May 2015, Heiko Przybyl wrote:
> > Hello again.
> >
> > The issue still bites me with 4.0.2.
> >
> > Though I was able to narrow down the issue, it's still hard to
> > deterministically trigger it.
> >
> > Anyway. The setup:
> > - OHCI controller with hooked up keyboard
> > - xhddled (program that toggles scrolllock on disk activity)
> > - disk activity
> >
> > I'm very certain that it happens, when usbhid issues an urb for scrolllock
> > led toggling, the ohci driver adding a duplicate to its eds_in_use list.
> > That's why I hit it way more often with disk activity. I also tried to
> > alter the program to just periodically toggle the scrolllock, but that
> > didn't seem to help for reproducibility.
>
> As long as you have a way to trigger the problem, you're in a good
> position to track it down.
>
> > On Friday 23 January 2015 15:21:11 Alan Stern wrote:
> >
> > As suggested, checking the eds_in_use list was a good idea. Now, I'm even
> > more convinced duplicates are being added to the eds_in_use list.
>
> Your tests gave me a clue as to where the problem might be. It looks
> like finish_unlinks() changes ed->state to ED_IDLE too soon. The state
> should remain set to ED_UNLINK while finish_urb() is called.
>
> See if the patch below eliminates your problem.
Yes it indeed does eliminate the problem. Took some time to check if the
problem is really gone, though. Without the patch I get lockups up to and
including release 4.1; and not a single lockup with the patch applied.
So for the patch I'd say
Tested-By: Heiko Przybyl <lil_tux@xxxxxx>
Thanks for looking into and fixing this issue,
Heiko
>
> Alan Stern
>
>
>
> Index: usb-4.0/drivers/usb/host/ohci-q.c
> ===================================================================
> --- usb-4.0.orig/drivers/usb/host/ohci-q.c
> +++ usb-4.0/drivers/usb/host/ohci-q.c
> @@ -980,10 +980,6 @@ rescan_all:
> int completed, modified;
> __hc32 *prev;
>
> - /* Is this ED already invisible to the hardware? */
> - if (ed->state == ED_IDLE)
> - goto ed_idle;
> -
> /* only take off EDs that the HC isn't using, accounting for
> * frame counter wraps and EDs with partially retired TDs
> */
> @@ -1011,12 +1007,10 @@ skip_ed:
> }
>
> /* ED's now officially unlinked, hc doesn't see */
> - ed->state = ED_IDLE;
> ed->hwHeadP &= ~cpu_to_hc32(ohci, ED_H);
> ed->hwNextED = 0;
> wmb();
> ed->hwINFO &= ~cpu_to_hc32(ohci, ED_SKIP | ED_DEQUEUE);
> -ed_idle:
>
> /* reentrancy: if we drop the schedule lock, someone might
> * have modified this list. normally it's just prepending
> @@ -1087,6 +1081,7 @@ rescan_this:
> if (list_empty(&ed->td_list)) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> + ed->state = ED_IDLE;
> list_del(&ed->in_use_list);
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> *last = ed->ed_next;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
