Re: [patch 2/2] x86: put offline CPUs into deepest mwaitcstate_subcstate

[Pallipadi, Venkatesh]

On Sun, 2009-05-24 at 17:56 -0700, Li, Shaohua wrote:
> On Sat, May 23, 2009 at 07:19:42AM +0800, Pallipadi, Venkatesh wrote:
> > Offline CPUs can save power by going into deepest cstate, subcstate
> > instead of hlt loop.
> > 
> > Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
> > ---
> >  arch/x86/kernel/acpi/cstate.c |   51 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 51 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
> > index bbbe4bb..5b0988a 100644
> > --- a/arch/x86/kernel/acpi/cstate.c
> > +++ b/arch/x86/kernel/acpi/cstate.c
> > @@ -150,6 +150,54 @@ void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cx)
> >  }
> >  EXPORT_SYMBOL_GPL(acpi_processor_ffh_cstate_enter);
> >  
> > +static unsigned long mwait_play_dead_eax;
> > +
> > +static void mwait_play_dead(void)
> > +{
> > +	if (boot_cpu_data.x86 >= 4)
> > +		wbinvd();
> > +
> > +	while (1) {
> > +		__monitor((void *)&current_thread_info()->flags, 0, 0);
> > +		smp_mb();
> > +		__mwait(mwait_play_dead_eax, 0);
> > +	}
> > +}
> CPU is dead, can current_thread_info() still be used? Maybe just monitor a never changed
> address.
> 

We still execute in this while loop even for an offline CPU, in case CPU
just wakes up for whatever reason (HT sibling woke up etc). So, we
should have stack and current_thread_info() even when offline, and we
can use it here. I had a never changing variable in my earlier version
of this patch. But removed it as I don't see why we should have couple
of cache lines of data when we can avoid it.

> Looks the patch will always take the highest native C-state, is this safe, considering
> BIOS usually limit C-state?
> 

It may not be safe in terms of wakeup latency etc. So, we use BIOS CST
for normal C-states. When offline, we don't really care about latency
part. If CPU supports a C-state, we should be able to enter it and we
can save most power with deepest C-state. If there are functionality
issues with C-state, I am sure CPU feature (hw or microcode) will have
it disabled, instead of depending on BIOSes to disable it.

Thanks,
Venki

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