Question: ARM: local timers: add num_possible_cpus() in local_timer_register()

Question: ARM: local timers: add num_possible_cpus() in local_timer_register()

[Steve zhan]

Hi Marc,

	Do you think we should add num_possible_cpus() in 
local_timer_register() function, When core number is 
> 1, then reutrn TRUE? Enable the local timer only if 
we have more than one CPU, if the current config is
SMP and setup_max_cpus >= 1, but if smp_init_cpus in 
platform code have set cpu possible value to One(that 
maybe read number from scu or other hardware), then 
percpu_timer_setup in smp_prepare_cpus have no chance
to be called.

	For local timer, because local_timer_register have
return 1, that will miss global timer register. 

	Could you pls check it?


Regards,
Steve.

Question: ARM: local timers: add num_possible_cpus() in local_timer_register()

[Russell King - ARM Linux]

On Mon, Jan 14, 2013 at 09:33:45PM +0800, Steve zhan wrote:
> Hi Marc,
> 
> 	Do you think we should add num_possible_cpus() in 
> local_timer_register() function, When core number is 
> > 1, then reutrn TRUE? Enable the local timer only if 
> we have more than one CPU, if the current config is
> SMP and setup_max_cpus >= 1, but if smp_init_cpus in 
> platform code have set cpu possible value to One(that 
> maybe read number from scu or other hardware), then 
> percpu_timer_setup in smp_prepare_cpus have no chance
> to be called.
> 
> 	For local timer, because local_timer_register have
> return 1, that will miss global timer register. 

What about platforms where all they have are the local timers and no
global timer?

Question: ARM: local timers: add num_possible_cpus() in local_timer_register()

[Marc Zyngier]

On 14/01/13 13:33, Steve zhan wrote:

Hi Steve,

> 	Do you think we should add num_possible_cpus() in 
> local_timer_register() function, When core number is 
>> 1, then reutrn TRUE? Enable the local timer only if 
> we have more than one CPU, if the current config is
> SMP and setup_max_cpus >= 1, but if smp_init_cpus in 
> platform code have set cpu possible value to One(that 
> maybe read number from scu or other hardware), then 
> percpu_timer_setup in smp_prepare_cpus have no chance
> to be called.
> 
> 	For local timer, because local_timer_register have
> return 1, that will miss global timer register. 
> 
> 	Could you pls check it?

I'm not sure I understand your question.

We only enable the use of the SMP local timers infrastructure if we're
SMP and we have more that one CPU. If you only have one CPU up and
running, then you can still use your CPU local timer as a global timer.

What would we gain by using the local timer infrastructure when we're
not running on SMP hardware?

	M.
-- 
Jazz is not dead. It just smells funny...

Question: ARM: local timers: add num_possible_cpus() in local_timer_register()

[steve.zhan]

Hi,
    Russel, Marc,
2013/1/14 Marc Zyngier <marc.zyngier@arm.com>:
> On 14/01/13 13:33, Steve zhan wrote:
>
> Hi Steve,
>
>>       Do you think we should add num_possible_cpus() in
>> local_timer_register() function, When core number is
>>> 1, then reutrn TRUE? Enable the local timer only if
>> we have more than one CPU, if the current config is
>> SMP and setup_max_cpus >= 1, but if smp_init_cpus in
>> platform code have set cpu possible value to One(that
>> maybe read number from scu or other hardware), then
>> percpu_timer_setup in smp_prepare_cpus have no chance
>> to be called.
>>
>>       For local timer, because local_timer_register have
>> return 1, that will miss global timer register.
>>
>>       Could you pls check it?
>
> I'm not sure I understand your question.
>
> We only enable the use of the SMP local timers infrastructure if we're
> SMP and we have more that one CPU. If you only have one CPU up and
> running, then you can still use your CPU local timer as a global timer.
>
> What would we gain by using the local timer infrastructure when we're
> not running on SMP hardware?
>
>         M.
> --
> Jazz is not dead. It just smells funny...
>

For these code:
        err = local_timer_register(&arch_timer_ops);
        if (err) {
                /*
                 * We couldn't register as a local timer (could be
                 * because we're on a UP platform, or because some
                 * other local timer is already present...). Try as a
                 * global timer instead.
                 */
                arch_timer_global_evt.cpumask = cpumask_of(0);
                err = arch_timer_setup(&arch_timer_global_evt);
        }

if err == 0, we have no register global arch timer,  but platform code
platsmp.c will set
cpu possible number to one, that let percpu_timer_setup in
smp_prepare_cpus have no
chance to call becaue ncores = num_possible_cpus() is one.
Is this right?


-- 
Steve Zhan

Question: ARM: local timers: add num_possible_cpus() in local_timer_register()

[Marc Zyngier]

On 14/01/13 14:31, steve.zhan wrote:
> Hi,
>     Russel, Marc,
> 2013/1/14 Marc Zyngier <marc.zyngier@arm.com>:
>> On 14/01/13 13:33, Steve zhan wrote:
>>
>> Hi Steve,
>>
>>>       Do you think we should add num_possible_cpus() in
>>> local_timer_register() function, When core number is
>>>> 1, then reutrn TRUE? Enable the local timer only if
>>> we have more than one CPU, if the current config is
>>> SMP and setup_max_cpus >= 1, but if smp_init_cpus in
>>> platform code have set cpu possible value to One(that
>>> maybe read number from scu or other hardware), then
>>> percpu_timer_setup in smp_prepare_cpus have no chance
>>> to be called.
>>>
>>>       For local timer, because local_timer_register have
>>> return 1, that will miss global timer register.
>>>
>>>       Could you pls check it?
>>
>> I'm not sure I understand your question.
>>
>> We only enable the use of the SMP local timers infrastructure if we're
>> SMP and we have more that one CPU. If you only have one CPU up and
>> running, then you can still use your CPU local timer as a global timer.
>>
>> What would we gain by using the local timer infrastructure when we're
>> not running on SMP hardware?
>>
>>         M.
>> --
>> Jazz is not dead. It just smells funny...
>>
> 
> For these code:
>         err = local_timer_register(&arch_timer_ops);
>         if (err) {
>                 /*
>                  * We couldn't register as a local timer (could be
>                  * because we're on a UP platform, or because some
>                  * other local timer is already present...). Try as a
>                  * global timer instead.
>                  */
>                 arch_timer_global_evt.cpumask = cpumask_of(0);
>                 err = arch_timer_setup(&arch_timer_global_evt);
>         }
> 
> if err == 0, we have no register global arch timer,  but platform code
> platsmp.c will set
> cpu possible number to one, that let percpu_timer_setup in
> smp_prepare_cpus have no
> chance to call becaue ncores = num_possible_cpus() is one.
> Is this right?

Ah, I get it now. Yes, this is a problem. Mark Rutland's patch series
actually fixes this issue entirely though. You may want to check it out.

	M.
-- 
Jazz is not dead. It just smells funny...