RE: + restore-missing-sysfs-max_cstate-attr.patch added to -mm tree

["Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>]

 

>-----Original Message-----
>From: Andrew Morton [mailto:akpm@linux-foundation.org] 
>Sent: Sunday, January 06, 2008 11:19 PM
>To: Mark Lord
>Cc: Pallipadi, Venkatesh; Arjan van de Ven; abelay@novell.com; 
>lenb@kernel.org; Ingo Molnar; linux-kernel@vger.kernel.org; 
>linux-acpi@vger.kernel.org; rjw@sisk.pl
>Subject: Re: + restore-missing-sysfs-max_cstate-attr.patch 
>added to -mm tree
>
>On Sun, 06 Jan 2008 16:34:16 -0500 Mark Lord <lkml@rtr.ca> wrote:
>
>> Venki Pallipadi wrote:
>> > Reintroduce run time configurable max_cstate for !CPU_IDLE case.
>> > 
>> Can we get this patch upstream so that a stock 2.6.24 will work here?
>> 
>
>umm, OK, I queued it for 2.6.24.  I'll give people a day or so 
>to comment
>on this.
>
>I had to invent some silly changlelog for it.  Please review it for
>accuracy and completeness?
>
>It isn't complete, really.  How come we only make max_cstate 
>writeable if
>CONFIG_CPU_IDLE=n?  What happens to people who were reliant 
>upon writeable
>max_cstate who now enable CPU_IDLE?  Things still break?  What is the
>rationale behind this?  What constraints led us to this decision?

It is done only for !CPU_IDLE case to take care of regression at hand.
CPU_IDLE case technically is not a regression as it is a new config
option. It is not easy to implement this with CPU_IDLE as acpi driver
only provides the C-state mechanism and does not have the policy in it
anymore with CPU_IDLE. It still can be done with some hacky code. But, I
am incliced to switch this to using latency interface which is more
cleaner than max_cstate interface.

For example, max_cstate does not mean anything to the user, as BIOSes
normally tend to hide one C-state or more than one C-states behind one
OS visible C-state. Like C2 mapped to real C3 etc. Saying that I don't
want CPUs to enter any C-state more than 100uS latency is cleaner in
comparison (even though we depend on the latency number coming from the
BIOS).

Mark said this latency interface is not working as it is expected to at
this moment. I will look at that soon and then we will have an alternate
mechanism for this limiting C-state thing.

I am OK with the below changelog.

Thanks,
Venki

>From: Venki Pallipadi <venkatesh.pallipadi@intel.com>
>
>This was writeable in 2.6.23 but the cpuidle merge made it 
>read-only.  But
>some people's scripts (ie: Mark's) were writing to it.
>
>As an unhappy compromise, make max_cstate writeable again if 
>the kernel was
>configured without CONFIG_CPU_IDLE.
>
>Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
>Cc: Mark Lord <lkml@rtr.ca>
>Cc: Arjan van de Ven <arjan@infradead.org>
>Cc: Len Brown <lenb@kernel.org>
>Cc: Ingo Molnar <mingo@elte.hu>
>Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>---
>
> drivers/acpi/processor_idle.c |    4 ++++
> 1 file changed, 4 insertions(+)
>
>diff -puN 
>drivers/acpi/processor_idle.c~reintroduce-run-time-configurable
>-max_cstate-for-cpu_idle-case drivers/acpi/processor_idle.c
>--- 
>a/drivers/acpi/processor_idle.c~reintroduce-run-time-configurab
>le-max_cstate-for-cpu_idle-case
>+++ a/drivers/acpi/processor_idle.c
>@@ -76,7 +76,11 @@ static void (*pm_idle_save) (void) __rea
> #define PM_TIMER_TICKS_TO_US(p)		(((p) * 
>1000)/(PM_TIMER_FREQUENCY/1000))
> 
> static unsigned int max_cstate __read_mostly = 
>ACPI_PROCESSOR_MAX_POWER;
>+#ifdef CONFIG_CPU_IDLE
> module_param(max_cstate, uint, 0000);
>+#else
>+module_param(max_cstate, uint, 0644);
>+#endif
> static unsigned int nocst __read_mostly;
> module_param(nocst, uint, 0000);
> 
>_
>
>