[PATCH 3/3] usb: gadget: Port dummy_hcd to hrtimers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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: <pktoss@xxxxxxxxx>
---
 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux