Re: USB interrupt times

On Tue, Aug 14, 2012 at 12:54:00PM +0200, Jiri Kosina wrote:

Actually, Henrik (added to CC) has been doing some latency improvements 
both for input core in general, and for HID devices as well lately. I 
still have his patchset in my to-review queue, as I have just came back 
from offline vacation, but the patch below definitely can't hurt and 
should significantly lower the time spent in handling the irq for hid 
device in common situation (i.e. noone listening for debugfs events).

Could you please measure how much it helps on your system?

Ok, it looks like it's changed the maximum USB interrupt execution
time from around 364us to 255us.

If I also do a similar trick with the debug code in hid_input_report()
then I get down to 212us - iow, something like the patch below.

Given that debugfs is fairly ubiquitous in the kernel, and that this is
fairly invasive in terms of interrupt execution impact, wouldn't having
this debug code enabled by a separate Kconfig symbol be reasonable too?

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 6ac0286..8daf4d1 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c

@@ -1220,8 +1220,6 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
 	struct hid_report_enum *report_enum;
 	struct hid_driver *hdrv;
 	struct hid_report *report;
-	char *buf;
-	unsigned int i;
 	int ret = 0;
 
 	if (!hid)

@@ -1243,23 +1241,27 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
 		goto unlock;
 	}
 
-	buf = kmalloc(sizeof(char) * HID_DEBUG_BUFSIZE, GFP_ATOMIC);
+	if (!list_empty(&hid->debug_list)) {
+		unsigned int i;
+		char *buf;
 
-	if (!buf)
-		goto nomem;
+		buf = kmalloc(sizeof(char) * HID_DEBUG_BUFSIZE, GFP_ATOMIC);
+		if (!buf)
+			goto nomem;
 
-	/* dump the report */
-	snprintf(buf, HID_DEBUG_BUFSIZE - 1,
+		/* dump the report */
+		snprintf(buf, HID_DEBUG_BUFSIZE - 1,
 			"\nreport (size %u) (%snumbered) = ", size, report_enum->numbered ? "" : "un");
-	hid_debug_event(hid, buf);
+		hid_debug_event(hid, buf);
 
-	for (i = 0; i < size; i++) {
-		snprintf(buf, HID_DEBUG_BUFSIZE - 1,
+		for (i = 0; i < size; i++) {
+			snprintf(buf, HID_DEBUG_BUFSIZE - 1,
 				" %02x", data[i]);
-		hid_debug_event(hid, buf);
+			hid_debug_event(hid, buf);
+		}
+		hid_debug_event(hid, "\n");
+		kfree(buf);
 	}
-	hid_debug_event(hid, "\n");
-	kfree(buf);
 
 nomem:
 	report = hid_get_report(report_enum, data);

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, 14 Aug 2012, Russell King wrote:

quoted

Actually, Henrik (added to CC) has been doing some latency improvements 
both for input core in general, and for HID devices as well lately. I 
still have his patchset in my to-review queue, as I have just came back 
from offline vacation, but the patch below definitely can't hurt and 
should significantly lower the time spent in handling the irq for hid 
device in common situation (i.e. noone listening for debugfs events).

Could you please measure how much it helps on your system?

Ok, it looks like it's changed the maximum USB interrupt execution
time from around 364us to 255us.

If I also do a similar trick with the debug code in hid_input_report()
then I get down to 212us - iow, something like the patch below.

Are you testing with kernel that contains b94e3c94aae04 already? That 
should cover that case.

-- 
Jiri Kosina
SUSE Labs

On Wed, Aug 15, 2012 at 11:02:37AM +0200, Jiri Kosina wrote:

On Tue, 14 Aug 2012, Russell King wrote:

quoted

quoted

Actually, Henrik (added to CC) has been doing some latency improvements 
both for input core in general, and for HID devices as well lately. I 
still have his patchset in my to-review queue, as I have just came back 
from offline vacation, but the patch below definitely can't hurt and 
should significantly lower the time spent in handling the irq for hid 
device in common situation (i.e. noone listening for debugfs events).

Could you please measure how much it helps on your system?

Ok, it looks like it's changed the maximum USB interrupt execution
time from around 364us to 255us.

If I also do a similar trick with the debug code in hid_input_report()
then I get down to 212us - iow, something like the patch below.

Are you testing with kernel that contains b94e3c94aae04 already? That 
should cover that case.

No - the kernel I'm dealing with is unfortunately based upon v3.5.0-rc5
with many other patches on top (which add support for this platform.)
However, I'll pick that commit into this tree.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of: