[PATCH] cpufreq/intel_pstate: Set timer timeout correctly

[PATCH] cpufreq/intel_pstate: Set timer timeout correctly

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

The current calculation of the delay time is wrong and a cut and paste
error from a previous experimental driver.  This can result in the
timeout being set to jiffies + 1 which setup the driver to race with
it's self if the apic timer interrupt happen at just the right time.


https://bugzilla.redhat.com/show_bug.cgi?id=920289

Reported-by: Adam Williamson <awilliam@redhat.com>
Reported-by: Parag Warudkar <parag.lkml@gmail.com>

Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
---
 drivers/cpufreq/intel_pstate.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 43ffe1c..4d6b988 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -502,7 +502,6 @@ static inline void intel_pstate_set_sample_time(struct cpudata *cpu)
 
 	sample_time = cpu->pstate_policy->sample_rate_ms;
 	delay = msecs_to_jiffies(sample_time);
-	delay -= jiffies % delay;
 	mod_timer_pinned(&cpu->timer, jiffies + delay);
 }
 
-- 
1.7.7.6

Re: [PATCH] cpufreq/intel_pstate: Set timer timeout correctly

[Viresh Kumar]

On Thu, Apr 4, 2013 at 11:05 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> The current calculation of the delay time is wrong and a cut and paste
> error from a previous experimental driver.  This can result in the
> timeout being set to jiffies + 1 which setup the driver to race with
> it's self if the apic timer interrupt happen at just the right time.
>
>
> https://bugzilla.redhat.com/show_bug.cgi?id=920289
>
> Reported-by: Adam Williamson <awilliam@redhat.com>
> Reported-by: Parag Warudkar <parag.lkml@gmail.com>
>
> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
> ---
>  drivers/cpufreq/intel_pstate.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

Looks fine, but I would like to see a Tested-by from Adam/Parag
as they haven't said anything about this patch (even in bugzilla).

Re: [PATCH] cpufreq/intel_pstate: Set timer timeout correctly

[Adam Williamson]

On 04/04/13 10:19 PM, Viresh Kumar wrote:
> On Thu, Apr 4, 2013 at 11:05 PM,  <dirk.brandewie@gmail.com> wrote:
>> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>>
>> The current calculation of the delay time is wrong and a cut and paste
>> error from a previous experimental driver.  This can result in the
>> timeout being set to jiffies + 1 which setup the driver to race with
>> it's self if the apic timer interrupt happen at just the right time.
>>
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=920289
>>
>> Reported-by: Adam Williamson <awilliam@redhat.com>
>> Reported-by: Parag Warudkar <parag.lkml@gmail.com>
>>
>> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
>> ---
>>   drivers/cpufreq/intel_pstate.c |    1 -
>>   1 files changed, 0 insertions(+), 1 deletions(-)
>
> Looks fine, but I would like to see a Tested-by from Adam/Parag
> as they haven't said anything about this patch (even in bugzilla).

I'll try. Note the bug is not reliably reproducible, all I can really do 
is run for a day or two and see if it crashes.
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | identi.ca: adamwfedora
http://www.happyassassin.net

Re: [PATCH] cpufreq/intel_pstate: Set timer timeout correctly

[Parag Warudkar]

On Fri, 5 Apr 2013, Viresh Kumar wrote:

> On Thu, Apr 4, 2013 at 11:05 PM,  <dirk.brandewie@gmail.com> wrote:
> > From: Dirk Brandewie <dirk.brandewie@gmail.com>
> >
> > The current calculation of the delay time is wrong and a cut and paste
> > error from a previous experimental driver.  This can result in the
> > timeout being set to jiffies + 1 which setup the driver to race with
> > it's self if the apic timer interrupt happen at just the right time.
> >
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=920289
> >
> > Reported-by: Adam Williamson <awilliam@redhat.com>
> > Reported-by: Parag Warudkar <parag.lkml@gmail.com>
> >
> > Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
> > ---
> >  drivers/cpufreq/intel_pstate.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> Looks fine, but I would like to see a Tested-by from Adam/Parag
> as they haven't said anything about this patch (even in bugzilla).
> 

I am running with the patch since yesterday - everything looks good.

The issue hasn't been reproducible on demand but some code reading and 
Dirk's explanation says the patch should fix the issue.

So - Tested-by: Parag Warudkar <parag.lkml@gmail.com>

Parag