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

[Arjan van de Ven <arjan@infradead.org>]

On Sun, 6 Jan 2008 23:18:48 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> 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.
> > > 
> > > Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > > 
> > > Index: linux-2.6.24-rc/drivers/acpi/processor_idle.c
> > > ===================================================================
> > > --- linux-2.6.24-rc.orig/drivers/acpi/processor_idle.c
> > > +++ linux-2.6.24-rc/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);
> > >  
> > ..
> > 
> > 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?



if we take a step back; Mark afaics only wants to put 1 in there...
And that makes sense; either you want the "no latency" C1, or you want the lot
(esp given that C2 and deeper are at the whim of the bios, what they mean varies
over time. Actually even C1 does that on some AMD systems);

Longer term I'd suggest we make an option that basically is "C1 only",
(or technically, "use hlt only")
that solves Marks VMWARE thing, and is a lot closer to what people really want.
Well, that and if VMWARE really can't deal with latency in their kernel module
they should use the proper code for that. It's also a ton easier to implement, since
it basically is "don't use the CPUIDLE idle loop, but use the traditional hlt one"