dummy_hcd uses jiffies and seems to assume that HZ=1000 and no tickless behavior. This makes its emulation not very faithful, especially for typical distro desktop kernels (HZ=250, tickless which means that e.g., when dummy_hcd asks for a delay of 1ms in reality it can get 4ms or more).
For some devices, this might manifest as unexpectedly low performance (e.g., 2-4MB/s for a tcm_gadget_usb backed by a ramdisk) compared to real hardware, others might even break if they are more sensitive to timing. This patch ports dummy_hcd to use hrtimers instead, which fixes these problems and hopefully allows both for more gadgets to work with dummy_hcd and for a better emulation user experience overall, especially in typical kernel configurations. For storage this brings performance to acceptable levels (around 25MB/s for BOT, 100MB/s for UAS) improving the user experience when testing, for other devices it might mean that now they will work properly with dummy_hcd when before they didn't. Implementation uses the tasklet_hrtimer framework. This means dummy_timer callback is executed in softirq context (as it was with wheel timers) which keeps required changes to a minimum. Signed-off-by: <[email protected]> --- drivers/usb/gadget/dummy_hcd.c | 45 ++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c index e80a01e..6e6cee7 100644 --- a/drivers/usb/gadget/dummy_hcd.c +++ b/drivers/usb/gadget/dummy_hcd.c @@ -166,7 +166,7 @@ enum dummy_rh_state { struct dummy_hcd { struct dummy *dum; enum dummy_rh_state rh_state; - struct timer_list timer; + struct tasklet_hrtimer ttimer; u32 port_status; u32 old_status; unsigned long re_timeout; @@ -1190,8 +1190,11 @@ static int dummy_urb_enqueue( urb->error_count = 1; /* mark as a new urb */ /* kick the scheduler, it'll do the rest */ - if (!timer_pending(&dum_hcd->timer)) - mod_timer(&dum_hcd->timer, jiffies + 1); + if (!hrtimer_is_queued(&dum_hcd->ttimer.timer)) { + tasklet_hrtimer_start(&dum_hcd->ttimer, + ms_to_ktime(1), + HRTIMER_MODE_REL); + } done: spin_unlock_irqrestore(&dum_hcd->dum->lock, flags); @@ -1211,9 +1214,12 @@ static int dummy_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) rc = usb_hcd_check_unlink_urb(hcd, urb, status); if (!rc && dum_hcd->rh_state != DUMMY_RH_RUNNING && - !list_empty(&dum_hcd->urbp_list)) - mod_timer(&dum_hcd->timer, jiffies); - + !list_empty(&dum_hcd->urbp_list) && + !hrtimer_is_queued(&dum_hcd->ttimer.timer)) { + tasklet_hrtimer_start(&dum_hcd->ttimer, + ns_to_ktime(100), + HRTIMER_MODE_REL); + } spin_unlock_irqrestore(&dum_hcd->dum->lock, flags); return rc; } @@ -1649,9 +1655,11 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb, /* drive both sides of the transfers; looks like irq handlers to * both drivers except the callbacks aren't in_irq(). */ -static void dummy_timer(unsigned long _dum_hcd) +static enum hrtimer_restart dummy_timer(struct hrtimer *timer) { - struct dummy_hcd *dum_hcd = (struct dummy_hcd *) _dum_hcd; + struct dummy_hcd *dum_hcd = container_of(timer, + struct dummy_hcd, ttimer.timer); + struct dummy *dum = dum_hcd->dum; struct urbp *urbp, *tmp; unsigned long flags; @@ -1675,11 +1683,9 @@ static void dummy_timer(unsigned long _dum_hcd) break; default: dev_err(dummy_dev(dum_hcd), "bogus device speed\n"); - return; + goto out; } - /* FIXME if HZ != 1000 this will probably misbehave ... */ - /* look at each urb queued by the host side driver */ spin_lock_irqsave(&dum->lock, flags); @@ -1687,7 +1693,7 @@ static void dummy_timer(unsigned long _dum_hcd) dev_err(dummy_dev(dum_hcd), "timer fired with no URBs pending?\n"); spin_unlock_irqrestore(&dum->lock, flags); - return; + goto out; } for (i = 0; i < DUMMY_ENDPOINTS; i++) { @@ -1859,10 +1865,14 @@ return_urb: dum_hcd->udev = NULL; } else if (dum_hcd->rh_state == DUMMY_RH_RUNNING) { /* want a 1 msec delay here */ - mod_timer(&dum_hcd->timer, jiffies + msecs_to_jiffies(1)); + tasklet_hrtimer_start(&dum_hcd->ttimer, ms_to_ktime(1), + HRTIMER_MODE_REL); } spin_unlock_irqrestore(&dum->lock, flags); + +out: + return HRTIMER_NORESTART; } /*-------------------------------------------------------------------------*/ @@ -2238,7 +2248,9 @@ static int dummy_bus_resume(struct usb_hcd *hcd) dum_hcd->rh_state = DUMMY_RH_RUNNING; set_link_state(dum_hcd); if (!list_empty(&dum_hcd->urbp_list)) - mod_timer(&dum_hcd->timer, jiffies); + tasklet_hrtimer_start(&dum_hcd->ttimer, + ms_to_ktime(1), + HRTIMER_MODE_REL); hcd->state = HC_STATE_RUNNING; } spin_unlock_irq(&dum_hcd->dum->lock); @@ -2325,9 +2337,8 @@ static int dummy_start(struct usb_hcd *hcd) */ if (dum_hcd->rh_state != DUMMY_RH_RUNNING) { spin_lock_init(&dum_hcd->dum->lock); - init_timer(&dum_hcd->timer); - dum_hcd->timer.function = dummy_timer; - dum_hcd->timer.data = (unsigned long)dum_hcd; + tasklet_hrtimer_init(&dum_hcd->ttimer, dummy_timer, + CLOCK_MONOTONIC, HRTIMER_MODE_ABS); INIT_LIST_HEAD(&dum_hcd->urbp_list); dum_hcd->rh_state = DUMMY_RH_RUNNING; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
