USB: HCD: support giveback of URB in tasklet context
This patch implements the mechanism of giveback of URB in
tasklet context, so that hardware interrupt handling time for
usb host controller can be saved much, and HCD interrupt handling
can be simplified.
Motivations:
1), on some arch(such as ARM), DMA mapping/unmapping is a bit
time-consuming, for example: when accessing usb mass storage
via EHCI on pandaboard, the common length of transfer buffer is 120KB,
the time consumed on DMA unmapping may reach hundreds of microseconds;
even on A15 based box, the time is still about scores of microseconds
2), on some arch, reading DMA coherent memoery is very time-consuming,
the most common example is usb video class driver[1]
3), driver's complete() callback may do much things which is driver
specific, so the time is consumed unnecessarily in hardware irq context.
4), running driver's complete() callback in hardware irq context causes
that host controller driver has to release its lock in interrupt handler,
so reacquiring the lock after return may busy wait a while and increase
interrupt handling time. More seriously, releasing the HCD lock makes
HCD becoming quite complicated to deal with introduced races.
So the patch proposes to run giveback of URB in tasklet context, then
time consumed in HCD irq handling doesn't depend on drivers' complete and
DMA mapping/unmapping any more, also we can simplify HCD since the HCD
lock isn't needed to be released during irq handling.
The patch should be reasonable and doable:
1), for drivers, they don't care if the complete() is called in hard irq
context or softirq context
2), the biggest change is the situation in which usb_submit_urb() is called
in complete() callback, so the introduced tasklet schedule delay might be a
con, but it shouldn't be a big deal:
- control/bulk asynchronous transfer isn't sensitive to schedule
delay
- the patch schedules giveback of periodic URBs using
tasklet_hi_schedule, so the introduced delay should be very
small
- for ISOC transfer, generally, drivers submit several URBs
concurrently to avoid interrupt delay, so it is OK with the
little schedule delay.
- for interrupt transfer, generally, drivers only submit one URB
at the same time, but interrupt transfer is often used in event
report, polling, ... situations, and a little delay should be OK.
Considered that HCDs may optimize on submitting URB in complete(), the
patch may cause the optimization not working, so introduces one flag to mark
if the HCD supports to run giveback URB in tasklet context. When all HCDs
are ready, the flag can be removed.
[1], http://marc.info/?t=136438111600010&r=1&w=2
Cc: Oliver Neukum <oliver@neukum.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index dc1346f..0ffe016 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -696,15 +696,7 @@
/* any errors get returned through the urb completion */
spin_lock_irq(&hcd_root_hub_lock);
usb_hcd_unlink_urb_from_ep(hcd, urb);
-
- /* This peculiar use of spinlocks echoes what real HC drivers do.
- * Avoiding calls to local_irq_disable/enable makes the code
- * RT-friendly.
- */
- spin_unlock(&hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb, status);
- spin_lock(&hcd_root_hub_lock);
-
spin_unlock_irq(&hcd_root_hub_lock);
return 0;
}
@@ -744,9 +736,7 @@
memcpy(urb->transfer_buffer, buffer, length);
usb_hcd_unlink_urb_from_ep(hcd, urb);
- spin_unlock(&hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb, 0);
- spin_lock(&hcd_root_hub_lock);
} else {
length = 0;
set_bit(HCD_FLAG_POLL_PENDING, &hcd->flags);
@@ -836,10 +826,7 @@
if (urb == hcd->status_urb) {
hcd->status_urb = NULL;
usb_hcd_unlink_urb_from_ep(hcd, urb);
-
- spin_unlock(&hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb, status);
- spin_lock(&hcd_root_hub_lock);
}
}
done:
@@ -1656,6 +1643,72 @@
/*-------------------------------------------------------------------------*/
+static void __usb_hcd_giveback_urb(struct urb *urb)
+{
+ struct usb_hcd *hcd = bus_to_hcd(urb->dev->bus);
+ int status = urb->unlinked;
+ unsigned long flags;
+
+ urb->hcpriv = NULL;
+ if (unlikely((urb->transfer_flags & URB_SHORT_NOT_OK) &&
+ urb->actual_length < urb->transfer_buffer_length &&
+ !status))
+ status = -EREMOTEIO;
+
+ unmap_urb_for_dma(hcd, urb);
+ usbmon_urb_complete(&hcd->self, urb, status);
+ usb_unanchor_urb(urb);
+
+ /* pass ownership to the completion handler */
+ urb->status = status;
+
+ /*
+ * 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);
+
+ atomic_dec(&urb->use_count);
+ if (unlikely(atomic_read(&urb->reject)))
+ wake_up(&usb_kill_urb_queue);
+ usb_put_urb(urb);
+}
+
+static void usb_giveback_urb_bh(unsigned long param)
+{
+ struct giveback_urb_bh *bh = (struct giveback_urb_bh *)param;
+ struct list_head local_list;
+
+ spin_lock_irq(&bh->lock);
+ bh->running = true;
+ restart:
+ list_replace_init(&bh->head, &local_list);
+ spin_unlock_irq(&bh->lock);
+
+ while (!list_empty(&local_list)) {
+ struct urb *urb;
+
+ urb = list_entry(local_list.next, struct urb, urb_list);
+ list_del_init(&urb->urb_list);
+ __usb_hcd_giveback_urb(urb);
+ }
+
+ /* check if there are new URBs to giveback */
+ spin_lock_irq(&bh->lock);
+ if (!list_empty(&bh->head))
+ goto restart;
+ bh->running = false;
+ spin_unlock_irq(&bh->lock);
+}
+
/**
* usb_hcd_giveback_urb - return URB from HCD to device driver
* @hcd: host controller returning the URB
@@ -1675,25 +1728,37 @@
*/
void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
{
- urb->hcpriv = NULL;
- if (unlikely(urb->unlinked))
- status = urb->unlinked;
- else if (unlikely((urb->transfer_flags & URB_SHORT_NOT_OK) &&
- urb->actual_length < urb->transfer_buffer_length &&
- !status))
- status = -EREMOTEIO;
+ struct giveback_urb_bh *bh;
+ bool running, high_prio_bh;
- unmap_urb_for_dma(hcd, urb);
- usbmon_urb_complete(&hcd->self, urb, status);
- usb_unanchor_urb(urb);
+ /* pass status to tasklet via unlinked */
+ if (likely(!urb->unlinked))
+ urb->unlinked = status;
- /* pass ownership to the completion handler */
- urb->status = status;
- urb->complete (urb);
- atomic_dec (&urb->use_count);
- if (unlikely(atomic_read(&urb->reject)))
- wake_up (&usb_kill_urb_queue);
- usb_put_urb (urb);
+ if (!hcd_giveback_urb_in_bh(hcd) && !is_root_hub(urb->dev)) {
+ __usb_hcd_giveback_urb(urb);
+ return;
+ }
+
+ if (usb_pipeisoc(urb->pipe) || usb_pipeint(urb->pipe)) {
+ bh = &hcd->high_prio_bh;
+ high_prio_bh = true;
+ } else {
+ bh = &hcd->low_prio_bh;
+ high_prio_bh = false;
+ }
+
+ spin_lock(&bh->lock);
+ list_add_tail(&urb->urb_list, &bh->head);
+ running = bh->running;
+ spin_unlock(&bh->lock);
+
+ if (running)
+ ;
+ else if (high_prio_bh)
+ tasklet_hi_schedule(&bh->bh);
+ else
+ tasklet_schedule(&bh->bh);
}
EXPORT_SYMBOL_GPL(usb_hcd_giveback_urb);
@@ -2322,6 +2387,14 @@
/*-------------------------------------------------------------------------*/
+static void init_giveback_urb_bh(struct giveback_urb_bh *bh)
+{
+
+ spin_lock_init(&bh->lock);
+ INIT_LIST_HEAD(&bh->head);
+ tasklet_init(&bh->bh, usb_giveback_urb_bh, (unsigned long)bh);
+}
+
/**
* usb_create_shared_hcd - create and initialize an HCD structure
* @driver: HC driver that will use this hcd
@@ -2590,6 +2663,10 @@
&& device_can_wakeup(&hcd->self.root_hub->dev))
dev_dbg(hcd->self.controller, "supports USB remote wakeup\n");
+ /* initialize tasklets */
+ init_giveback_urb_bh(&hcd->high_prio_bh);
+ init_giveback_urb_bh(&hcd->low_prio_bh);
+
/* enable irqs just before we start the controller,
* if the BIOS provides legacy PCI irqs.
*/
@@ -2698,6 +2775,16 @@
usb_disconnect(&rhdev); /* Sets rhdev to NULL */
mutex_unlock(&usb_bus_list_lock);
+ /*
+ * tasklet_kill() isn't needed here because:
+ * - driver's disconnect() called from usb_disconnect() should
+ * make sure its URBs are completed during the disconnect()
+ * callback
+ *
+ * - it is too late to run complete() here since driver may have
+ * been removed already now
+ */
+
/* Prevent any more root-hub status calls from the timer.
* The HCD might still restart the timer (if a port status change
* interrupt occurs), but usb_hcd_poll_rh_status() won't invoke