linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clocksource: arc_timer: RTC: allow registration despite SMP
@ 2017-02-02  0:50 Vineet Gupta
  2017-02-02 14:27 ` Daniel Lezcano
  0 siblings, 1 reply; 3+ messages in thread
From: Vineet Gupta @ 2017-02-02  0:50 UTC (permalink / raw)
  To: linux-snps-arc
  Cc: linux-kernel, Vineet Gupta, John Stultz, Thomas Gleixner, Daniel Lezcano

So far we didn't allow CPU private 64-bit RTC timer to register in SMP
as the individual counters may not be synchronized across cores.

However there is a situation when we build SMP kernel but want to use
the same image on UP as well as SMP hardware. Here we would certainly
want to use RTC, but current code doesn't allow as it only uses build
info (CONFIG_SMP) and not runtime info (num_online_cpus() or some such).

We can't possibly use num_online_cpus() anyways because clocksource
probe happens before other cpus are brought online

The simple fix is allow ETC probe for SMP and rely on higher rating of
GFRC to take over in general SMP case. We leave the pr_warn to notify
the user.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 drivers/clocksource/arc_timer.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clocksource/arc_timer.c b/drivers/clocksource/arc_timer.c
index 7517f959cba7..87f193794bf2 100644
--- a/drivers/clocksource/arc_timer.c
+++ b/drivers/clocksource/arc_timer.c
@@ -145,10 +145,8 @@ static int __init arc_cs_setup_rtc(struct device_node *node)
 	}
 
 	/* Local to CPU hence not usable in SMP */
-	if (IS_ENABLED(CONFIG_SMP)) {
+	if (IS_ENABLED(CONFIG_SMP))
 		pr_warn("Local-64-bit-Ctr not usable in SMP");
-		return -EINVAL;
-	}
 
 	ret = arc_get_timer_clk(node);
 	if (ret)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] clocksource: arc_timer: RTC: allow registration despite SMP
  2017-02-02  0:50 [PATCH] clocksource: arc_timer: RTC: allow registration despite SMP Vineet Gupta
@ 2017-02-02 14:27 ` Daniel Lezcano
  2017-02-22  1:16   ` Vineet Gupta
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Lezcano @ 2017-02-02 14:27 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: linux-snps-arc, linux-kernel, John Stultz, Thomas Gleixner

On Wed, Feb 01, 2017 at 04:50:15PM -0800, Vineet Gupta wrote:
> So far we didn't allow CPU private 64-bit RTC timer to register in SMP
> as the individual counters may not be synchronized across cores.

That is the case for the other archs and the per cpu clocksource are used, the
kernel is supposed to be immune against clocks drift. There are mechanisms to
have a synchronized view of the clock when they are per cpu and the kernel does
not compare clocks between cpus. 

I didn't realize that when we did the modification around this check.

May be you can double check these clocksources are really not suitable for SMP,
because if it is not the case the if !CONFIG_SMP could be simply removed.
 
A sidenote: RTC is confusing and should be changed to something else.

> However there is a situation when we build SMP kernel but want to use
> the same image on UP as well as SMP hardware. Here we would certainly
> want to use RTC, but current code doesn't allow as it only uses build
> info (CONFIG_SMP) and not runtime info (num_online_cpus() or some such).
> 
> We can't possibly use num_online_cpus() anyways because clocksource
> probe happens before other cpus are brought online
> 
> The simple fix is allow ETC probe for SMP and rely on higher rating of
> GFRC to take over in general SMP case. We leave the pr_warn to notify
> the user.
> 
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
>  drivers/clocksource/arc_timer.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arc_timer.c b/drivers/clocksource/arc_timer.c
> index 7517f959cba7..87f193794bf2 100644
> --- a/drivers/clocksource/arc_timer.c
> +++ b/drivers/clocksource/arc_timer.c
> @@ -145,10 +145,8 @@ static int __init arc_cs_setup_rtc(struct device_node *node)
>  	}
>  
>  	/* Local to CPU hence not usable in SMP */
> -	if (IS_ENABLED(CONFIG_SMP)) {
> +	if (IS_ENABLED(CONFIG_SMP))
>  		pr_warn("Local-64-bit-Ctr not usable in SMP");
> -		return -EINVAL;
> -	}
>  
>  	ret = arc_get_timer_clk(node);
>  	if (ret)
> -- 
> 2.7.4
> 

-- 

 <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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] clocksource: arc_timer: RTC: allow registration despite SMP
  2017-02-02 14:27 ` Daniel Lezcano
@ 2017-02-22  1:16   ` Vineet Gupta
  0 siblings, 0 replies; 3+ messages in thread
From: Vineet Gupta @ 2017-02-22  1:16 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: linux-snps-arc, linux-kernel, John Stultz, Thomas Gleixner

Hi Daniel,

Sorry for delayed response, was away from Linux for a bit :-(

On 02/02/2017 06:27 AM, Daniel Lezcano wrote:
> On Wed, Feb 01, 2017 at 04:50:15PM -0800, Vineet Gupta wrote:
>> So far we didn't allow CPU private 64-bit RTC timer to register in SMP
>> as the individual counters may not be synchronized across cores.
> 
> That is the case for the other archs and the per cpu clocksource are used, the
> kernel is supposed to be immune against clocks drift. There are mechanisms to
> have a synchronized view of the clock when they are per cpu and the kernel does
> not compare clocks between cpus. 

Right, we have tick broadcast etc. However as discussed before I really don't like
the overhead - specially given that SMP cores will have a truly cross-core-sync
GFRC timer and will serve as clocksrc anyways.

> I didn't realize that when we did the modification around this check.
> 
> May be you can double check these clocksources are really not suitable for SMP,
> because if it is not the case the if !CONFIG_SMP could be simply removed.

Technically with tick broadcast RTC could be used, but I would like to avoid that.
The usecase here is a CONFIG_SMP built kernel running on a UP only hardware (I was
playing with running same binary on UP / SMP hardware).

However it seems CONFIG_SMP overhead seems to be significant for production UP
hardware so this patch might not be important after all.

> A sidenote: RTC is confusing and should be changed to something else.

Yeah it is - but that is what hardware guys decided to name it

-Vineet

> 
>> However there is a situation when we build SMP kernel but want to use
>> the same image on UP as well as SMP hardware. Here we would certainly
>> want to use RTC, but current code doesn't allow as it only uses build
>> info (CONFIG_SMP) and not runtime info (num_online_cpus() or some such).
>>
>> We can't possibly use num_online_cpus() anyways because clocksource
>> probe happens before other cpus are brought online
>>
>> The simple fix is allow ETC probe for SMP and rely on higher rating of
>> GFRC to take over in general SMP case. We leave the pr_warn to notify
>> the user.
>>
>> Cc: John Stultz <john.stultz@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>> ---
>>  drivers/clocksource/arc_timer.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/arc_timer.c b/drivers/clocksource/arc_timer.c
>> index 7517f959cba7..87f193794bf2 100644
>> --- a/drivers/clocksource/arc_timer.c
>> +++ b/drivers/clocksource/arc_timer.c
>> @@ -145,10 +145,8 @@ static int __init arc_cs_setup_rtc(struct device_node *node)
>>  	}
>>  
>>  	/* Local to CPU hence not usable in SMP */
>> -	if (IS_ENABLED(CONFIG_SMP)) {
>> +	if (IS_ENABLED(CONFIG_SMP))
>>  		pr_warn("Local-64-bit-Ctr not usable in SMP");
>> -		return -EINVAL;
>> -	}
>>  
>>  	ret = arc_get_timer_clk(node);
>>  	if (ret)
>> -- 
>> 2.7.4
>>
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-02-22  1:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-02  0:50 [PATCH] clocksource: arc_timer: RTC: allow registration despite SMP Vineet Gupta
2017-02-02 14:27 ` Daniel Lezcano
2017-02-22  1:16   ` Vineet Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).