Re: [PATCH 3/3] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled

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

 



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




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

  Powered by Linux