Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1274702 > unrolled thread

RE: [PATCH] [v4] x86, suspend: Save/restore extra MSR registers for suspend

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH] [v4] x86, suspend: Save/restore extra MSR registers for suspend "Doug Smythies" <dsmythies@telus.net> - 2015-11-21 17:50 +0100
    RE: [PATCH] [v4] x86, suspend: Save/restore extra MSR registers for suspend "Doug Smythies" <dsmythies@telus.net> - 2015-11-27 04:30 +0100
      Re: [PATCH] [v4] x86, suspend: Save/restore extra MSR registers for  suspend Yu Chen <yu.c.chen@intel.com> - 2015-11-27 07:00 +0100

#1274702 — RE: [PATCH] [v4] x86, suspend: Save/restore extra MSR registers for suspend

On 2015.11.12 01:42 Chen, Yu C wrote:
> On 2015.11.06 11:34 Doug Smythies wrote: 
>> On 2015.11.01 08:50 Chen, Yu C wrote:
>>>> On 2015.10.10 19:27 Chen, Yu C wrote:
>>>>> On 2105.10.10 02:56 Doug Smythies wrote:
>>>>>
>>>>>>> The current version of the intel_pstate driver is incompatible
>>>>>>> with any use of Clock Modulation, always resulting in driving the
>>>>>>> target pstate to the minimum, regardless of load. The result is
>>>>>>> the apparent CPU frequency stuck at minimum * modulation percent.
>>>>>>
>>>>>>> The acpi-cpufreq driver works fine with Clock Modulation,
>>>>>>> resulting in desired frequency * modulation percent.
>>>>>>
>>>>
>>>>> [Yu] Why intel_pstate driver is incompatible with Clock Modulation?
>>>>
>>>> It is simply how the current control algorithm responds to the scenario.
>>>>
>>>> The problem is in intel_pstate_get_scaled_busy, here:
>>>>
>>>>         /*
>>>>          * core_busy is the ratio of actual performance to max
>>>>          * max_pstate is the max non turbo pstate available
>>>>          * current_pstate was the pstate that was requested during
>>>>          *      the last sample period.
>>>>          *
>>>>          * We normalize core_busy, which was our actual percent
>>>>          * performance to what we requested during the last sample
>>>>          * period. The result will be a percentage of busy at a
>>>>          * specified pstate.
>>>>          */
>>>>         core_busy = cpu->sample.core_pct_busy;
>>>>         max_pstate = int_tofp(cpu->pstate.max_pstate);
>>>>         current_pstate = int_tofp(cpu->pstate.current_pstate);
>>>>         core_busy = mul_fp(core_busy, div_fp(max_pstate,
>>>> current_pstate));
>>>>
>>>> With Clock Modulation enabled, the actual performance percent will
>>>> always be less than what was asked for, basically meaning
>>>> current_pstate is much less than what was asked for. Thus the
>>>> algorithm will drive down the target pstate regardless of load.
>>>>
>>> [Yu] Do you mean, there is some problem with the normalization,and we
>>> should use the actual pstate rather than the theoretical
>>> current_pstate, for example, the pseudocode might looked like:
>>>
>>> -  current_pstate = int_tofp(cpu->pstate.current_pstate);
>>> + current_pstate = int_tofp(cpu->pstate.current_pstat)*0.85;
>> 
>> I did not think of normalizing / compensating at this point.
>> That is a good idea.
>> Just for a test, I tried it and it seems to work well.
>> Before normalizing / compensating core_busy can be quite a small for lesser
>> clock modulation duty cycles, and so becomes a little noisy afterwards.
>> 
>> For my test, on an otherwise unaltered kernel v4.3 I did this:
>> 
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index aa33b92..97a90e1 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -821,6 +821,7 @@ static inline int32_t
>> intel_pstate_get_scaled_busy(struct cpudata *cpu)
>>         int32_t core_busy, max_pstate, current_pstate, sample_ratio;
>>         s64 duration_us;
>>         u32 sample_time;
>> +       u64 clock_modulation;
>> 
>>         /*
>          * core_busy is the ratio of actual performance to max @@ -836,6
>> +837,17 @@ static inline int32_t intel_pstate_get_scaled_busy(struct
>> cpudata *cpu)
>>         core_busy = cpu->sample.core_pct_busy;
>>         max_pstate = int_tofp(cpu->pstate.max_pstate);
>>         current_pstate = int_tofp(cpu->pstate.current_pstate);
>> +
>> +//     rdmsrl(MSR_IA32_CLOCK_MODULATION, clock_modulation);
>> +       rdmsrl(MSR_IA32_THERM_CONTROL, clock_modulation);
>> +       if(clock_modulation && 0X10) {
>> +               clock_modulation = clock_modulation & 0x0F;
>> +               if(clock_modulation == 0) clock_modulation = 8;
>> +               core_busy = mul_fp(core_busy, int_tofp(0x10));
>> +               core_busy = div_fp(core_busy, int_tofp(clock_modulation));
>> +       }
>> +
> rdmsr_safe  might be better,

I'll look into it, thanks.

> you can refer to acpi_throttling_rdmsr

I don't understand.

 ,
> and I'm OK with this code, are you planning to send a formal patch? 

The delay here is because I have always thought that some actual load
content needs to be brought back to the intel_pstate driver, which would
(or at least should) eliminate the need for this patch.

Anyway, and at least for the interim, I'll try to make and submit a formal version.

... Doug


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1278514

On 2015.11.21 08:45 Doug Smythies wrote:
>On 2015.11.12 01:42 Chen, Yu C wrote:
>> On 2015.11.06 11:34 Doug Smythies wrote: 

[cut]

>> rdmsr_safe  might be better,

> I'll look into it, thanks.

>> you can refer to acpi_throttling_rdmsr

> I don't understand.

>> and I'm OK with this code, are you planning to send a formal patch? 

> The delay here is because I have always thought that some actual load
> content needs to be brought back to the intel_pstate driver, which would
> (or at least should) eliminate the need for this patch.

> Anyway, and at least for the interim, I'll try to make and submit a formal version.

I made a mistake in my initial testing. I put a 100% load on CPU 7 and then
cycled through all the clock modulation values to show that my test version of
a possible patch compensated / normalized the Clock Modulation. Indeed, if the
system is already asking for the maximum pstate, it will stay there. However,
whenever the load drops, the target pstate will drop to minimum and it will
never kick back up again, regardless of load.

I am returning to my initial assertion copied below:

>>>>>>> The current version of the intel_pstate driver is incompatible
>>>>>>> with any use of Clock Modulation, always resulting in driving the
>>>>>>> target pstate to the minimum, regardless of load. The result is
>>>>>>> the apparent CPU frequency stuck at minimum * modulation percent.
>>>>>>
>>>>>>> The acpi-cpufreq driver works fine with Clock Modulation,
>>>>>>> resulting in desired frequency * modulation percent.

Chen,

Thanks though for the suggestion to try normalizing.

... Doug


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278550 — Re: [PATCH] [v4] x86, suspend: Save/restore extra MSR registers for suspend

Hi,
On 11/27/2015 11:28 AM, Doug Smythies wrote:
> On 2015.11.21 08:45 Doug Smythies wrote:
>> On 2015.11.12 01:42 Chen, Yu C wrote:
>>> On 2015.11.06 11:34 Doug Smythies wrote:
>
> [cut]
>
>>> rdmsr_safe  might be better,
>
>> I'll look into it, thanks.
>
>>> you can refer to acpi_throttling_rdmsr
>
>> I don't understand.
>
>>> and I'm OK with this code, are you planning to send a formal patch?
>
>> The delay here is because I have always thought that some actual load
>> content needs to be brought back to the intel_pstate driver, which would
>> (or at least should) eliminate the need for this patch.
>
>> Anyway, and at least for the interim, I'll try to make and submit a formal version.
>
> I made a mistake in my initial testing. I put a 100% load on CPU 7 and then
> cycled through all the clock modulation values to show that my test version of
> a possible patch compensated / normalized the Clock Modulation. Indeed, if the
> system is already asking for the maximum pstate, it will stay there. However,
> whenever the load drops, the target pstate will drop to minimum and it will
> never kick back up again, regardless of load.
>
  Do you mean even with your
  patch applied, the cpufreq policy would choose a smaller target?
  I looked up the SDM, it says in 14.7.3: on Hyper-Threading Technology
  enabled processors, the clock modulation might behave differently:
"if the programmed duty cycle is not identical for all logical
processors in the same core, the
processor core will modulate at the lowest programmed duty cycle "
I dont know if this is related to the problem.
> I am returning to my initial assertion copied below:
>
>>>>>>>> The current version of the intel_pstate driver is incompatible
>>>>>>>> with any use of Clock Modulation, always resulting in driving the
>>>>>>>> target pstate to the minimum, regardless of load. The result is
>>>>>>>> the apparent CPU frequency stuck at minimum * modulation percent.
>>>>>>>
>>>>>>>> The acpi-cpufreq driver works fine with Clock Modulation,
>>>>>>>> resulting in desired frequency * modulation percent.
>
> Chen,
>
> Thanks though for the suggestion to try normalizing.
>
I'll try to reproduce your problem, and let's discuss this offline.
> ... Doug
>
thanks,
Yu

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web