On 12/4/2014 5:38 PM, Kristen Carlson Accardi wrote:
> On Thu, 04 Dec 2014 23:10:58 +0100
> "Rafael J. Wysocki" <r...@rjwysocki.net> wrote:
> 
>> On Thursday, December 04, 2014 11:07:31 AM Ethan Zhao wrote:
>>> To force loading on Oracle Sun X86 servers, provide one kernel command line
>>> parameter
>>>
>>>   intel_pstate = ora_force
>>
>> I would suggest to change the name of the option to "oracle_force" or 
>> "sun_force"
>> for clarity.
>>
>> Anyway, I need an ACK from Kristen if this patch is to be applied.
>>
>>> For those who be aware of the risk of no power capping capabily working and
>>> try to get better performance with this driver.
>>>
>>> Signed-off-by: Ethan Zhao <ethan.z...@oracle.com>
>>> ---
>>>  v2: change to hardware vendor specific naming parameter.
>>>  v4: refine code and doc.
>>>  v5&v6: fix a typo in doc.
>>>  v7: change enum PCC to PPC.
>>>
>>>  Documentation/kernel-parameters.txt | 5 +++++
>>>  drivers/cpufreq/intel_pstate.c      | 6 +++++-
>>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/kernel-parameters.txt 
>>> b/Documentation/kernel-parameters.txt
>>> index 479f332..7d0983e 100644
>>> --- a/Documentation/kernel-parameters.txt
>>> +++ b/Documentation/kernel-parameters.txt
>>> @@ -1446,6 +1446,11 @@ bytes respectively. Such letter suffixes can also be 
>>> entirely omitted.
>>>                    disable
>>>                      Do not enable intel_pstate as the default
>>>                      scaling driver for the supported processors
>>> +                  ora_force
>>> +                    Force loading intel_pstate on Oracle Sun Servers(X86).
>>> +                    only for those who be aware of the risk of no power 
>>> capping
>>> +                    capability working and try to get better performance 
>>> with this
>>> +                    driver.
>>
>> That is not sufficiently clear.  What does "risk of no power capping 
>> capability
>> working" mean, in particular?
>>
>>>  
>>>     intremap=       [X86-64, Intel-IOMMU]
>>>                     on      enable Interrupt Remapping (default)
>>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>>> index 1bb62ca..2654e13 100644
>>> --- a/drivers/cpufreq/intel_pstate.c
>>> +++ b/drivers/cpufreq/intel_pstate.c
>>> @@ -866,6 +866,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>>>  };
>>>  
>>>  static int __initdata no_load;
>>> +static unsigned int  ora_force;
>>>  
>>>  static int intel_pstate_msrs_not_valid(void)
>>>  {
>>> @@ -1003,7 +1004,8 @@ static bool 
>>> intel_pstate_platform_pwr_mgmt_exists(void)
>>>                     case PSS:
>>>                             return intel_pstate_no_acpi_pss();
>>>                     case PPC:
>>> -                           return intel_pstate_has_acpi_ppc();
>>> +                           return intel_pstate_has_acpi_ppc() &&
>>> +                                   (!ora_force);
>>>                     }
>>>     }
>>>  
>>> @@ -1078,6 +1080,8 @@ static int __init intel_pstate_setup(char *str)
>>>  
>>>     if (!strcmp(str, "disable"))
>>>             no_load = 1;
>>> +   if (!strcmp(str, "ora_force"))
>>> +           ora_force = 1;
>>>     return 0;
>>>  }
>>>  early_param("intel_pstate", intel_pstate_setup);
>>
>> And can anyone please remind me what was wrong with a "force" option that 
>> would
>> work for everyone, not just Oracle/Sun?
>>
> 
> That was my suggestion as well (i.e. a parameter to bypass the vendor
> checks), but Linda didn't like it.  My personal opinion is that unless
> it's generic, I don't really feel like having a force option solely for
> oracle.  I'm not convinced you want this for production machines, and I
> think for debug purposes I don't want a vendor specific param.

I'd be happy with it if it somehow disabled what the platform is doing,
but it doesn't.  I don't see the point of forcing intel_pstate if you
can't force the platform to stop doing power management at the same time.
Even if it's for test/debug purposes, I'm not sure what you're testing
when you have dueling power management.

The description would need to be different too since I think on
ProLiant, power capping can happen at any time, even if the
system is in OS control mode and the intel_pstate driver is
loaded.

Can anyone suggest a description for a force option that would
make sense generically?

-- ljk



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

Reply via email to