[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 1/3] cpuidle: If disable_cpuidle() is called, set pm_idle to default_idle.
- To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
- From: Deepthi Dharwar <deepthi@xxxxxxxxxxxxxxxxxx>
- Date: Wed, 23 Nov 2011 16:36:24 +0530
- Cc: len.brown@xxxxxxxxx, jeremy@xxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx, x86@xxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, stable@xxxxxxxxxx, mingo@xxxxxxxxxx, bp@xxxxxxxxx, hpa@xxxxxxxxx, tj@xxxxxxxxxx, tglx@xxxxxxxxxxxxx, trenn@xxxxxxx
- Delivery-date: Wed, 23 Nov 2011 11:07:38 +0000
- List-id: Xen developer discussion <xen-devel.lists.xensource.com>
Hi Konrad,
On 11/22/2011 07:16 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 15, 2011 at 09:40:04AM -0500, Konrad Rzeszutek Wilk wrote:
>>> Well I was with a view that if cpuidle is disabled, mwait and arm_e400
>>> would take precedence over default_idle. But if is ok to have default_idle instead,
>>> then go ahead.
>>>
>>>> Perhaps there is another way do this is:
>>>>
>>>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>>>> index becd6d9..04b10a4 100644
>>>> --- a/drivers/cpuidle/cpuidle.c
>>>> +++ b/drivers/cpuidle/cpuidle.c
>>>> @@ -38,6 +38,7 @@ int cpuidle_disabled(void)
>>>> void disable_cpuidle(void)
>>>> {
>>>> off = 1;
>>>> + pm_idle = default_idle;
>>>> }
>>>>
>>> Brining pm_idle pointer back to cpuidle code is going a step back coz
>>> we wanted to complete remove using pm_idle going forward. As having
>>> a pointer exported into various files is not a good thing. That's the
>>> reason we started the clean up in the first place :)
>>
>> Perhaps then something along these lines, where we still set default_idle
>> but don't fiddle with the pm_idle (and can still rip out the
>> EXPORT_SYMBOL_GPL(default_idle) in 2012):
>>
>> (Hadn't tested this yet).
>
> Now have tested it.
>
As long as you dont bring back exporting pm_idle and using it in
cpuidle code
it should be ok. So one should not use pm_idle in
disable_cpuidle function.
>>
>> diff --git a/arch/x86/include/asm/system.h b/arch/x86/include/asm/system.h
>> index c2ff2a1..2d2f01c 100644
>> --- a/arch/x86/include/asm/system.h
>> +++ b/arch/x86/include/asm/system.h
>> @@ -401,6 +401,7 @@ extern unsigned long arch_align_stack(unsigned long sp);
>> extern void free_init_pages(char *what, unsigned long begin, unsigned long end);
>>
>> void default_idle(void);
>> +bool set_pm_idle_to_default(void);
>>
>> void stop_this_cpu(void *dummy);
>>
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> index e7e3b01..bfb113f 100644
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -403,6 +403,14 @@ void default_idle(void)
>> EXPORT_SYMBOL(default_idle);
>> #endif
>>
>> +bool set_pm_idle_to_default()
>> +{
>> + if (!pm_idle) {
>> + pm_idle = default_idle;
>> + return true;
>> + }
>> + return false;
>> +}
>> void stop_this_cpu(void *dummy)
>> {
>> local_irq_disable();
>> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
>> index 46d6d21..7506181 100644
>> --- a/arch/x86/xen/setup.c
>> +++ b/arch/x86/xen/setup.c
>> @@ -448,6 +448,6 @@ void __init xen_arch_setup(void)
>> #endif
>> disable_cpuidle();
>> boot_option_idle_override = IDLE_HALT;
>> -
>> + WARN_ON(!set_pm_idle_to_default());
>> fiddle_vdso();
>> }
>>
This looks good, where we see setting of default_idle only in
Xen setup code and not the in the generic cpuidle code path.
This would allow other architectures to use mwait or arm_e400
to be enabled when cpuidle is disabled.
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>
Cheers,
Deepthi
|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|