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:
> > On Tue, 20 Jan 2015, Heiko Przybyl wrote:
> > > I'm not 100% sure, but then it's probably a race between urb
> > > enqueuing (duplicates?) and the watchdog orphan cleanup.
> > >
> > > The crash log already shows the double list add in ohci_urb_enqueue
> > > "
> > > ohci-hcd.c:238: list_add(&ed->in_use_list, &ohci->eds_in_use);
> > > "
> > > This is probably due to the ed returned by ed_get() being reused before
> > > the
> > > watchdog ran, thus the same in_use_list re-added to ohci.eds_in_use.
> >
> > But this won't happen unless ed->state was ED_IDLE; and then
> > ed_schedule() changes ed->state to ED_OPER before ed is added to the
> > in_use_list.
>
> 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.
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
