Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-14 Thread Santosh Shilimkar
On Thursday 14 March 2013 12:01 AM, Thomas Gleixner wrote:
 On Wed, 13 Mar 2013, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 07:49 PM, Thomas Gleixner wrote:
 Though making the rating of the dummy lower is definitely a good
 thing, so a real hardware device which is detected later can replace
 the dummy device. So yes, the rating should be low for the dummy
 timer.

 Exactly. As Mark pointed out, you have already applied Mark's patch.
 I was just wondering whether you can take the $subject patch as well
 with ack from Russell, Mark.
 
 It can go through the ARM tree as well, no dependencies on my stuff.
 
Sure. I will push that through ARM tree.

Regards,
Santosh

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-14 Thread Thomas Gleixner
On Thu, 14 Mar 2013, Santosh Shilimkar wrote:

 On Wednesday 13 March 2013 09:48 PM, Mark Rutland wrote:
  On Wed, Mar 13, 2013 at 03:44:03PM +, Santosh Shilimkar wrote:
  On Wednesday 13 March 2013 05:55 PM, Mark Rutland wrote:
  On Wed, Mar 13, 2013 at 11:24:01AM +, Santosh Shilimkar wrote:
  On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote:
  Hi Santosh,
 
  [..]
 
 
  Is the problem that the dummy timer is being registered as the broadcast
  source, or that it is selected as a local timer in preference of real 
  timers?
 
  Dummy timer is preferred over real broadcast timer.
 
  Ok.
  
  [...]
  
  I do agree it'd be worth lowering the dummy timer's rating to ensure it 
  doesn't
  override a real timer elsewhere. 
 
  Yep. Can I add you acked-by tag then for $subject patch ?
  Would be good to get this one merged as well.
  
  Sure. Though could you reword the commit message? The patch solves the more
  general issue of a dummy being preferred over real hardware even outside of
  choosing the broadcast device.
  
  Acked-by: Mark Rutland mark.rutl...@arm.com
 
 Thanks. For record, patch is in end of the email which I plan
 to put into patch system.
 
 Regards,
 Santosh
 
 From 57c501bcdc88c7ff26a5c63956be07e94a5083c5 Mon Sep 17 00:00:00 2001
 From: Santosh Shilimkar santosh.shilim...@ti.com
 Date: Wed, 13 Mar 2013 12:33:16 +0530
 Subject: [PATCH 1/2] ARM: smp: Avoid dummy clockevent being preferred over 
 real hardware clock-event
 
 With recent arm broadcast time clean-up from Mark Rutland, the dummy
 broadcast device is always registered with timer subsystem. And since
 the rating of the dummy clock event is very high, it may be preferred
 over a real clock event.
 
 This is a change in behavior from past and not an intended
 one. So reduce the rating of the dummy clockevent so that
 real clockevent device is selected when available.
 
 Acked-by: Mark Rutland mark.rutl...@arm.com

Acked-by: Thomas Gleixner t...@linutronix.de

 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 ---
  arch/arm/kernel/smp.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
 index 31644f1..79078ed 100644
 --- a/arch/arm/kernel/smp.c
 +++ b/arch/arm/kernel/smp.c
 @@ -480,7 +480,7 @@ static void __cpuinit broadcast_timer_setup(struct 
 clock_event_device *evt)
   evt-features   = CLOCK_EVT_FEAT_ONESHOT |
 CLOCK_EVT_FEAT_PERIODIC |
 CLOCK_EVT_FEAT_DUMMY;
 - evt-rating = 400;
 + evt-rating = 100;
   evt-mult   = 1;
   evt-set_mode   = broadcast_timer_set_mode;
  
 -- 
 1.7.9.5
 
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-14 Thread Mark Rutland
On Thu, Mar 14, 2013 at 07:45:14AM +, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 09:48 PM, Mark Rutland wrote:
  On Wed, Mar 13, 2013 at 03:44:03PM +, Santosh Shilimkar wrote:
  On Wednesday 13 March 2013 05:55 PM, Mark Rutland wrote:
  On Wed, Mar 13, 2013 at 11:24:01AM +, Santosh Shilimkar wrote:
  On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote:
  Hi Santosh,
 
  [..]
 
 
  Is the problem that the dummy timer is being registered as the broadcast
  source, or that it is selected as a local timer in preference of real 
  timers?
 
  Dummy timer is preferred over real broadcast timer.
 
  Ok.
  
  [...]
  
  I do agree it'd be worth lowering the dummy timer's rating to ensure it 
  doesn't
  override a real timer elsewhere. 
 
  Yep. Can I add you acked-by tag then for $subject patch ?
  Would be good to get this one merged as well.
  
  Sure. Though could you reword the commit message? The patch solves the more
  general issue of a dummy being preferred over real hardware even outside of
  choosing the broadcast device.
  
  Acked-by: Mark Rutland mark.rutl...@arm.com
 
 Thanks. For record, patch is in end of the email which I plan
 to put into patch system.
 
 Regards,
 Santosh

The below patch seems fine. Are you intending for this to go in as a fix for
3.9-rc*, or as a cleanup for 3.10?

If you're aiming for the latter, it's going to clash with Stephen Boyd's local
timer API removal [1], in which the generic dummy timer driver [2] (replacing
the arm-specific dummy [3]) also has a rating of 100.

It would be nice if we could reduce the possibility of a conflict.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/154724.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/154725.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/154726.html

Thanks,
Mark.

 
 From 57c501bcdc88c7ff26a5c63956be07e94a5083c5 Mon Sep 17 00:00:00 2001
 From: Santosh Shilimkar santosh.shilim...@ti.com
 Date: Wed, 13 Mar 2013 12:33:16 +0530
 Subject: [PATCH 1/2] ARM: smp: Avoid dummy clockevent being preferred over 
 real hardware clock-event
 
 With recent arm broadcast time clean-up from Mark Rutland, the dummy
 broadcast device is always registered with timer subsystem. And since
 the rating of the dummy clock event is very high, it may be preferred
 over a real clock event.
 
 This is a change in behavior from past and not an intended
 one. So reduce the rating of the dummy clockevent so that
 real clockevent device is selected when available.
 
 Acked-by: Mark Rutland mark.rutl...@arm.com
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 ---
  arch/arm/kernel/smp.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
 index 31644f1..79078ed 100644
 --- a/arch/arm/kernel/smp.c
 +++ b/arch/arm/kernel/smp.c
 @@ -480,7 +480,7 @@ static void __cpuinit broadcast_timer_setup(struct 
 clock_event_device *evt)
   evt-features   = CLOCK_EVT_FEAT_ONESHOT |
 CLOCK_EVT_FEAT_PERIODIC |
 CLOCK_EVT_FEAT_DUMMY;
 - evt-rating = 400;
 + evt-rating = 100;
   evt-mult   = 1;
   evt-set_mode   = broadcast_timer_set_mode;
  
 -- 
 1.7.9.5
 
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-14 Thread Stephen Boyd
On 03/14/13 03:28, Mark Rutland wrote:
 On Thu, Mar 14, 2013 at 07:45:14AM +, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 09:48 PM, Mark Rutland wrote:
 On Wed, Mar 13, 2013 at 03:44:03PM +, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 05:55 PM, Mark Rutland wrote:
 I do agree it'd be worth lowering the dummy timer's rating to ensure it 
 doesn't
 override a real timer elsewhere. 

 Yep. Can I add you acked-by tag then for $subject patch ?
 Would be good to get this one merged as well.
 Sure. Though could you reword the commit message? The patch solves the more
 general issue of a dummy being preferred over real hardware even outside of
 choosing the broadcast device.

 Acked-by: Mark Rutland mark.rutl...@arm.com
 Thanks. For record, patch is in end of the email which I plan
 to put into patch system.

 Regards,
 Santosh
 The below patch seems fine. Are you intending for this to go in as a fix for
 3.9-rc*, or as a cleanup for 3.10?

 If you're aiming for the latter, it's going to clash with Stephen Boyd's local
 timer API removal [1], in which the generic dummy timer driver [2] (replacing
 the arm-specific dummy [3]) also has a rating of 100.

Thanks for the heads up. Looks like the conflict will be trivial, but I
wonder why need to put the patch at all? Per my understanding the
regression has been fixed by your patch in Thomas' tree and then in 3.10
we can merge the local timer removal patches and fix up the rating at
the same time.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Santosh Shilimkar
(Forgot to CC Thomas)

On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote:
 With recent arm broadcast time clean-up from Mark Rutland, the dummy
 broadcast device is always registered with timer subsystem. And since
 the rating of the dummy clock event is very high, it is preferred
 over a real broad-cast clock event.
 
 This is a change in behavior from past and not an intended
 one. So reduce the rating of the dummy clockevent so that
 real broadcast device is selected when available.
 
 Without this all the C states with C3STOP won't work since
 the broad cast notifier will take an abort.
 
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Russell King li...@arm.linux.org.uk
 
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 ---
 Its a regression so hopefully can get into the 3.9-rcx. Noticed
 this one on A15 platform. A9 platform the issue may not be seen
 since the local timer check avoids dummy timer registration.
 
Some one pointed me to a fix made by Mark which was discussed
under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject.
That patch seems to be more of work around since the root of the
problem is incorrect dummy timer rating. Either way, both patches
fix the issue.

  arch/arm/kernel/smp.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
 index 31644f1..79078ed 100644
 --- a/arch/arm/kernel/smp.c
 +++ b/arch/arm/kernel/smp.c
 @@ -480,7 +480,7 @@ static void __cpuinit broadcast_timer_setup(struct 
 clock_event_device *evt)
   evt-features   = CLOCK_EVT_FEAT_ONESHOT |
 CLOCK_EVT_FEAT_PERIODIC |
 CLOCK_EVT_FEAT_DUMMY;
 - evt-rating = 400;
 + evt-rating = 100;
   evt-mult   = 1;
   evt-set_mode   = broadcast_timer_set_mode;
  
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Mark Rutland
Hi Santosh,

On Wed, Mar 13, 2013 at 09:28:22AM +, Santosh Shilimkar wrote:
 (Forgot to CC Thomas)
 
 On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote:
  With recent arm broadcast time clean-up from Mark Rutland, the dummy
  broadcast device is always registered with timer subsystem. And since
  the rating of the dummy clock event is very high, it is preferred
  over a real broad-cast clock event.
  
  This is a change in behavior from past and not an intended
  one. So reduce the rating of the dummy clockevent so that
  real broadcast device is selected when available.
  
  Without this all the C states with C3STOP won't work since
  the broad cast notifier will take an abort.
  
  Cc: Mark Rutland mark.rutl...@arm.com
  Cc: Russell King li...@arm.linux.org.uk
  
  Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
  ---
  Its a regression so hopefully can get into the 3.9-rcx. Noticed
  this one on A15 platform. A9 platform the issue may not be seen
  since the local timer check avoids dummy timer registration.
  
 Some one pointed me to a fix made by Mark which was discussed
 under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject.
 That patch seems to be more of work around since the root of the
 problem is incorrect dummy timer rating. Either way, both patches
 fix the issue.

Is the problem that the dummy timer is being registered as the broadcast
source, or that it is selected as a local timer in preference of real timers?

If it is the former, Then I believe my patch solve the issue more generally -
if you happen to register a dummy timer before other timers, it will become the
broadcast source. Regardless of how temporary this is, it should never happen,
and lowering the rating of the dummy won't fix this.

If it is the latter, then this patch would ensure that a real timer with a
rating over 100 is selected in preference to the dummy, which is certainly what
we want. The proposed generic dummy timer in Stephen Boyd's local timer API
removal series [1] similarly uses a low rating to ensure that real timers are
selected in preference to an always-registered dummy. I note that the
architected timer has a higher rating (450) than the dummy (400), so this
shouldn't currently be a problem.

Have I missed something?

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/153208.html

Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Santosh Shilimkar
On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote:
 Hi Santosh,
 
 On Wed, Mar 13, 2013 at 09:28:22AM +, Santosh Shilimkar wrote:
 (Forgot to CC Thomas)

 On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote:
 With recent arm broadcast time clean-up from Mark Rutland, the dummy
 broadcast device is always registered with timer subsystem. And since
 the rating of the dummy clock event is very high, it is preferred
 over a real broad-cast clock event.

 This is a change in behavior from past and not an intended
 one. So reduce the rating of the dummy clockevent so that
 real broadcast device is selected when available.

 Without this all the C states with C3STOP won't work since
 the broad cast notifier will take an abort.

 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Russell King li...@arm.linux.org.uk

 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 ---
 Its a regression so hopefully can get into the 3.9-rcx. Noticed
 this one on A15 platform. A9 platform the issue may not be seen
 since the local timer check avoids dummy timer registration.

 Some one pointed me to a fix made by Mark which was discussed
 under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject.
 That patch seems to be more of work around since the root of the
 problem is incorrect dummy timer rating. Either way, both patches
 fix the issue.
 
 Is the problem that the dummy timer is being registered as the broadcast
 source, or that it is selected as a local timer in preference of real timers?

Dummy timer is preferred over real broadcast timer.
 
 If it is the former, Then I believe my patch solve the issue more generally -
 if you happen to register a dummy timer before other timers, it will become 
 the
 broadcast source. Regardless of how temporary this is, it should never happen,
 and lowering the rating of the dummy won't fix this.
 
Well by the time we need active broadcast functionality, clock-events are
already chosen if the ratings are appropriate.

 If it is the latter, then this patch would ensure that a real timer with a
 rating over 100 is selected in preference to the dummy, which is certainly 
 what
 we want. The proposed generic dummy timer in Stephen Boyd's local timer API
 removal series [1] similarly uses a low rating to ensure that real timers are
 selected in preference to an always-registered dummy. I note that the
 architected timer has a higher rating (450) than the dummy (400), so this
 shouldn't currently be a problem.
 
Because we always register dummy broadcast on ARM now 
and with higher rating, it is picked as broadcast source. We definitely don't
want such a behavior when we have real broadcast device is available.


With your patch, we are trying to avoid the registration which goes
against the the whole idea of registering it always and picking
the right clock-event based on rating by clock-event core.
The clock-event core except the proper ratings to be provided based on
the capability of the timer source and its resolution and in that case
dummy should have the lowest rating which is what I tried to patch.

 Have I missed something?
 
Not much. I was just looking at x86 code as well after your email
to see how the LAPIC issue is handled. They seems to also have
correct rating to take care of the selection.

Probably we can merge both the fixes but from clock-event core
code perspective, the ratings fix is more than enough.

Thomas might be able to comment better.

Regards,
Santosh


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Mark Rutland
On Wed, Mar 13, 2013 at 11:24:01AM +, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote:
  Hi Santosh,
  
  On Wed, Mar 13, 2013 at 09:28:22AM +, Santosh Shilimkar wrote:
  (Forgot to CC Thomas)
 
  On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote:
  With recent arm broadcast time clean-up from Mark Rutland, the dummy
  broadcast device is always registered with timer subsystem. And since
  the rating of the dummy clock event is very high, it is preferred
  over a real broad-cast clock event.
 
  This is a change in behavior from past and not an intended
  one. So reduce the rating of the dummy clockevent so that
  real broadcast device is selected when available.
 
  Without this all the C states with C3STOP won't work since
  the broad cast notifier will take an abort.
 
  Cc: Mark Rutland mark.rutl...@arm.com
  Cc: Russell King li...@arm.linux.org.uk
 
  Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
  ---
  Its a regression so hopefully can get into the 3.9-rcx. Noticed
  this one on A15 platform. A9 platform the issue may not be seen
  since the local timer check avoids dummy timer registration.
 
  Some one pointed me to a fix made by Mark which was discussed
  under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject.
  That patch seems to be more of work around since the root of the
  problem is incorrect dummy timer rating. Either way, both patches
  fix the issue.
  
  Is the problem that the dummy timer is being registered as the broadcast
  source, or that it is selected as a local timer in preference of real 
  timers?
 
 Dummy timer is preferred over real broadcast timer.

Ok.

  
  If it is the former, Then I believe my patch solve the issue more generally 
  -
  if you happen to register a dummy timer before other timers, it will become 
  the
  broadcast source. Regardless of how temporary this is, it should never 
  happen,
  and lowering the rating of the dummy won't fix this.
  
 Well by the time we need active broadcast functionality, clock-events are
 already chosen if the ratings are appropriate.

That's a fair point. I still think it's worth having the check in the core
broadcast code - rejecting dummy timers is always a sensible thing to do, and
it prevents future crashes if new timers are added without sensible rating
values.

 
  If it is the latter, then this patch would ensure that a real timer with a
  rating over 100 is selected in preference to the dummy, which is certainly 
  what
  we want. The proposed generic dummy timer in Stephen Boyd's local timer API
  removal series [1] similarly uses a low rating to ensure that real timers 
  are
  selected in preference to an always-registered dummy. I note that the
  architected timer has a higher rating (450) than the dummy (400), so this
  shouldn't currently be a problem.
  
 Because we always register dummy broadcast on ARM now 
 and with higher rating, it is picked as broadcast source. We definitely don't
 want such a behavior when we have real broadcast device is available.

Agreed. We *never* want to pick a dummy timer as a broadcast source, as this
never makes sense.

 With your patch, we are trying to avoid the registration which goes
 against the the whole idea of registering it always and picking
 the right clock-event based on rating by clock-event core.
 The clock-event core except the proper ratings to be provided based on
 the capability of the timer source and its resolution and in that case
 dummy should have the lowest rating which is what I tried to patch.
 
  Have I missed something?
  
 Not much. I was just looking at x86 code as well after your email
 to see how the LAPIC issue is handled. They seems to also have
 correct rating to take care of the selection.
 
 Probably we can merge both the fixes but from clock-event core
 code perspective, the ratings fix is more than enough.

I do agree it'd be worth lowering the dummy timer's rating to ensure it doesn't
override a real timer elsewhere. 

Thomas has already pulled my fix into tip timers/urgent [1].

[1] 
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=timers/urgentid=a7dc19b8652c862d5b7c4d2339bd3c428bd29c4a

Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Thomas Gleixner
On Wed, 13 Mar 2013, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote:
  With recent arm broadcast time clean-up from Mark Rutland, the dummy
  broadcast device is always registered with timer subsystem. And since
  the rating of the dummy clock event is very high, it is preferred
  over a real broad-cast clock event.
  
  This is a change in behavior from past and not an intended
  one. So reduce the rating of the dummy clockevent so that
  real broadcast device is selected when available.
  
  Without this all the C states with C3STOP won't work since
  the broad cast notifier will take an abort.
  
  Cc: Mark Rutland mark.rutl...@arm.com
  Cc: Russell King li...@arm.linux.org.uk
  
  Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
  ---
  Its a regression so hopefully can get into the 3.9-rcx. Noticed
  this one on A15 platform. A9 platform the issue may not be seen
  since the local timer check avoids dummy timer registration.
  
 Some one pointed me to a fix made by Mark which was discussed
 under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject.
 That patch seems to be more of work around since the root of the
 problem is incorrect dummy timer rating. Either way, both patches
 fix the issue.

Well, using a dummy timer as the broadcast event device is a bug, no
matter what the rating is. The fix is in Linus tree already.

Though making the rating of the dummy lower is definitely a good
thing, so a real hardware device which is detected later can replace
the dummy device. So yes, the rating should be low for the dummy
timer.

Thanks,

tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Santosh Shilimkar
On Wednesday 13 March 2013 07:49 PM, Thomas Gleixner wrote:
 On Wed, 13 Mar 2013, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 02:36 PM, Santosh Shilimkar wrote:
 With recent arm broadcast time clean-up from Mark Rutland, the dummy
 broadcast device is always registered with timer subsystem. And since
 the rating of the dummy clock event is very high, it is preferred
 over a real broad-cast clock event.

 This is a change in behavior from past and not an intended
 one. So reduce the rating of the dummy clockevent so that
 real broadcast device is selected when available.

 Without this all the C states with C3STOP won't work since
 the broad cast notifier will take an abort.

 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Russell King li...@arm.linux.org.uk

 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 ---
 Its a regression so hopefully can get into the 3.9-rcx. Noticed
 this one on A15 platform. A9 platform the issue may not be seen
 since the local timer check avoids dummy timer registration.

 Some one pointed me to a fix made by Mark which was discussed
 under '[BUG] ARM Architected timers appear broken in 3.9-rc1' subject.
 That patch seems to be more of work around since the root of the
 problem is incorrect dummy timer rating. Either way, both patches
 fix the issue.
 
 Well, using a dummy timer as the broadcast event device is a bug, no
 matter what the rating is. The fix is in Linus tree already.

Agree.
 
 Though making the rating of the dummy lower is definitely a good
 thing, so a real hardware device which is detected later can replace
 the dummy device. So yes, the rating should be low for the dummy
 timer.
 
Exactly. As Mark pointed out, you have already applied Mark's patch.
I was just wondering whether you can take the $subject patch as well
with ack from Russell, Mark.

Regards,
Santosh

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Santosh Shilimkar
On Wednesday 13 March 2013 05:55 PM, Mark Rutland wrote:
 On Wed, Mar 13, 2013 at 11:24:01AM +, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote:
 Hi Santosh,

[..]


 Is the problem that the dummy timer is being registered as the broadcast
 source, or that it is selected as a local timer in preference of real 
 timers?

 Dummy timer is preferred over real broadcast timer.
 
 Ok.
 
  
 If it is the former, Then I believe my patch solve the issue more generally 
 -
 if you happen to register a dummy timer before other timers, it will become 
 the
 broadcast source. Regardless of how temporary this is, it should never 
 happen,
 and lowering the rating of the dummy won't fix this.

 Well by the time we need active broadcast functionality, clock-events are
 already chosen if the ratings are appropriate.
 
 That's a fair point. I still think it's worth having the check in the core
 broadcast code - rejecting dummy timers is always a sensible thing to do, and
 it prevents future crashes if new timers are added without sensible rating
 values.
 

 If it is the latter, then this patch would ensure that a real timer with a
 rating over 100 is selected in preference to the dummy, which is certainly 
 what
 we want. The proposed generic dummy timer in Stephen Boyd's local timer API
 removal series [1] similarly uses a low rating to ensure that real timers 
 are
 selected in preference to an always-registered dummy. I note that the
 architected timer has a higher rating (450) than the dummy (400), so this
 shouldn't currently be a problem.

 Because we always register dummy broadcast on ARM now 
 and with higher rating, it is picked as broadcast source. We definitely don't
 want such a behavior when we have real broadcast device is available.
 
 Agreed. We *never* want to pick a dummy timer as a broadcast source, as this
 never makes sense.
 
 With your patch, we are trying to avoid the registration which goes
 against the the whole idea of registering it always and picking
 the right clock-event based on rating by clock-event core.
 The clock-event core except the proper ratings to be provided based on
 the capability of the timer source and its resolution and in that case
 dummy should have the lowest rating which is what I tried to patch.

 Have I missed something?

 Not much. I was just looking at x86 code as well after your email
 to see how the LAPIC issue is handled. They seems to also have
 correct rating to take care of the selection.

 Probably we can merge both the fixes but from clock-event core
 code perspective, the ratings fix is more than enough.
 
 I do agree it'd be worth lowering the dummy timer's rating to ensure it 
 doesn't
 override a real timer elsewhere. 
 
Yep. Can I add you acked-by tag then for $subject patch ?
Would be good to get this one merged as well.

Regards
Santosh

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Mark Rutland
On Wed, Mar 13, 2013 at 03:44:03PM +, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 05:55 PM, Mark Rutland wrote:
  On Wed, Mar 13, 2013 at 11:24:01AM +, Santosh Shilimkar wrote:
  On Wednesday 13 March 2013 03:46 PM, Mark Rutland wrote:
  Hi Santosh,
 
 [..]
 
 
  Is the problem that the dummy timer is being registered as the broadcast
  source, or that it is selected as a local timer in preference of real 
  timers?
 
  Dummy timer is preferred over real broadcast timer.
  
  Ok.

[...]

  I do agree it'd be worth lowering the dummy timer's rating to ensure it 
  doesn't
  override a real timer elsewhere. 
  
 Yep. Can I add you acked-by tag then for $subject patch ?
 Would be good to get this one merged as well.

Sure. Though could you reword the commit message? The patch solves the more
general issue of a dummy being preferred over real hardware even outside of
choosing the broadcast device.

Acked-by: Mark Rutland mark.rutl...@arm.com
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: smp: Allow real broadcast device selection instead of always dummy

2013-03-13 Thread Thomas Gleixner
On Wed, 13 Mar 2013, Santosh Shilimkar wrote:
 On Wednesday 13 March 2013 07:49 PM, Thomas Gleixner wrote:
  Though making the rating of the dummy lower is definitely a good
  thing, so a real hardware device which is detected later can replace
  the dummy device. So yes, the rating should be low for the dummy
  timer.
  
 Exactly. As Mark pointed out, you have already applied Mark's patch.
 I was just wondering whether you can take the $subject patch as well
 with ack from Russell, Mark.

It can go through the ARM tree as well, no dependencies on my stuff.

Thanks,

tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html