Re: [PATCH] Inhibit NMI watchdog when Alt-SysRq-T operation is underway.

From: Konrad Rzeszutek
Date: Fri Jul 20 2007 - 07:18:32 EST


Hey Andrew,

I tested your patch along with mine and found two things out:

1). Missing this patch (for i386 platform)

diff --git a/arch/i386/kernel/traps.c b/arch/i386/kernel/traps.c
index 90da057..9f3a7ff 100644
--- a/arch/i386/kernel/traps.c
+++ b/arch/i386/kernel/traps.c
@@ -207,6 +207,7 @@ static void print_trace_address(void *da
{
printk("%s [<%08lx>] ", (char *)data, addr);
print_symbol("%s\n", addr);
+ touch_nmi_watchdog();
}

static struct stacktrace_ops print_trace_ops = {


2). If I run Alt-SysRq-t about 5000 times in a loop, the slow down
with this change is about 5%. Is this a big issue? (This was
testing both i686 and x86_64).

On Fri, Jul 13, 2007 at 04:45:04PM -0700, Andrew Morton wrote:
> On Mon, 9 Jul 2007 11:53:02 -0400
> darnok@xxxxxxx wrote:
>
> > static void print_trace_address(void *data, unsigned long addr)
> > {
> > + static int i = 0;
> > + if (i && ((i % 8) == 0))
> > + touch_nmi_watchdog();
> > + i++;
> > printk_address(addr);
> > }
>
> I doubt if the "% 8" thing is really needed? printk_address() is pretty
> slow and touch_nmi_watchdog is _reasonably_ fast. It could be made heaps
> faster by:
>
> From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>
> Avoid dirtying remote cpu's memory if it already has the correct value.
>
> Cc: Andi Kleen <ak@xxxxxxx>
> Cc: Konrad Rzeszutek <konrad@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> arch/i386/kernel/nmi.c | 8 +++++---
> x86_64/kernel/nmi.c | 0
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff -puN arch/i386/kernel/nmi.c~i386-speedup-touch_nmi_watchdog arch/i386/kernel/nmi.c
> --- a/arch/i386/kernel/nmi.c~i386-speedup-touch_nmi_watchdog
> +++ a/arch/i386/kernel/nmi.c
> @@ -298,7 +298,7 @@ static unsigned int
> last_irq_sums [NR_CPUS],
> alert_counter [NR_CPUS];
>
> -void touch_nmi_watchdog (void)
> +void touch_nmi_watchdog(void)
> {
> if (nmi_watchdog > 0) {
> unsigned cpu;
> @@ -307,8 +307,10 @@ void touch_nmi_watchdog (void)
> * Just reset the alert counters, (other CPUs might be
> * spinning on locks we hold):
> */
> - for_each_present_cpu (cpu)
> - alert_counter[cpu] = 0;
> + for_each_present_cpu(cpu) {
> + if (alert_counter[cpu])
> + alert_counter[cpu] = 0;
> + }
> }
>
> /*
>
> So I'd be inclined to simplify your patch to a bare
>
> From: Konrad Rzeszutek <konrad@xxxxxxxxxx>
>
> On large memory configuration with not so fast CPUs the NMI watchdog is
> triggered when memory addresses are being gathered and printed. The code
> paths for Alt-SysRq-t are sprinkled with touch_nmi_watchdog in various
> places but not in this routine (or in the loop that utilizes this
> function). The patch has been tested for regression on large CPU+memory
> configuration (128 logical CPUs + 224 GB) and 1,2,4,16-CPU sockets with
> various memory sizes (1,2,4,6,20).
>
> Cc: Andi Kleen <ak@xxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> arch/x86_64/kernel/traps.c | 1 +
> 1 files changed, 1 insertion(+)
>
> diff -puN arch/x86_64/kernel/traps.c~inhibit-nmi-watchdog-when-alt-sysrq-t-operation-is-underway arch/x86_64/kernel/traps.c
> --- a/arch/x86_64/kernel/traps.c~inhibit-nmi-watchdog-when-alt-sysrq-t-operation-is-underway
> +++ a/arch/x86_64/kernel/traps.c
> @@ -397,6 +397,7 @@ static int print_trace_stack(void *data,
>
> static void print_trace_address(void *data, unsigned long addr)
> {
> + touch_nmi_watchdog();
> printk_address(addr);
> }
>
> _
>
>
> OK?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/