On 2018-05-22 15:14:17 [-0400], Alan Stern wrote:
> On Tue, 22 May 2018, Sebastian Andrzej Siewior wrote:
>
> > On 2018-05-07 11:37:29 [-0400], Alan Stern wrote:
> > > > As far as I understand it there should be no deadlock. Without the
> > > > local_irq_save() things should not deadlock because the HCD invokes USB
> > > > driver's (usb-storage for instance) ->complete callback always in the
> > > > same way. If you mix the usb-driver with two different HCDs (say ehci
> > > > and xhci) then lockdep would complain. However, the locks which were
> > > > allocated by usb-storage for the ehci driver would never be used in the
> > > > context of the xhci driver. So lockdep would report a deacklock but
> > > > there wouldn't be one (again, if I got the piece right).
> > >
> > > That argument would be correct if the completion routines were the only
> > > places where the higher-level drivers (such as usb-storage or usbhid)
> > > acquired their locks. But we can't depend on this being true; you
> > > would have to audit each USB driver to make sure.
> >
> > an entry point for IRQ usage outside of the driver would be the usage of
> > hrtimer.
>
> Sorry, I don't understand that sentence at all. And I don't see how it
> could be relevant to the point I was trying to make.
>
> Consider, for example, drivers/hid/usbhid/hid-core.c. In that file,
> hid_io_error() is called by hid_irq_in(), which is an URB completion
> handler. hid_io_error() acquires usbhid_lock. Therefore it would be
> necessary to audit the usbhid driver to see whether interrupts are
> enabled or disabled any place where usbhid_lock is acquired. And in
> fact, hid_ctrl() (another completion handler) calls
> spin_lock(&usbhid->lock) -- this could cause problems for you. And
> usbhid->lock is acquired in other places, ones that are not inside
> completion handlers.
>
> None of this has anything to do with IRQ usage or hrtimers.
Yeah, I get what you mean.
> > You mean the "Reserved Bandwidth Transfers:" paragraph?
>
> Paragraphs (plural). Three paragraphs, to be precise.
>
> > > It's possible to rewrite the HCDs not to rely on this (I did exactly
> > > that for ehci-hcd), but it is a nontrivial job.
> >
> > are you referring to commit 9118f9eb4f1e ("USB: EHCI: improve interrupt
> > qh unlink")?
>
> That one, plus:
>
> 46c73d1d3ebc ("USB: EHCI: handle isochronous underruns with tasklets")
> e4e18cbd52c8 ("USB: EHCI: code rearrangement in iso_stream_schedule()")
> 24f531371de1 ("USB: EHCI: accept very late isochronous URBs")
> 35371e4fbc3e ("USB: EHCI: improve ehci_endpoint_disable")
>
> Not all parts of all those commits were relevant, but as far as I
> recall, they each contributed something. And I may have omitted
> one or two commits by mistake.
Thank you, let me look at those so I can see what is needed…
> Alan Stern
Sebastian
--
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
