Re: [PATCH v3] cpuidle: poll_state: Add time limit to poll_idle()

Rafael J. Wysocki
2018-03-27T21:06:11Z

On Tue, Mar 27, 2018 at 7:59 PM, Rik van Riel <riel@surriel.com> wrote:
> On Thu, 2018-03-22 at 18:09 +0100, Rafael J. Wysocki wrote:
>
>> Does it improve if you do something like the below on top of it?
>
> First off, apologies for doing my testing wrong.
> There was an error, which was cleverly hidden by
> scripts, and things were not running the way they
> should.

No worries.

I'm glad that what you really see appears to match my understanding of
things. :-)

> After running the tests right, I have seen some
> results. My own test patch of disabling polling,
> and always dropping into at least C1, resulted in
> a CPU use increase of 0.4%.
>
> Your patch below, with a single cpu_relax(), and
> the local variable to not read the TSC too often,
> seems to give the best performance here, with a
> 0.2% reduction in CPU use.

Cool.

> Again, apologies for messing up the testing.
>
> I will kick off proper tests for that other patch
> series right now.

Thanks!

>> > Let me go test the nohz idle series by itself,
>> > without this patch.
>>
>> OK
>>
>> ---
>> drivers/cpuidle/poll_state.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> Index: linux-pm/drivers/cpuidle/poll_state.c
>> ===================================================================
>> --- linux-pm.orig/drivers/cpuidle/poll_state.c
>> +++ linux-pm/drivers/cpuidle/poll_state.c
>> @@ -10,6 +10,7 @@
>> #include <linux/sched/idle.h>
>>
>> #define POLL_IDLE_TIME_LIMIT (TICK_NSEC / 16)
>> +#define POLL_IDLE_COUNT 1000
>>
>> static int __cpuidle poll_idle(struct cpuidle_device *dev,
>> struct cpuidle_driver *drv, int
>> index)
>> @@ -18,9 +19,14 @@ static int __cpuidle poll_idle(struct cp
>>
>> local_irq_enable();
>> if (!current_set_polling_and_test()) {
>> + unsigned int loop_count = 0;
>> +
>> while (!need_resched()) {
>> cpu_relax();
>> + if (loop_count++ < POLL_IDLE_COUNT)
>> + continue;
>>
>> + loop_count = 0;
>> if (local_clock() - time_start >
>> POLL_IDLE_TIME_LIMIT)
>> break;
>> }
>>
>>
> --
> All Rights Reversed.