On 10/07/2012 11:26 PM, Rafael J. Wysocki wrote:
> On Tuesday 25 of September 2012 00:43:54 Daniel Lezcano wrote:
>> With the tegra3 and the big.LITTLE [1] new architectures, several cpus
>> with different characteristics (latencies and states) can co-exists on the
>> system.
>>
>> The cpuidle framework has the limitation of handling only identical cpus.
>>
>> This patch removes this limitation by introducing the multiple driver support
>> for cpuidle.
>>
>> This option is configurable at compile time and should be enabled for the
>> architectures mentioned above. So there is no impact for the other platforms
>> if the option is disabled. The option defaults to 'n'. Note the multiple
>> drivers
>> support is also compatible with the existing drivers, even if just one
>> driver is
>> needed, all the cpu will be tied to this driver using an extra small chunk of
>> processor memory.
>>
>> The multiple driver support use a per-cpu driver pointer instead of a global
>> variable and the accessor to this variable are done from a cpu context.
Thanks Rafael for the review.
I took into account all your remarks for the V2.
[ cut ]
>> +static int __cpuidle_register_all_cpu_driver(struct cpuidle_driver *drv)
>> +{
>> + int ret = 0;
>> + int i, cpu;
>> +
>> + for_each_present_cpu(cpu) {
>> + ret = __cpuidle_register_driver(drv, cpu);
>> + if (!ret)
>> + continue;
>> + for (i = cpu - 1; i >= 0; i--)
> I wonder if this is going to work in all cases. For example, is there any
> guarantee that the CPU numbers start from 0 and that there are no gaps?
AFAICS, the cpumask.h is not assuming the cpu numbers start from zero
and they are contiguous.
I will fix this reverse loop, thanks for spotting this.
[ cut ]
>> void cpuidle_unregister_driver(struct cpuidle_driver *drv)
>> {
>> spin_lock(&cpuidle_driver_lock);
>> - __cpuidle_unregister_driver(drv);
>> +#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
>> + __cpuidle_unregister_all_cpu_driver(drv);
>> +#else
>> + __cpuidle_unregister_driver(drv, smp_processor_id());
>> +#endif
> I'm slightly cautious about using smp_processor_id() above.
> get_cpu()/put_cpu() is the preferred way of doing this nowadays (although
> this particular code is under the spinlock, so it should be OK).
yes, get_cpu does preempt_disable() and smp_processor_id()
As spin_lock does also preempt_disable() that should be ok.
But I changed the code to use the preferred way.
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
