Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2] x86,nmi: Fix page faults by nmiaction if kmemcheck is enabled | From | Li Zhong <> | Date | Fri, 09 Mar 2012 05:12:18 -0500 |
| |
On Thu, 2012-03-08 at 16:02 -0500, Don Zickus wrote: > From: Li Zhong <zhong@linux.vnet.ibm.com> > > This patch tries to fix the problem of page fault exception caused by > accessing nmiaction structure in nmi if kmemcheck is enabled. > > If kmemcheck is enabled, the memory allocated through slab are in pages > that are marked non-present, so that some checks could be done in the > page fault handling code ( e.g. whether the memory is read before > written to ). > As nmiaction is allocated in this way, so it resides in a non-present > page. Then there is a page fault while the nmi code accessing the > nmiaction structure, which would then cause a warning by > WARN_ON_ONCE(in_nmi()) in kmemcheck_fault(), called by do_page_fault(). > > v2: as Peter suggested, changed the nmiaction to use static storage. > > v3: as Peter suggested, use macro to shorten the codes. Also keep the > original usage of register_nmi_handler, so users of this call doesn't > need change. > > [simplified wrappers -dcz] > > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com> > Signed-off-by: Don Zickus <dzickus@redhat.com> > --- > arch/x86/include/asm/nmi.h | 20 ++++++++++++- > arch/x86/kernel/nmi.c | 65 ++++--------------------------------------- > 2 files changed, 24 insertions(+), 61 deletions(-) > > diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h > index fd3f9f1..2a17e57 100644 > --- a/arch/x86/include/asm/nmi.h > +++ b/arch/x86/include/asm/nmi.h > @@ -35,8 +35,24 @@ enum { > > typedef int (*nmi_handler_t)(unsigned int, struct pt_regs *); > > -int register_nmi_handler(unsigned int, nmi_handler_t, unsigned long, > - const char *); > +struct nmiaction { > + struct list_head list; > + nmi_handler_t handler; > + unsigned int flags; > + const char *name; > +}; > + > +#define register_nmi_handler(t, fn, fg, n) \ > +({ \ > + static struct nmiaction fn##_na = { \ > + .handler = (fn), \ > + .name = (n), \ > + .flags = (fg), \ > + }; \ > + __register_nmi_handler((t), &fn##_na); \ > +})
Thank you, Don.
As flags is moved into the macro, it may cause following compile error, drivers/watchdog/hpwdt.c: In function 'hpwdt_init_nmi_decoding': drivers/watchdog/hpwdt.c:737: error: initializer element is not constant drivers/watchdog/hpwdt.c:737: error: (near initialization for 'hpwdt_pretimeout_na.flags')
So following fix might be needed:
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c index 3c166d3..e1161ea 100644 --- a/drivers/watchdog/hpwdt.c +++ b/drivers/watchdog/hpwdt.c @@ -734,9 +734,12 @@ static int __devinit hpwdt_init_nmi_decoding(struct pci_dev *dev) * die notify list to handle a critical NMI. The default is to * be last so other users of the NMI signal can function. */ - retval = register_nmi_handler(NMI_UNKNOWN, hpwdt_pretimeout, - (priority) ? NMI_FLAG_FIRST : 0, - "hpwdt"); + if (priority) + retval = register_nmi_handler(NMI_UNKNOWN, hpwdt_pretimeout, + NMI_FLAG_FIRST, "hpwdt"); + else + retval = register_nmi_handler(NMI_UNKNOWN, hpwdt_pretimeout, + 0, "hpwdt"); if (retval != 0) { dev_warn(&dev->dev, "Unable to register a die notifier (err=%d).\n",
| |