On Mon, Aug 19, 2013 at 10:33 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Why would those events not need to be handled?
For IAA_WATCHDOG event, it needn't to handle when
IAA interrupt comes.
For START_UNLINK_INTR event, we don't need to unlink intr
qh any more if intr urb submit request comes.
> What does this help with? Measurements please.
The patch may avoid unnecessary CPU wakeups caused by
timer expiration, and measurement is provided below.
On Mon, Aug 19, 2013 at 11:30 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 19 Aug 2013, Ming Lei wrote:
>
>> This patch introduces ehci_disable_event(), which is applied on
>> IAA_WATCHDOG and START_UNLINK_INTR events in case that the two
>> events needn't to be handled, so that we may avoid unnecessary CPU
>> wakeup.
>
>> @@ -100,6 +100,20 @@ static void ehci_enable_event(struct ehci_hcd *ehci, unsigned event,
>> }
>>
>>
>
> Only one blank line here, please.
OK.
>
>> +/* Warning: don't call this function from hrtimer handler context */
>> +static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event)
>> +{
>> + ehci->enabled_hrtimer_events &= ~(1 << event);
>> + if (!ehci->enabled_hrtimer_events) {
>> + ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
>> + hrtimer_cancel(&ehci->hrtimer);
>> + } else if (ehci->next_hrtimer_event == event) {
>> + ehci->next_hrtimer_event =
>> + ffs(ehci->enabled_hrtimer_events) - 1;
>
> Should the timer be rescheduled here? It's hard to say without seeing
> some test results.
You are right, the timer should be rescheduled here, otherwise there is no
effect on HCD with need_io_watchdog set.
With below change on ehci_disable_event(), about 7~8 times timer
expiration can be saved when one asix network NIC is connected
to EHCI without network traffic(the device responds interrupt poll
7~8 times per second constantly for link status query), no matter
ehci->need_io_watchdog is set or not.
BTW, the timer expiration event is observed via /proc/timer_stats.
/* Warning: don't call this function from hrtimer handler context */
static void ehci_disable_event(struct ehci_hcd *ehci, unsigned event)
{
ehci->enabled_hrtimer_events &= ~(1 << event);
if (!ehci->enabled_hrtimer_events) {
ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
hrtimer_cancel(&ehci->hrtimer);
} else if (ehci->next_hrtimer_event == event) {
ehci->next_hrtimer_event = EHCI_HRTIMER_NO_EVENT;
enum ehci_hrtimer_event next =
ffs(ehci->enabled_hrtimer_events) - 1;
ehci_enable_event(ehci, next, 0);
}
}
Thanks,
--
Ming Lei
--
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
