Re: [V4][PATCH 4/6] x86, nmi: add in logic to handle multiple events and unknown NMIs

[Robert Richter <robert.richter@amd.com>]

On 13.09.11 16:58:27, Don Zickus wrote:
> @@ -87,6 +87,16 @@ static int notrace __kprobes nmi_handle(unsigned int type, struct pt_regs *regs)
>  
>  		handled += a->handler(type, regs);
>  
> +		/*
> + 		 * Optimization: only loop once if this is not a 
> + 		 * back-to-back NMI.  The idea is nothing is dropped
> + 		 * on the first NMI, only on the second of a back-to-back
> + 		 * NMI.  No need to waste cycles going through all the
> + 		 * handlers.
> + 		 */
> +		if (!b2b && handled)
> +			break;

Don, if I am not missing something, this actually does not work
because perfctr NMIs do not re-trigger. Suppose a handler running
before perfctr. It sets 'handled' and the chain is stopped here. To
run through the perfctr handler the NMI must retrigger which it
doesn't.

I tested the above with enclosed patch.

The patch handles the nmi and then stops the chain. You see by the PMI
count that the perfctr nmi is not handled:

Patch applied:

 # echo $(($(grep PMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 0
 # echo $(($(grep NMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 0
 # perf record -e cpu-cycles bash -c 'perl -e "while(1) {}" & sleep 5 ; kill $!'
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.011 MB perf.data (~472 samples) ]
 # echo $(($(grep PMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 128
 # echo $(($(grep NMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 1387

W/o the patch (tip/perf/core: 51887c8):

 # echo $(($(grep NMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 0
 # echo $(($(grep PMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 0
 # perf record -e cpu-cycles bash -c 'perl -e "while(1) {}" & sleep 5 ; kill $!'
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.194 MB perf.data (~8455 samples) ]
 # echo $(($(grep PMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 4918
 # echo $(($(grep NMI /proc/interrupts | sed -e 's/.*: *//;s/ *Non.*//;s/  */ + /g')))
 4918

So we may not jump out the while loop.

-Robert

> +
>  		a = next_a;
>  	}
>  	rcu_read_unlock();



>From b1d68bf037cfa78f073cf71c296057ff422294f2 Mon Sep 17 00:00:00 2001
From: Robert Richter <robert.richter@amd.com>
Date: Wed, 14 Sep 2011 11:44:49 +0200
Subject: [PATCH] perf_nmi_test

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/kernel/cpu/perf_event.c |   45 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 594d425..2255221 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -31,6 +31,7 @@
 #include <asm/compat.h>
 #include <asm/smp.h>
 #include <asm/alternative.h>
+#include <asm/hardirq.h>
 
 #if 0
 #undef wrmsrl
@@ -43,6 +44,8 @@ do {								\
 } while (0)
 #endif
 
+#define irq_stats(x)              (&per_cpu(irq_stat, x))
+
 /*
  *          |   NHM/WSM    |      SNB     |
  * register -------------------------------
@@ -1383,6 +1386,7 @@ perf_event_nmi_handler(struct notifier_block *self,
 	struct die_args *args = __args;
 	unsigned int this_nmi;
 	int handled;
+	int cpu;
 
 	if (!atomic_read(&active_events))
 		return NOTIFY_DONE;
@@ -1408,6 +1412,9 @@ perf_event_nmi_handler(struct notifier_block *self,
 	}
 
 	handled = x86_pmu.handle_irq(args->regs);
+	cpu = smp_processor_id();
+	trace_printk("perf: NMI: %d, PMI: %d, handled: %d\n", irq_stats(cpu)->__nmi_count,
+		     irq_stats(cpu)->apic_perf_irqs, handled);
 	if (!handled)
 		return NOTIFY_DONE;
 
@@ -1961,3 +1968,41 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
 
 	return misc;
 }
+
+static DEFINE_PER_CPU(unsigned long, save_rip);
+
+static int __kprobes perf_test_nmi_handler(struct notifier_block *self,
+					   unsigned long cmd, void *__args)
+{
+	struct die_args *args = __args;
+	bool b2b = false;
+	int cpu = smp_processor_id();
+
+	if (cmd != DIE_NMI)
+		return NOTIFY_DONE;
+
+	if (args->regs->ip == __this_cpu_read(save_rip))
+		b2b = true;
+
+	__this_cpu_write(save_rip, args->regs->ip);
+
+	trace_printk("skip: NMI: %d, PMI: %d, b2b: %d\n", irq_stats(cpu)->__nmi_count,
+		     irq_stats(cpu)->apic_perf_irqs, b2b);
+
+	if (!b2b)
+		return NOTIFY_STOP;
+
+	return NOTIFY_DONE;
+}
+
+static __read_mostly struct notifier_block perf_test_nmi_notifier = {
+	.notifier_call		= perf_test_nmi_handler,
+	.priority		= NMI_LOCAL_HIGH_PRIOR,
+};
+
+static __init int perf_nmi_test_init(void)
+{
+	return register_die_notifier(&perf_test_nmi_notifier);
+}
+
+device_initcall(perf_nmi_test_init);
-- 
1.7.6.1



-- 
Advanced Micro Devices, Inc.
Operating System Research Center