Skip to contentSkip to conversation

SMP local timers and their CPUfreq notifiers setup on OMAP3

DISCUSSION Kevin Hilman 4 participants 12 messages Feb 16, 2012 – Feb 21, 2012 Arm (32-bit)

lore
Share & tools
lore

Kevin Hilman

author
#0 lore raw

Hi Santosh,

Using v3.3-rc3 and building an OMAP2+ kernel I noticed that CPUfreq transitions fault because of the TWD cpufreq notifiers being called even on OMAP3.

Of course, the TWD doesn't exist on OMAP3, but the TWD init is still setting up notifiers. I tried just adding a !cpu_is_omap44xx() check in local_timer_setup(), but that didn't do it, so there's more timer init that needs to be fixed up. Can you have a look at this?

It's easy to reproduce by just manually triggering a frequency change with the userspace governor. Here's what I did on my 3430/n900:

# cd /sys/devices/system/cpu/cpu0/cpufreq
# cat scaling_available_frequencies
# echo 250000 > scaling_setspeed           
[   13.785797] Unable to handle kernel paging request at virtual address 007e900
[   13.793426] pgd = cdc20000                                                   
[   13.796295] [007e9000] *pgd=00000000                                         
[   13.800079] Internal error: Oops: 5 [#1] SMP                                 
[   13.804595] Modules linked in:                                               
[   13.807830] CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)         
[   13.814544] PC is at twd_update_frequency+0x34/0x48                          
[   13.819671] LR is at twd_update_frequency+0x10/0x48                          
[   13.824829] pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093          
[   13.824829] sp : ce311dd8  ip : 00000000  fp : 00000000                      
[   13.836914] r10: 00000000  r9 : 00000001  r8 : ce310000                      
[   13.842437] r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74       
[   13.849304] r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000       
[   13.856201] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
[   13.863800] Control: 10c5387d  Table: 8dc20019  DAC: 00000015                
[   13.869842] Process sh (pid: 599, stack limit = 0xce3102f8)                  
[   13.875732] Stack: (0xce311dd8 to 0xce312000)                                
[   13.880310] 1dc0:                                                       6000c
[   13.888946] 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
[   13.897583] 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
[   13.906188] 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
[   13.914825] 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
[   13.923431] 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
[   13.932067] 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
[   13.940673] 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
[   13.949310] 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
[   13.957946] 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
[   13.966552] 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
[   13.975189] 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
[   13.983795] 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
[   13.992431] 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
[   14.001037] 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
[   14.009674] 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
[   14.018280] 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
[   14.026916] 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
[   14.035552] [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
[   14.046478] [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
[   14.057586] [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
[   14.068054] [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
[   14.078887] [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
[   14.090179] [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
[   14.101470] [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
[   14.111755] [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
[   14.121765] [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
[   14.131561] [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
[   14.141296] [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
[   14.150482] [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
[   14.159118] [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
[   14.168212] [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
[   14.176666] [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
[   14.185577] Code: e594300c e792210c e1a01000 e5840004 (e7930002)             
[   14.192169] ---[ end trace 5da3b5167c1ecdda ]---
#1 lore raw

(+ linux-arm, Marc)

Quoted 6 lines · Kevin Hilman

Hi Santosh,

Using v3.3-rc3 and building an OMAP2+ kernel I noticed that CPUfreq transitions fault because of the TWD cpufreq notifiers being called even on OMAP3.

Quoted 8 lines

Of course, the TWD doesn't exist on OMAP3, but the TWD init is still setting up notifiers. I tried just adding a !cpu_is_omap44xx() check in local_timer_setup(), but that didn't do it, so there's more timer init that needs to be fixed up. Can you have a look at this?

It's easy to reproduce by just manually triggering a frequency change with the userspace governor. Here's what I did on my 3430/n900:

Below patch fixes the issue.

Regards
Santosh

Quoted 1 line

From 3a16f7a6694c14e201fdf6ad195c821816b2de84 Mon Sep 17 00:00:00 2001

From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG.

Fix it by uisng an initialised variable. Though the twd_timer_setup() is percpu, the idea here is to avoid the CPUFREQ registration. Hence percpu initialised variable is not used.

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arm/kernel/smp_twd.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +2 -1
index 4285daa..753ae37 100644
@@ -30,6 +30,7 @@ void __iomem *twd_base;
 
 static struct clk *twd_clk;
 static unsigned long twd_timer_rate;
+static bool twd_initialised;
 
 static struct clock_event_device __percpu **twd_evt;
 

@@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!IS_ERR(twd_clk)) && twd_initialised)
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);
 

@@ -267,4 +268,5 @@ void __cpuinit twd_timer_setup(struct

clock_event_device *clk)
clockevents_config_and_register(clk, twd_timer_rate,
0xf, 0xffffffff);
enable_percpu_irq(clk->irq, 0);
+ twd_initialised = 1;
}

Signature · 9 lines
1.7.4.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-smp_twd-Don-t-register-CPUFREQ-notifiers-if-loca.patch
Type: text/x-patch
Size: 5006 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120217/9c7cf781/attachment-0001.bin>
#2 lore raw

Hi Santosh,

Quoted 94 lines · Santosh Shilimkar

From 3a16f7a6694c14e201fdf6ad195c821816b2de84 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG.

Fix it by uisng an initialised variable. Though the twd_timer_setup() is percpu, the idea here is to avoid the CPUFREQ registration. Hence percpu initialised variable is not used.

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arm/kernel/smp_twd.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +1 -0
index 4285daa..753ae37 100644
@@ -30,6 +30,7 @@ void __iomem *twd_base;
 
 static struct clk *twd_clk;
 static unsigned long twd_timer_rate;
+static bool twd_initialised;
 
 static struct clock_event_device __percpu **twd_evt;

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 135

Quoted 7 lines
arch/arm/kernel/smp_twd.c +1 -1
@@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!IS_ERR(twd_clk)) && twd_initialised)
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);

Testing twd_evt and *__this_cpu_ptr(twd_evt) should have a similar effect, and save the additional variable.

        M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
#3 lore raw
Quoted 6 lines · Marc Zyngier

Hi Santosh,

Quoted 1 line · Santosh Shilimkar

[...]

Quoted 17 lines
Quoted 17 lines

?arch/arm/kernel/smp_twd.c | ? ?4 +++-
?1 files changed, 3 insertions(+), 1 deletions(-)

arch/arm/kernel/smp_twd.c +1 -0
index 4285daa..753ae37 100644
@@ -30,6 +30,7 @@ void __iomem *twd_base;
 
 ?static struct clk *twd_clk;
 ?static unsigned long twd_timer_rate;
+static bool twd_initialised;
 
 ?static struct clock_event_device __percpu **twd_evt;
 

@@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 10 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 135

Quoted 6 lines
arch/arm/kernel/smp_twd.c +1 -1
 ?static int twd_cpufreq_init(void)
 ?{
- ? ? if (!IS_ERR(twd_clk))
+ ? ? if ((!IS_ERR(twd_clk)) && twd_initialised)
 ? ? ? ? ? ? ? return cpufreq_register_notifier(&twd_cpufreq_nb,
 ? ? ? ? ? ? ? ? ? ? ? CPUFREQ_TRANSITION_NOTIFIER);

Testing twd_evt and *__this_cpu_ptr(twd_evt) should have a similar effect, and save the additional variable.

Indeed.
Updated patch below.

Quoted 1 line

From 4d351f59e5dabe065d51304455ee01b191cb56d6 Mon Sep 17 00:00:00 2001

From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arm/kernel/smp_twd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +1 -1
index 4285daa..265a18a 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);
Signature · 8 lines
1.7.4.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-smp_twd-Don-t-register-CPUFREQ-notifiers-if-loca.patch
Type: application/octet-stream
Size: 4482 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120217/1f86683b/attachment-0001.obj>
#4 lore raw

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 132

Quoted 2 lines · Santosh
arch/arm/kernel/smp_twd.c +1 -1
-	if (!IS_ERR(twd_clk))
+	if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Too many parens.

#5 lore raw
In reply to Shilimkar, Santosh
Quoted 121 lines · Santosh
Quoted 6 lines · Marc Zyngier

Hi Santosh,

Quoted 1 line · Santosh Shilimkar

[...]

Quoted 17 lines
Quoted 17 lines
 arch/arm/kernel/smp_twd.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +1 -0
index 4285daa..753ae37 100644
@@ -30,6 +30,7 @@ void __iomem *twd_base;
 
 static struct clk *twd_clk;
 static unsigned long twd_timer_rate;
+static bool twd_initialised;
 
 static struct clock_event_device __percpu **twd_evt;
 

@@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 10 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 134

Quoted 6 lines
arch/arm/kernel/smp_twd.c +1 -1
 static int twd_cpufreq_init(void)
 {
-     if (!IS_ERR(twd_clk))
+     if ((!IS_ERR(twd_clk)) && twd_initialised)
              return cpufreq_register_notifier(&twd_cpufreq_nb,
                      CPUFREQ_TRANSITION_NOTIFIER);

Testing twd_evt and *__this_cpu_ptr(twd_evt) should have a similar effect, and save the additional variable.

Indeed.
Updated patch below.

From 4d351f59e5dabe065d51304455ee01b191cb56d6 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arm/kernel/smp_twd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +0 -0
index 4285daa..265a18a 100644

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 132

Quoted 5 lines
arch/arm/kernel/smp_twd.c +1 -1
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Erm... Shouldn't that rather be:

	if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 135

Quoted 3 lines
arch/arm/kernel/smp_twd.c +0 -0
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);
 

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#6 lore raw
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Santosh
Quoted 6 lines · Marc Zyngier

Hi Santosh,

Quoted 1 line · Santosh Shilimkar

[..]

Quoted 6 lines
Quoted 6 lines
arch/arm/kernel/smp_twd.c +0 -0
index 4285daa..265a18a 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 8 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 132

Quoted 4 lines
arch/arm/kernel/smp_twd.c +1 -1
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Erm... Shouldn't that rather be:

	if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

My bad. You are right. Updated patch below.
Tested on OMAP3430 SDP with CPUFREQ enabled.

Regards
Santosh

Quoted 1 line

From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001

From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
---
 arch/arm/kernel/smp_twd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +1 -1
index 4285daa..9834851 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	 if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);
Signature · 11 lines
1.7.4.1



-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-smp_twd-Don-t-register-CPUFREQ-notifiers-if-loca.patch
Type: text/x-patch
Size: 4478 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120217/e144ee6b/attachment.bin>
Acked-by
#7 lore raw
Quoted 103 lines · Santosh Shilimkar
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Santosh
Quoted 6 lines · Marc Zyngier

Hi Santosh,

Quoted 1 line · Santosh Shilimkar

[..]

Quoted 6 lines
Quoted 6 lines
arch/arm/kernel/smp_twd.c +0 -0
index 4285daa..265a18a 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 8 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = {

Quoted 4 lines
arch/arm/kernel/smp_twd.c +1 -1
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Erm... Shouldn't that rather be:

	if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

My bad. You are right. Updated patch below.
Tested on OMAP3430 SDP with CPUFREQ enabled.

Regards
Santosh

From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Quoted 2 lines
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
Acked-by Marc Zyngier <[email protected]>
Quoted 17 lines
---
 arch/arm/kernel/smp_twd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/kernel/smp_twd.c +1 -1
index 4285daa..9834851 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	 if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
 		return cpufreq_register_notifier(&twd_cpufreq_nb,
 			CPUFREQ_TRANSITION_NOTIFIER);
Signature · 1 line
Jazz is not dead. It just smells funny...
author Tested-by
#8 lore raw
In reply to Santosh Shilimkar
Quoted 103 lines · Santosh Shilimkar
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Santosh
Quoted 6 lines · Marc Zyngier

Hi Santosh,

Quoted 1 line · Santosh Shilimkar

[..]

Quoted 6 lines
Quoted 6 lines
arch/arm/kernel/smp_twd.c +0 -0
index 4285daa..265a18a 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 8 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = {

Quoted 4 lines
arch/arm/kernel/smp_twd.c +1 -1
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Erm... Shouldn't that rather be:

	if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

My bad. You are right. Updated patch below.
Tested on OMAP3430 SDP with CPUFREQ enabled.

Regards
Santosh

From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc20000
 [007e9000] *pgd=00000000
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : [<c001382c>]    lr : [<c0013808>]    psr: 60000093
 sp : ce311dd8  ip : 00000000  fp : 00000000
 r10: 00000000  r9 : 00000001  r8 : ce310000
 r7 : c0440458  r6 : c00137f8  r5 : 00000000  r4 : c0947a74
 r3 : 00000000  r2 : 007e9000  r1 : 00000000  r0 : 00000000
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 00000015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:                                                       6000c
 1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000
 1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4
 1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330
 1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034
 1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358
 1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c
 1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004
 1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000
 1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202
 1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000
 [<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] )
 [<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013)
 [<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>)
 [<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] ()
 [<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f)
 [<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2)
 [<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333)
 [<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf)
 [<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24)
 [<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc)
 [<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>)
 [<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi)
 [<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs)
 [<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write)
 [<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---
Quoted 2 lines
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
Tested-by Kevin Hilman <[email protected]>

CAn you please add to Russell's patch system. This needs to be fixed for v3.3-rc.

Thanks,

Kevin

#9 lore raw
Quoted 105 lines · Kevin Hilman
Quoted 103 lines · Santosh Shilimkar
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Santosh
Quoted 6 lines · Marc Zyngier

Hi Santosh,

Quoted 1 line · Santosh Shilimkar

[..]

Quoted 6 lines
Quoted 6 lines
arch/arm/kernel/smp_twd.c +0 -0
index 4285daa..265a18a 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 8 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 132

Quoted 4 lines
arch/arm/kernel/smp_twd.c +1 -1
 ?static int twd_cpufreq_init(void)
 ?{
- ? if (!IS_ERR(twd_clk))
+ ? if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Erm... Shouldn't that rather be:
? ? ?if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

My bad. You are right. Updated patch below.
Tested on OMAP3430 SDP with CPUFREQ enabled.

Regards
Santosh

From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case the twd_timer_setup() isn't called. That seems to be wrong and would eventually lead to kernel crash on the CPU frequency transitions on the SOCs where the local timer doesn't exist or broken because of hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

?Unable to handle kernel paging request at virtual address 007e900 ?pgd = cdc20000
?[007e9000] *pgd=00000000
?Internal error: Oops: 5 [#1] SMP
?Modules linked in:
?CPU: 0 ? ?Not tainted ?(3.3.0-rc3-pm+debug+initramfs #9) ?PC is at twd_update_frequency+0x34/0x48
?LR is at twd_update_frequency+0x10/0x48
?pc : [<c001382c>] ? ?lr : [<c0013808>] ? ?psr: 60000093 ?sp : ce311dd8 ?ip : 00000000 ?fp : 00000000
?r10: 00000000 ?r9 : 00000001 ?r8 : ce310000
?r7 : c0440458 ?r6 : c00137f8 ?r5 : 00000000 ?r4 : c0947a74 ?r3 : 00000000 ?r2 : 007e9000 ?r1 : 00000000 ?r0 : 00000000 ?Flags: nZCv ?IRQs off ?FIQs on ?Mode SVC_32 ?ISA ARM ?Segment usr ?Control: 10c5387d ?Table: 8dc20019 ?DAC: 00000015 ?Process sh (pid: 599, stack limit = 0xce3102f8) ?Stack: (0xce311dd8 to 0xce312000)
?1dc0: ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 6000c ?1de0: 00000001 00000002 00000000 00000000 00000000 00000000 00000 ?1e00: ffffffff c093d8f0 00000000 ce311ebc 00000001 00000001 ce310 ?1e20: c001386c c0437c4c c0e95b60 c0e95ba8 00000001 c0e95bf8 ffff4 ?1e40: 00000000 00000000 c005ef74 ce310000 c0435cf0 ce311ebc 00000 ?1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 00000 ?1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 ffff8 ?1ea0: c08ba040 c033364c ce311ecc c0433b50 00000002 ffffffea c0330 ?1ec0: 0007a120 0007a120 22222201 00000000 22222222 00000000 ce357 ?1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 00000002 c032f47c 00034 ?1f00: c0331cac ce352b40 00000007 c032f6d0 ce352bbc 0003d090 c0930 ?1f20: c093d8bc c03306a4 00000007 ce311f80 00000007 cdc2aec0 ce358 ?1f40: ce8d20c0 00000007 b6fe5000 ce311f80 00000007 ce310000 0000c ?1f60: c000de74 ce987400 ce8d20c0 b6fe5000 00000000 00000000 0000c ?1f80: 00000000 00000000 001fbac8 00000000 00000007 001fbac8 00004 ?1fa0: c000df04 c000dd60 00000007 001fbac8 00000001 b6fe5000 00000 ?1fc0: 00000007 001fbac8 00000007 00000004 b6fe5000 00000000 00202 ?1fe0: 00000000 beb565f8 00101ffc 00008e8c 60000010 00000001 00000 ?[<c001382c>] (twd_update_frequency+0x34/0x48) from [<c008ac4c>] ) ?[<c008ac4c>] (smp_call_function_single+0x17c/0x1c8) from [<c0013) ?[<c0013890>] (twd_cpufreq_transition+0x24/0x30) from [<c0437c4c>) ?[<c0437c4c>] (notifier_call_chain+0x44/0x84) from [<c005efe4>] () ?[<c005efe4>] (__srcu_notifier_call_chain+0x70/0xa4) from [<c005f) ?[<c005f030>] (srcu_notifier_call_chain+0x18/0x20) from [<c032fe2) ?[<c032fe2c>] (cpufreq_notify_transition+0xc8/0x1b0) from [<c0333) ?[<c033364c>] (omap_target+0x1b4/0x28c) from [<c032f47c>] (__cpuf) ?[<c032f47c>] (__cpufreq_driver_target+0x50/0x64) from [<c0331d24) ?[<c0331d24>] (cpufreq_set+0x78/0x98) from [<c032f6d0>] (store_sc) ?[<c032f6d0>] (store_scaling_setspeed+0x5c/0x74) from [<c03306a4>) ?[<c03306a4>] (store+0x58/0x74) from [<c014d868>] (sysfs_write_fi) ?[<c014d868>] (sysfs_write_file+0x80/0xb4) from [<c00f2c2c>] (vfs) ?[<c00f2c2c>] (vfs_write+0xa8/0x138) from [<c00f2e9c>] (sys_write) ?[<c00f2e9c>] (sys_write+0x40/0x6c) from [<c000dd60>] (ret_fast_s) ?Code: e594300c e792210c e1a01000 e5840004 (e7930002) ?---[ end trace 5da3b5167c1ecdda ]---

Quoted 8 lines
Quoted 2 lines
Reported-by Kevin Hilman <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>
Tested-by Kevin Hilman <[email protected]>

CAn you please add to Russell's patch system. ?This needs to be fixed for v3.3-rc.

Done .
7328/1 in patch system.

Regards
Santosh

#10 lore raw
In reply to Santosh Shilimkar

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 132

Quoted 10 lines · Santosh Shilimkar
arch/arm/kernel/smp_twd.c +1 -1
index 4285daa..9834851 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
 
 static int twd_cpufreq_init(void)
 {
-	if (!IS_ERR(twd_clk))
+	 if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

You seem to have an additional space before 'if'.

#11 lore raw
Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Santosh Shilimkar
arch/arm/kernel/smp_twd.c +0 -0
index 4285daa..9834851 100644
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
Quoted 6 lines

arch/arm/kernel/smp_twd.c static struct notifier_block twd_cpufreq_nb = { · line 132

Quoted 4 lines
arch/arm/kernel/smp_twd.c +1 -1
 ?static int twd_cpufreq_init(void)
 ?{
- ? ? if (!IS_ERR(twd_clk))
+ ? ? ?if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

You seem to have an additional space before 'if'.

Yep.
Fixed in the updated version in patch system.
patch 7336/1

Regards
Santosh