Skip to contentSkip to conversation

parisc: avoid WARNING: at kernel/cpu/idle.c:96

PATCH In review Helge Deller 3 participants 5 messages May 8, 2013 – May 18, 2013 PA-RISC architecture

lore
Share & tools
lore

Helge Deller

author maintainer
#0 lore raw

PATCH 1/1 parisc: avoid WARNING: at kernel/cpu/idle.c:96

We need to enable local irqs to avoid this runtime warning.

Signed-off-by Helge Deller <[email protected]>
arch/parisc/kernel/process.c +5 -0
index 55f92b6..2840d43 100644
@@ -286,3 +286,8 @@ void *dereference_function_descriptor(void *ptr)
 	return ptr;
 }
 #endif
+
+void arch_cpu_idle(void)
+{
+	local_irq_enable();
+}
maintainer
#1 lore raw
Quoted 9 lines · Helge Deller

We need to enable local irqs to avoid this runtime warning.

Signed-off-by Helge Deller <[email protected]>
arch/parisc/kernel/process.c +0 -0
index 55f92b6..2840d43 100644

arch/parisc/kernel/process.c void *dereference_function_descriptor(void *ptr) · line 292

Quoted 7 lines
arch/parisc/kernel/process.c +4 -0
@@ -286,3 +286,8 @@ void *dereference_function_descriptor(void *ptr)
 	return ptr;
 }
 #endif
+
+void arch_cpu_idle(void)
+{
+	local_irq_enable();

This is definitely wrong. we'd need at least a cpu_relax() in there. Without that the cpu will bounce around in the idle loop. I'd advise that we simply keep the default cpu_idle_poll().

However, this looks like a kernel bug to me in cpu_idle_loop()? not something we should be working around in the arch code.

Thomas, what's going on here? It looks like you can never avoid the WARN_ON_ONCE if you don't provide any arch specific idle functions. If you wish to allow this case then the correct patch (keeping the WARN_ON) would appear to be this one.

James

----

kernel/cpu/idle.c +1 -0
index 8b86c0c..829e41f 100644
@@ -56,6 +56,7 @@ void __weak arch_cpu_idle_dead(void) { }
 void __weak arch_cpu_idle(void)
 {
 	cpu_idle_force_poll = 1;
+	local_irq_enable();
 }
 
 /*
maintainer
#2 lore raw
Quoted 22 lines · James Bottomley
Quoted 9 lines · Helge Deller

We need to enable local irqs to avoid this runtime warning.

Signed-off-by Helge Deller <[email protected]>
arch/parisc/kernel/process.c +0 -0
index 55f92b6..2840d43 100644

arch/parisc/kernel/process.c void *dereference_function_descriptor(void *ptr) · line 292

Quoted 7 lines
arch/parisc/kernel/process.c +4 -0
@@ -286,3 +286,8 @@ void *dereference_function_descriptor(void *ptr)
 	return ptr;
 }
 #endif
+
+void arch_cpu_idle(void)
+{
+	local_irq_enable();

This is definitely wrong. we'd need at least a cpu_relax() in there. Without that the cpu will bounce around in the idle loop. I'd advise that we simply keep the default cpu_idle_poll().

Quoted 7 lines

However, this looks like a kernel bug to me in cpu_idle_loop()? not something we should be working around in the arch code.

Thomas, what's going on here? It looks like you can never avoid the WARN_ON_ONCE if you don't provide any arch specific idle functions. If you wish to allow this case then the correct patch (keeping the WARN_ON) would appear to be this one.

Ping on this, please, Thomas ... the patch looks fairly obviously correct, do you just want us to take it through the parisc tree?

James

Quoted 21 lines

James

----

kernel/cpu/idle.c +1 -1
index 8b86c0c..829e41f 100644
@@ -56,6 +56,7 @@ void __weak arch_cpu_idle_dead(void) { }
 void __weak arch_cpu_idle(void)
 {
 	cpu_idle_force_poll = 1;
+	local_irq_enable();
 }
 
 /*
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Reviewed-by
#3 lore raw
In reply to James Bottomley
Quoted 39 lines · James Bottomley
Quoted 9 lines · Helge Deller

We need to enable local irqs to avoid this runtime warning.

Signed-off-by Helge Deller <[email protected]>
arch/parisc/kernel/process.c +0 -0
index 55f92b6..2840d43 100644

arch/parisc/kernel/process.c void *dereference_function_descriptor(void *ptr) · line 292

Quoted 7 lines
arch/parisc/kernel/process.c +4 -0
@@ -286,3 +286,8 @@ void *dereference_function_descriptor(void *ptr)
 	return ptr;
 }
 #endif
+
+void arch_cpu_idle(void)
+{
+	local_irq_enable();

This is definitely wrong. we'd need at least a cpu_relax() in there. Without that the cpu will bounce around in the idle loop. I'd advise that we simply keep the default cpu_idle_poll().

However, this looks like a kernel bug to me in cpu_idle_loop()? not something we should be working around in the arch code.

Thomas, what's going on here? It looks like you can never avoid the WARN_ON_ONCE if you don't provide any arch specific idle functions. If you wish to allow this case then the correct patch (keeping the WARN_ON) would appear to be this one.

James

----

kernel/cpu/idle.c +0 -0
index 8b86c0c..829e41f 100644

kernel/cpu/idle.c void __weak arch_cpu_idle_dead(void) { } · line 62

Quoted 7 lines
kernel/cpu/idle.c +1 -0
@@ -56,6 +56,7 @@ void __weak arch_cpu_idle_dead(void) { }
 void __weak arch_cpu_idle(void)
 {
 	cpu_idle_force_poll = 1;
+	local_irq_enable();
 }
 
 /*

This patch looks like the correct fix to me.

Reviewed-by Srivatsa S. Bhat <[email protected]>

Regards,
Srivatsa S. Bhat

author maintainer
#4 lore raw
Quoted 45 lines · Srivatsa S. Bhat
Quoted 44 lines · James Bottomley
Quoted 9 lines · Helge Deller

We need to enable local irqs to avoid this runtime warning.

Signed-off-by Helge Deller <[email protected]>
arch/parisc/kernel/process.c +0 -0
index 55f92b6..2840d43 100644

arch/parisc/kernel/process.c void *dereference_function_descriptor(void *ptr) · line 292

Quoted 7 lines
arch/parisc/kernel/process.c +4 -0
@@ -286,3 +286,8 @@ void *dereference_function_descriptor(void *ptr)
 	return ptr;
 }
 #endif
+
+void arch_cpu_idle(void)
+{
+	local_irq_enable();

This is definitely wrong. we'd need at least a cpu_relax() in there. Without that the cpu will bounce around in the idle loop. I'd advise that we simply keep the default cpu_idle_poll().

However, this looks like a kernel bug to me in cpu_idle_loop()? not something we should be working around in the arch code.

Thomas, what's going on here? It looks like you can never avoid the WARN_ON_ONCE if you don't provide any arch specific idle functions. If you wish to allow this case then the correct patch (keeping the WARN_ON) would appear to be this one.

James

----

kernel/cpu/idle.c +1 -0
index 8b86c0c..829e41f 100644
@@ -56,6 +56,7 @@ void __weak arch_cpu_idle_dead(void) { }
 void __weak arch_cpu_idle(void)
 {
 	cpu_idle_force_poll = 1;
+	local_irq_enable();
 }
Quoted 6 lines

kernel/cpu/idle.c void __weak arch_cpu_idle_dead(void) { } · line 62

Quoted 2 lines
kernel/cpu/idle.c +0 -0
 /*

This patch looks like the correct fix to me.

Reviewed-by Srivatsa S. Bhat <[email protected]>

Thanks Srivatsa!

I can push it through the parisc tree if nobody objects...

Helge