On 2018-05-04 16:07:22 [-0400], Alan Stern wrote:
> On Fri, 4 May 2018, Sebastian Andrzej Siewior wrote:
>
> > Hi Alan,
> >
> > I posted a RFC [0] to let the HCD complete the URB in threaded-IRQ
> > context to avoid the softirq/tasklet. Mauro reported that this does not
> > help him and never came back after that.
> > You did not oppose the approach as long as there is no performance issue
> > which I did not see. Would you mind if I revisit the approach and
> > convert all HCDs to threaded IRQs while at it?
>
> I don't mind revisiting the approach, although it certainly would be
> good to find out why it doesn't help Mauro's video problem.
>
> However, converting _all_ the HCDs to threaded IRQs is a different
> story. It should be okay to convert the ones that currently use
> tasklets, but I can't approve changes to all the others. Only the
> drivers that I maintain (ohci-hcd and uhci-hcd).
Sure. The plan was to convert all drivers to one model so we could get
rid of:
|static void __usb_hcd_giveback_urb(struct urb *urb)
|{
|…
| /*
| * We disable local IRQs here avoid possible deadlock because
| * drivers may call spin_lock() to hold lock which might be
| * acquired in one hard interrupt handler.
| *
| * The local_irq_save()/local_irq_restore() around complete()
| * will be removed if current USB drivers have been cleaned up
| * and no one may trigger the above deadlock situation when
| * running complete() in tasklet.
| */
| local_irq_save(flags);
| urb->complete(urb);
| local_irq_restore(flags);
|…
|}
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).
And I was thinking about converting all drivers to one model and then we
could get rid of the block I quoted above.
If nobody rejects the approach as such I would go and start hacking…
> And even for those two, the conversion will not be easy. Simply
> changing the giveback routines would violate the documented guarantees
> for isochronous transfers.
The requirement was that the ISO urb is completed before the BULK urb,
right?
If so, the last patch would enqueue the ISO urb and wait until the BULK
urb was completed before it completed the ISO urb.
> 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
