lkml.org 
[lkml]   [2011]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [V5][PATCH 2/6] x86, nmi: create new NMI handler routines
On 09/20/2011 10:43 PM, Don Zickus wrote:
> The NMI handlers used to rely on the notifier infrastructure. This worked
> great until we wanted to support handling multiple events better.
>
> One of the key ideas to the nmi handling is to process _all_ the handlers for
> each NMI. The reason behind this switch is because NMIs are edge triggered.
> If enough NMIs are triggered, then they could be lost because the cpu can
> only latch at most one NMI (besides the one currently being processed).
>
> In order to deal with this we have decided to process all the NMI handlers
> for each NMI. This allows the handlers to determine if they recieved an
> event or not (the ones that can not determine this will be left to fend
> for themselves on the unknown NMI list).
>
> As a result of this change it is now possible to have an extra NMI that
> was destined to be received for an already processed event. Because the
> event was processed in the previous NMI, this NMI gets dropped and becomes
> an 'unknown' NMI. This of course will cause printks that scare people.
>
> However, we prefer to have extra NMIs as opposed to losing NMIs and as such
> are have developed a basic mechanism to catch most of them. That will be
> a later patch.
>
> To accomplish this idea, I unhooked the nmi handlers from the notifier
> routines and created a new mechanism loosely based on doIRQ. The reason
> for this is the notifier routines have a couple of shortcomings. One we
> could't guarantee all future NMI handlers used NOTIFY_OK instead of
> NOTIFY_STOP. Second, we couldn't keep track of the number of events being
> handled in each routine (most only handle one, perf can handle more than one).
> Third, I wanted to eventually display which nmi handlers are registered in
> the system in /proc/interrupts to help see who is generating NMIs.
>
> The patch below just implements the new infrastructure but doesn't wire it up
> yet (that is the next patch). Its design is based on doIRQ structs and the
> atomic notifier routines. So the rcu stuff in the patch isn't entirely untested
> (as the notifier routines have soaked it) but it should be double checked in
> case I copied the code wrong.
>
> V2:
> - use kstrdup to copy/allocate device name
> - fix-up _GPL oops
>
> V3:
> - fix leak in register_nmi_handler error path
> - removed _raw annotations from rcu_dereference
>
> V4:
> - handle kstrndup failure
>
> Signed-off-by: Don Zickus <dzickus@redhat.com>
> ---
> arch/x86/include/asm/nmi.h | 19 +++++
> arch/x86/kernel/nmi.c | 157 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 176 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h
> index 4886a68..6d04b28 100644
> --- a/arch/x86/include/asm/nmi.h
> +++ b/arch/x86/include/asm/nmi.h
> @@ -42,6 +42,25 @@ void arch_trigger_all_cpu_backtrace(void);
> #define NMI_LOCAL_NORMAL_PRIOR (NMI_LOCAL_BIT | NMI_NORMAL_PRIOR)
> #define NMI_LOCAL_LOW_PRIOR (NMI_LOCAL_BIT | NMI_LOW_PRIOR)
>
> +#define NMI_FLAG_FIRST 1
> +
> +enum {
> + NMI_LOCAL=0,
> + NMI_UNKNOWN,
> + NMI_EXTERNAL,
> + NMI_MAX
> +};
> +
> +#define NMI_DONE 0
> +#define NMI_HANDLED 1
> +
> +typedef int (*nmi_handler_t)(unsigned int, struct pt_regs *);
> +
> +int register_nmi_handler(unsigned int, nmi_handler_t, unsigned long,
> + const char *);
> +
> +void unregister_nmi_handler(unsigned int, const char *);
> +
> void stop_nmi(void);
> void restart_nmi(void);
>
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 68d758a..c2df58a 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -13,6 +13,9 @@
> #include <linux/kprobes.h>
> #include <linux/kdebug.h>
> #include <linux/nmi.h>
> +#include <linux/delay.h>
> +#include <linux/hardirq.h>
> +#include <linux/slab.h>
>
> #if defined(CONFIG_EDAC)
> #include <linux/edac.h>
> @@ -21,6 +24,28 @@
> #include <linux/atomic.h>
> #include <asm/traps.h>
> #include <asm/mach_traps.h>
> +#include <asm/nmi.h>
> +
> +#define NMI_MAX_NAMELEN 16
> +struct nmiaction {
> + struct nmiaction __rcu *next;

Why not just use struct list_head here and use list_xxx_rcu family to
operate on the list? IMHO, that will make code simpler without much
overhead.

Best Regards,
Huang Ying


\
 
 \ /
  Last update: 2011-09-21 07:39    [from the cache]
©2003-2011 Jasper Spaans