Currently, arch_trigger_all_cpu_backtrace() is defined in only x86 and sparc which has nmi interrupt.
But in case of softlockup not a hardlockup, it could be possible to dump backtrace of all cpus. and this could be helpful for debugging.
for example, if system has 2 cpus.
CPU 0 CPU 1
acquire read_lock()
try to do write_lock()
,,,
missing read_unlock()
In this case, softlockup will occur becasuse CPU 0 does not call read_unlock().
And dump_stack() print only backtrace for "CPU 0". If CPU1's backtrace is printed it's very helpful.
This patch adds arch_trigger_all_cpu_backtrace() for mips architecture.
Maybe someone make better patch than this. I just suggest the idea.
---
arch/mips/include/asm/irq.h | 3 +++
arch/mips/kernel/process.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
index 39f07ae..5a4e1bb 100644
--- a/arch/mips/include/asm/irq.h
+++ b/arch/mips/include/asm/irq.h
@@ -48,4 +48,7 @@ extern int cp0_compare_irq;
extern int cp0_compare_irq_shift;
extern int cp0_perfcount_irq;
+void arch_trigger_all_cpu_backtrace(bool);
+#define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
+
#endif /* _ASM_IRQ_H */
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 636b074..9f51d3d 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -42,6 +42,7 @@
#include <asm/isadep.h>
#include <asm/inst.h>
#include <asm/stacktrace.h>
+#include <asm/irq_regs.h>
#ifdef CONFIG_HOTPLUG_CPU
void arch_cpu_idle_dead(void)
@@ -532,3 +533,20 @@ unsigned long arch_align_stack(unsigned long sp)
return sp & ALMASK;
}
+
+static void arch_dump_stack(void *info)
+{
+ struct pt_regs *regs;
+
+ regs = get_irq_regs();
+
+ if(regs)
+ show_regs(regs);
+
+ dump_stack();
+}
+
+void arch_trigger_all_cpu_backtrace(bool include_self)
+{
+ smp_call_function(arch_dump_stack, NULL, 1);
+}
--
1.7.0.1
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Hi Eubong,
one small question inline ...
On 22/10/2014 08:39, Eunbong Song wrote:
>
> Currently, arch_trigger_all_cpu_backtrace() is defined in only x86
> and sparc which has nmi interrupt. But in case of softlockup not a
> hardlockup, it could be possible to dump backtrace of all cpus. and
> this could be helpful for debugging.
>
> for example, if system has 2 cpus.
>
> CPU 0 CPU 1 acquire read_lock()
>
> try to do write_lock()
>
> ,,, missing read_unlock()
>
> In this case, softlockup will occur becasuse CPU 0 does not call
> read_unlock(). And dump_stack() print only backtrace for "CPU 0".
> If CPU1's backtrace is printed it's very helpful.
>
> This patch adds arch_trigger_all_cpu_backtrace() for mips
> architecture.
>
> Maybe someone make better patch than this. I just suggest the
> idea. --- arch/mips/include/asm/irq.h | 3 +++
> arch/mips/kernel/process.c | 18 ++++++++++++++++++ 2 files
> changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/arch/mips/include/asm/irq.h
> b/arch/mips/include/asm/irq.h index 39f07ae..5a4e1bb 100644 ---
> a/arch/mips/include/asm/irq.h +++ b/arch/mips/include/asm/irq.h @@
> -48,4 +48,7 @@ extern int cp0_compare_irq; extern int
> cp0_compare_irq_shift; extern int cp0_perfcount_irq;
>
> +void arch_trigger_all_cpu_backtrace(bool); +#define
> arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
What is the purpose of this define ? is this maybe a leftover from
some regex/cleanups ?
John
> + #endif /* _ASM_IRQ_H */ diff --git a/arch/mips/kernel/process.c
> b/arch/mips/kernel/process.c index 636b074..9f51d3d 100644 ---
> a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@
> -42,6 +42,7 @@ #include <asm/isadep.h> #include <asm/inst.h>
> #include <asm/stacktrace.h> +#include <asm/irq_regs.h>
>
> #ifdef CONFIG_HOTPLUG_CPU void arch_cpu_idle_dead(void) @@ -532,3
> +533,20 @@ unsigned long arch_align_stack(unsigned long sp)
>
> return sp & ALMASK; } + +static void arch_dump_stack(void *info)
> +{ + struct pt_regs *regs; + + regs = get_irq_regs(); + + if(regs)
> + show_regs(regs); + + dump_stack(); +} + +void
> arch_trigger_all_cpu_backtrace(bool include_self) +{ +
> smp_call_function(arch_dump_stack, NULL, 1); +}
>
> Hi Eubong,
> one small question inline ...
>> +void arch_trigger_all_cpu_backtrace(bool); +#define
>> arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
> What is the purpose of this define ? is this maybe a leftover from
> some regex/cleanups ?
Hi John.
Actually, I just follow the same function of sparc architecture.
You can find this in arch/sparc/include/asm/irq_64.h as below
void arch_trigger_all_cpu_backtrace(bool);
#define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
I guess this is used for conditional compile.
See below.
include/linux/nmi.h
#ifdef arch_trigger_all_cpu_backtrace
static inline bool trigger_all_cpu_backtrace(void)
{
arch_trigger_all_cpu_backtrace(true);
return true;
}
static inline bool trigger_allbutself_cpu_backtrace(void)
{
arch_trigger_all_cpu_backtrace(false);
return true;
}
#else
static inline bool trigger_all_cpu_backtrace(void)
{
return false;
}
static inline bool trigger_allbutself_cpu_backtrace(void)
{
return false;
}
#endif
Thanks.
> John
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 22/10/2014 08:54, Eunbong Song wrote:
>
>> Hi Eubong,
>
>> one small question inline ...
>
>>> +void arch_trigger_all_cpu_backtrace(bool); +#define
>>> arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
>
>> What is the purpose of this define ? is this maybe a leftover from
>> some regex/cleanups ?
>
> Hi John.
> Actually, I just follow the same function of sparc architecture.
> You can find this in arch/sparc/include/asm/irq_64.h as below
>
> void arch_trigger_all_cpu_backtrace(bool);
> #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
>
> I guess this is used for conditional compile.
> See below.
> include/linux/nmi.h
> #ifdef arch_trigger_all_cpu_backtrace
> static inline bool trigger_all_cpu_backtrace(void)
> {
> arch_trigger_all_cpu_backtrace(true);
>
> return true;
> }
> static inline bool trigger_allbutself_cpu_backtrace(void)
> {
> arch_trigger_all_cpu_backtrace(false);
> return true;
> }
> #else
> static inline bool trigger_all_cpu_backtrace(void)
> {
> return false;
> }
> static inline bool trigger_allbutself_cpu_backtrace(void)
> {
> return false;
> }
> #endif
>
> Thanks.
>> John
>
i don't see how this is required for conditional compiles. the code
define a->a which is bogus i think.
John
On Wed, Oct 22, 2014 at 06:39:56AM +0000, Eunbong Song wrote:
Applying - but:
> + if(regs)
^^^
There should be a blank between if and opening parenthesis.
Ralf
On Wed, Oct 22, 2014 at 06:39:56AM +0000, Eunbong Song wrote:
> This patch adds arch_trigger_all_cpu_backtrace() for mips architecture.
Don't forget your Signed-off-by
> +static void arch_dump_stack(void *info)
> +{
> + struct pt_regs *regs;
> +
> + regs = get_irq_regs();
> +
> + if(regs)
> + show_regs(regs);
> +
> + dump_stack();
> +}
> +
> +void arch_trigger_all_cpu_backtrace(bool include_self)
> +{
> + smp_call_function(arch_dump_stack, NULL, 1);
should this call arch_dump_stack directly too if include_self?
Cheers
James
Hi John,
On Wed, Oct 22, 2014 at 09:11:39AM +0200, John Crispin wrote:
> On 22/10/2014 08:54, Eunbong Song wrote:
> >>> +void arch_trigger_all_cpu_backtrace(bool); +#define
> >>> arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
> >
> >> What is the purpose of this define ? is this maybe a leftover from
> >> some regex/cleanups ?
> >
> > Hi John.
> > Actually, I just follow the same function of sparc architecture.
> > You can find this in arch/sparc/include/asm/irq_64.h as below
> >
> > void arch_trigger_all_cpu_backtrace(bool);
> > #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
> >
> > I guess this is used for conditional compile.
> > See below.
> > include/linux/nmi.h
> > #ifdef arch_trigger_all_cpu_backtrace
> > static inline bool trigger_all_cpu_backtrace(void)
> > {
> > arch_trigger_all_cpu_backtrace(true);
> >
> > return true;
> > }
> > static inline bool trigger_allbutself_cpu_backtrace(void)
> > {
> > arch_trigger_all_cpu_backtrace(false);
> > return true;
> > }
> > #else
> > static inline bool trigger_all_cpu_backtrace(void)
> > {
> > return false;
> > }
> > static inline bool trigger_allbutself_cpu_backtrace(void)
> > {
> > return false;
> > }
> > #endif
> >
> > Thanks.
> >> John
> >
>
> i don't see how this is required for conditional compiles. the code
> define a->a which is bogus i think.
It's a pretty common pattern in the asm headers, in order to allow
generic code to use the preprocessor to see whether it was defined or
not.
Cheers
James
>> This patch adds arch_trigger_all_cpu_backtrace() for mips architecture.
> Don't forget your Signed-off-by
I'm sorry fot this.
>> +static void arch_dump_stack(void *info)
>> +{
>> + struct pt_regs *regs;
>> +
>> + regs = get_irq_regs();
>> +
>> + if(regs)
>> + show_regs(regs);
>> +
>> + dump_stack();
>> +}
>> +
>> +void arch_trigger_all_cpu_backtrace(bool include_self)
>> +{
>> + smp_call_function(arch_dump_stack, NULL, 1);
> should this call arch_dump_stack directly too if include_self?
Currently, in case of mips there is no case include_self is true, so this is not a problem.
arch_trigger_all_cpu_backtrace can only be called from trigger_allbutself_cpu_backtrace() in kernel/watchdog.c.
But as you said, if the case will be added, we should consider that.
Thanks.
> Cheers
> James????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?