| Message ID | 1371567833-9077-5-git-send-email-ming.lei@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 18 Jun 2013, Ming Lei wrote: > If HCD_BH is set for HC driver's flags, URB giveback will be > done in tasklet context instead of interrupt context, so the > ehci->lock needn't to be released any more before calling > usb_hcd_giveback_urb(). It is premature to do this now. This should be part of the 6/6 patch, when it won't be necessary to test hcd_giveback_urb_in_bh(). Alan Stern
On Wed, Jun 19, 2013 at 12:43 AM, Alan Stern <stern@rowland.harvard.edu> wrote: > On Tue, 18 Jun 2013, Ming Lei wrote: > >> If HCD_BH is set for HC driver's flags, URB giveback will be >> done in tasklet context instead of interrupt context, so the >> ehci->lock needn't to be released any more before calling >> usb_hcd_giveback_urb(). > > It is premature to do this now. This should be part of the 6/6 patch, It is fine since HCD_BH isn't enabled. > when it won't be necessary to test hcd_giveback_urb_in_bh(). Keeping it is easy to compare test results between running complete() in tasklet and in hard irq handler. Also it might be helpful for out of tree drivers. But it isn't a big deal, I can merge it to 6/6 if you care. Thanks, -- Ming Lei
On Wed, 19 Jun 2013, Ming Lei wrote: > On Wed, Jun 19, 2013 at 12:43 AM, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Tue, 18 Jun 2013, Ming Lei wrote: > > > >> If HCD_BH is set for HC driver's flags, URB giveback will be > >> done in tasklet context instead of interrupt context, so the > >> ehci->lock needn't to be released any more before calling > >> usb_hcd_giveback_urb(). > > > > It is premature to do this now. This should be part of the 6/6 patch, > > It is fine since HCD_BH isn't enabled. It's not "fine". It will work -- nothing will crash -- but that doesn't mean it is good style. > > when it won't be necessary to test hcd_giveback_urb_in_bh(). > > Keeping it is easy to compare test results between running complete() > in tasklet and in hard irq handler. It's just as easy to do your tests by keeping two copies of ehci-hcd.ko lying around; one with your changes and one without. You can insmod whichever one you want to test. > Also it might be helpful for out of tree drivers. > > But it isn't a big deal, I can merge it to 6/6 if you care. Yes, please. Alan Stern
On Wed, Jun 19, 2013 at 11:47 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > On Wed, 19 Jun 2013, Ming Lei wrote: > >> On Wed, Jun 19, 2013 at 12:43 AM, Alan Stern <stern@rowland.harvard.edu> wrote: >> > On Tue, 18 Jun 2013, Ming Lei wrote: >> > >> >> If HCD_BH is set for HC driver's flags, URB giveback will be >> >> done in tasklet context instead of interrupt context, so the >> >> ehci->lock needn't to be released any more before calling >> >> usb_hcd_giveback_urb(). >> > >> > It is premature to do this now. This should be part of the 6/6 patch, >> >> It is fine since HCD_BH isn't enabled. > > It's not "fine". It will work -- nothing will crash -- but that > doesn't mean it is good style. > >> > when it won't be necessary to test hcd_giveback_urb_in_bh(). >> >> Keeping it is easy to compare test results between running complete() >> in tasklet and in hard irq handler. > > It's just as easy to do your tests by keeping two copies of ehci-hcd.ko > lying around; one with your changes and one without. You can insmod > whichever one you want to test. > >> Also it might be helpful for out of tree drivers. >> >> But it isn't a big deal, I can merge it to 6/6 if you care. > > Yes, please. OK. Thanks, -- Ming Lei
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c index d34b399..0387a81 100644 --- a/drivers/usb/host/ehci-q.c +++ b/drivers/usb/host/ehci-q.c @@ -283,9 +283,11 @@ __acquires(ehci->lock) /* complete() can reenter this HCD */ usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb); - spin_unlock (&ehci->lock); + if (!hcd_giveback_urb_in_bh(ehci_to_hcd(ehci))) + spin_unlock(&ehci->lock); usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status); - spin_lock (&ehci->lock); + if (!hcd_giveback_urb_in_bh(ehci_to_hcd(ehci))) + spin_lock(&ehci->lock); } static int qh_schedule (struct ehci_hcd *ehci, struct ehci_qh *qh);
If HCD_BH is set for HC driver's flags, URB giveback will be done in tasklet context instead of interrupt context, so the ehci->lock needn't to be released any more before calling usb_hcd_giveback_urb(). Cc: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Ming Lei <ming.lei@canonical.com> --- drivers/usb/host/ehci-q.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)