| Message ID | 20120814121804.GC7105@flint.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Russell, > 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. With the on-review input patches on top of that, the latency should drop further. I have measured a total of 2.5 times lower latency in other areas, so I would expect something like 150 us in 3.7 for your case. > 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. Linus' master already has a patch for that code path, actually. > 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? I would like that, too. Thanks, Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 14 Aug 2012, Russell King 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. Are you testing with kernel that contains b94e3c94aae04 already? That should cover that case.
On Wed, Aug 15, 2012 at 11:02:37AM +0200, Jiri Kosina wrote: > On Tue, 14 Aug 2012, Russell King 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. > > 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.
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);