2008-06-25 07:13:38

by Vegard Nossum

[permalink] [raw]
Subject: [PATCH] softlockup: show irqtrace

Hi,

Ingo, can you please drop the "softirq softlockup debugging" patch and
apply this instead? This much, much prettier, reuses existing debugging
infrastructure, and is a lot more generic.

(Compile tested on allyesconfig, allnoconfig, allnoconfig + softlockup
debugging.)


Vegard


From: Vegard Nossum <[email protected]>
Date: Wed, 25 Jun 2008 08:50:10 +0200
Subject: [PATCH] softlockup: show irqtrace

This patch adds some information about when interrupts were last
enabled and disabled to the output of the softlockup detector.

Signed-off-by: Vegard Nossum <[email protected]>
---
kernel/softlockup.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/softlockup.c b/kernel/softlockup.c
index 01b6522..980f703 100644
--- a/kernel/softlockup.c
+++ b/kernel/softlockup.c
@@ -13,6 +13,7 @@
#include <linux/delay.h>
#include <linux/freezer.h>
#include <linux/kthread.h>
+#include <linux/lockdep.h>
#include <linux/notifier.h>
#include <linux/module.h>

@@ -115,6 +116,7 @@ void softlockup_tick(void)
printk(KERN_ERR "BUG: soft lockup - CPU#%d stuck for %lus! [%s:%d]\n",
this_cpu, now - touch_timestamp,
current->comm, task_pid_nr(current));
+ print_irqtrace_events(current);
if (regs)
show_regs(regs);
else
--
1.5.4.1


2008-06-25 16:00:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] softlockup: show irqtrace


* Vegard Nossum <[email protected]> wrote:

> Hi,
>
> Ingo, can you please drop the "softirq softlockup debugging" patch and
> apply this instead? This much, much prettier, reuses existing
> debugging infrastructure, and is a lot more generic.

applied to tip/core/softlockup, thanks Vegard.

> (Compile tested on allyesconfig, allnoconfig, allnoconfig + softlockup
> debugging.)

hm, that must have taken quite some time to do on your desktop, right?

FYI, there's no mandatory need to go through that buerocratic testing
hassle with patches submitted to -tip - especially with small patches
that look obvious and have a high chance of being right. We do non-stop
allyes/allno/allmod+randconfig 64-bit/32-bit build and boot testing for
every single patch added to the repository.

Building, booting and testing it in the environment where you expect it
to be used primarily should be more than enough.

Ingo

2008-06-25 17:11:24

by Vegard Nossum

[permalink] [raw]
Subject: Re: [PATCH] softlockup: show irqtrace

On 6/25/08, Ingo Molnar <[email protected]> wrote:
> > (Compile tested on allyesconfig, allnoconfig, allnoconfig + softlockup
> > debugging.)
>
> hm, that must have taken quite some time to do on your desktop, right?

Hm, no. I admit to having improved the process of compilation
somewhat; I compiled only the affected files, or in case a CONFIG
prevents a file from being built, the complete parent directory. I
find that this works well in practice; the compiler usually warns
about implicit declarations (which would otherwise show up as linker
errors), so there is no need to link everything. (Sadly, this is not
true when declarations are provided regardless of definition -- I
think this is actually a case against putting declarations outside
#ifdefs.)

> FYI, there's no mandatory need to go through that buerocratic testing
> hassle with patches submitted to -tip - especially with small patches
> that look obvious and have a high chance of being right. We do non-stop
> allyes/allno/allmod+randconfig 64-bit/32-bit build and boot testing for
> every single patch added to the repository.
>
> Building, booting and testing it in the environment where you expect it
> to be used primarily should be more than enough.

Well, it's still somewhat emberassing to submit patches that don't compile :-)

This patch was also a typical error scenario; I introduce the call of
a function which requires the inclusion of a new header, one which
seemingly depends on a particular config option (lockdep). So I simply
wanted to be sure myself -- and then I might as well put it in the
e-mail. That also implicitly means that I did not boot test it at all.
(Because, yeah, what it actually DOES should be fairly obvious.)

Thanks, though! It's really nice to know the world's not coming to an
end because of a broken patch ;-)


Vegard

--
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
-- E. W. Dijkstra, EWD1036

2008-06-25 21:15:56

by Johannes Weiner

[permalink] [raw]
Subject: Re: [PATCH] softlockup: show irqtrace

Hi,

Vegard Nossum <[email protected]> writes:

> Hi,
>
> Ingo, can you please drop the "softirq softlockup debugging" patch and
> apply this instead? This much, much prettier, reuses existing debugging
> infrastructure, and is a lot more generic.
>
> (Compile tested on allyesconfig, allnoconfig, allnoconfig + softlockup
> debugging.)
>
>
> Vegard
>
>
> From: Vegard Nossum <[email protected]>
> Date: Wed, 25 Jun 2008 08:50:10 +0200
> Subject: [PATCH] softlockup: show irqtrace
>
> This patch adds some information about when interrupts were last
> enabled and disabled to the output of the softlockup detector.

Nice idea!

> Signed-off-by: Vegard Nossum <[email protected]>

Acked-by: Johannes Weiner <[email protected]>