All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cpufreq: allow ordinary boolean options to be passed on the command line
@ 2015-10-30 17:49 Jan Beulich
  2015-10-30 19:22 ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2015-10-30 17:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Jinsong Liu

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

I was quite surprised to find "cpufreq=off" not doing what one would
expect it to do. Fix this.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -391,11 +391,12 @@ If set, force use of the performance cou
 available support.
 
 ### cpufreq
-> `= dom0-kernel | none | xen[,[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]`
+> `= none | {{ <boolean> | xen } [:[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]} | dom0-kernel`
 
 > Default: `xen`
 
-Indicate where the responsibility for driving power states lies.
+Indicate where the responsibility for driving power states lies.  Note that the
+choice of `dom0-kernel` is deprecated and not supported by all Dom0 kernels.
 
 * Default governor policy is ondemand.
 * `<maxfreq>` and `<minfreq>` are integers which represent max and min processor frequencies
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -64,9 +64,14 @@ enum cpufreq_controller cpufreq_controll
 
 static void __init setup_cpufreq_option(char *str)
 {
-    char *arg;
+    char *arg = strpbrk(str, ",:");
+    int choice;
 
-    if ( !strcmp(str, "dom0-kernel") )
+    if ( arg )
+        *arg++ = '\0';
+    choice = parse_bool(str);
+
+    if ( choice < 0 && !strcmp(str, "dom0-kernel") )
     {
         xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_dom0_kernel;
@@ -74,19 +79,20 @@ static void __init setup_cpufreq_option(
         return;
     }
 
-    if ( !strcmp(str, "none") )
+    if ( choice == 0 || !strcmp(str, "none") )
     {
         xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_none;
         return;
     }
 
-    if ( (arg = strpbrk(str, ",:")) != NULL )
-        *arg++ = '\0';
-
-    if ( !strcmp(str, "xen") )
+    if ( choice > 0 || !strcmp(str, "xen") )
+    {
+        xen_processor_pmbits |= XEN_PROCESSOR_PM_PX;
+        cpufreq_controller = FREQCTL_xen;
         if ( arg && *arg )
             cpufreq_cmdline_parse(arg);
+    }
 }
 custom_param("cpufreq", setup_cpufreq_option);
 




[-- Attachment #2: cpufreq-cmdline-allow-bool.patch --]
[-- Type: text/plain, Size: 2327 bytes --]

cpufreq: allow ordinary boolean options to be passed on the command line

I was quite surprised to find "cpufreq=off" not doing what one would
expect it to do. Fix this.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -391,11 +391,12 @@ If set, force use of the performance cou
 available support.
 
 ### cpufreq
-> `= dom0-kernel | none | xen[,[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]`
+> `= none | {{ <boolean> | xen } [:[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]} | dom0-kernel`
 
 > Default: `xen`
 
-Indicate where the responsibility for driving power states lies.
+Indicate where the responsibility for driving power states lies.  Note that the
+choice of `dom0-kernel` is deprecated and not supported by all Dom0 kernels.
 
 * Default governor policy is ondemand.
 * `<maxfreq>` and `<minfreq>` are integers which represent max and min processor frequencies
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -64,9 +64,14 @@ enum cpufreq_controller cpufreq_controll
 
 static void __init setup_cpufreq_option(char *str)
 {
-    char *arg;
+    char *arg = strpbrk(str, ",:");
+    int choice;
 
-    if ( !strcmp(str, "dom0-kernel") )
+    if ( arg )
+        *arg++ = '\0';
+    choice = parse_bool(str);
+
+    if ( choice < 0 && !strcmp(str, "dom0-kernel") )
     {
         xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_dom0_kernel;
@@ -74,19 +79,20 @@ static void __init setup_cpufreq_option(
         return;
     }
 
-    if ( !strcmp(str, "none") )
+    if ( choice == 0 || !strcmp(str, "none") )
     {
         xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
         cpufreq_controller = FREQCTL_none;
         return;
     }
 
-    if ( (arg = strpbrk(str, ",:")) != NULL )
-        *arg++ = '\0';
-
-    if ( !strcmp(str, "xen") )
+    if ( choice > 0 || !strcmp(str, "xen") )
+    {
+        xen_processor_pmbits |= XEN_PROCESSOR_PM_PX;
+        cpufreq_controller = FREQCTL_xen;
         if ( arg && *arg )
             cpufreq_cmdline_parse(arg);
+    }
 }
 custom_param("cpufreq", setup_cpufreq_option);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cpufreq: allow ordinary boolean options to be passed on the command line
  2015-10-30 17:49 [PATCH] cpufreq: allow ordinary boolean options to be passed on the command line Jan Beulich
@ 2015-10-30 19:22 ` Andrew Cooper
  2015-11-02 10:54   ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:22 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Jinsong Liu


[-- Attachment #1.1: Type: text/plain, Size: 2642 bytes --]

On 30/10/15 17:49, Jan Beulich wrote:
> I was quite surprised to find "cpufreq=off" not doing what one would
> expect it to do. Fix this.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -391,11 +391,12 @@ If set, force use of the performance cou
>  available support.
>  
>  ### cpufreq
> -> `= dom0-kernel | none | xen[,[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]`
> +> `= none | {{ <boolean> | xen } [:[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]} | dom0-kernel`

If I am reading the parsing correctly below, the insertion if ':' is to
match the previous behaviour? (or have I missed something?)

~Andrew

>  
>  > Default: `xen`
>  
> -Indicate where the responsibility for driving power states lies.
> +Indicate where the responsibility for driving power states lies.  Note that the
> +choice of `dom0-kernel` is deprecated and not supported by all Dom0 kernels.
>  
>  * Default governor policy is ondemand.
>  * `<maxfreq>` and `<minfreq>` are integers which represent max and min processor frequencies
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -64,9 +64,14 @@ enum cpufreq_controller cpufreq_controll
>  
>  static void __init setup_cpufreq_option(char *str)
>  {
> -    char *arg;
> +    char *arg = strpbrk(str, ",:");
> +    int choice;
>  
> -    if ( !strcmp(str, "dom0-kernel") )
> +    if ( arg )
> +        *arg++ = '\0';
> +    choice = parse_bool(str);
> +
> +    if ( choice < 0 && !strcmp(str, "dom0-kernel") )
>      {
>          xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
>          cpufreq_controller = FREQCTL_dom0_kernel;
> @@ -74,19 +79,20 @@ static void __init setup_cpufreq_option(
>          return;
>      }
>  
> -    if ( !strcmp(str, "none") )
> +    if ( choice == 0 || !strcmp(str, "none") )
>      {
>          xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
>          cpufreq_controller = FREQCTL_none;
>          return;
>      }
>  
> -    if ( (arg = strpbrk(str, ",:")) != NULL )
> -        *arg++ = '\0';
> -
> -    if ( !strcmp(str, "xen") )
> +    if ( choice > 0 || !strcmp(str, "xen") )
> +    {
> +        xen_processor_pmbits |= XEN_PROCESSOR_PM_PX;
> +        cpufreq_controller = FREQCTL_xen;
>          if ( arg && *arg )
>              cpufreq_cmdline_parse(arg);
> +    }
>  }
>  custom_param("cpufreq", setup_cpufreq_option);
>  
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3470 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cpufreq: allow ordinary boolean options to be passed on the command line
  2015-10-30 19:22 ` Andrew Cooper
@ 2015-11-02 10:54   ` Jan Beulich
  2015-11-02 11:11     ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2015-11-02 10:54 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Jinsong Liu, xen-devel

>>> On 30.10.15 at 20:22, <andrew.cooper3@citrix.com> wrote:
> On 30/10/15 17:49, Jan Beulich wrote:
>> I was quite surprised to find "cpufreq=off" not doing what one would
>> expect it to do. Fix this.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -391,11 +391,12 @@ If set, force use of the performance cou
>>  available support.
>>  
>>  ### cpufreq
>> -> `= dom0-kernel | none | xen[,[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]`
>> +> `= none | {{ <boolean> | xen } [:[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]} | dom0-kernel`
> 
> If I am reading the parsing correctly below, the insertion if ':' is to
> match the previous behaviour? (or have I missed something?)

We've always supported , and : there. I do think that documenting
comma here is bad (since it's also used as the separator between
sub-options), and hence I've taken the opportunity to make this
match other command line options (where we generally use colon
for separation of main and sub-options).

Jan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cpufreq: allow ordinary boolean options to be passed on the command line
  2015-11-02 10:54   ` Jan Beulich
@ 2015-11-02 11:11     ` Andrew Cooper
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2015-11-02 11:11 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Jinsong Liu, xen-devel

On 02/11/15 10:54, Jan Beulich wrote:
>>>> On 30.10.15 at 20:22, <andrew.cooper3@citrix.com> wrote:
>> On 30/10/15 17:49, Jan Beulich wrote:
>>> I was quite surprised to find "cpufreq=off" not doing what one would
>>> expect it to do. Fix this.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -391,11 +391,12 @@ If set, force use of the performance cou
>>>  available support.
>>>  
>>>  ### cpufreq
>>> -> `= dom0-kernel | none | xen[,[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]`
>>> +> `= none | {{ <boolean> | xen } [:[powersave|performance|ondemand|userspace][,<maxfreq>][,[<minfreq>][,[verbose]]]]} | dom0-kernel`
>> If I am reading the parsing correctly below, the insertion if ':' is to
>> match the previous behaviour? (or have I missed something?)
> We've always supported , and : there. I do think that documenting
> comma here is bad (since it's also used as the separator between
> sub-options), and hence I've taken the opportunity to make this
> match other command line options (where we generally use colon
> for separation of main and sub-options).

Agreed.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-11-02 11:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-30 17:49 [PATCH] cpufreq: allow ordinary boolean options to be passed on the command line Jan Beulich
2015-10-30 19:22 ` Andrew Cooper
2015-11-02 10:54   ` Jan Beulich
2015-11-02 11:11     ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.