* [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault @ 2013-10-15 20:39 Steven Rostedt 2013-10-16 6:11 ` Ingo Molnar 2013-10-16 11:40 ` Frederic Weisbecker 0 siblings, 2 replies; 18+ messages in thread From: Steven Rostedt @ 2013-10-15 20:39 UTC (permalink / raw) To: LKML Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng Since the NMI iretq nesting has been fixed, there's no reason that an NMI handler can not take a page fault for vmalloc'd code. No locks are taken in that code path, and the software now handles nested NMIs when the fault re-enables NMIs on iretq. Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that warn on triggers a vmalloc fault for some reason, then we can go into an infinite loop (the WARN_ON_ONCE() does the WARN() before updating the variable to make it happen "once"). Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- arch/x86/mm/fault.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 3aaeffc..78926c6 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -268,8 +268,6 @@ static noinline __kprobes int vmalloc_fault(unsigned long address) if (!(address >= VMALLOC_START && address < VMALLOC_END)) return -1; - WARN_ON_ONCE(in_nmi()); - /* * Synchronize this task's top level page-table * with the 'reference' page table. -- 1.8.1.4 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-15 20:39 [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault Steven Rostedt @ 2013-10-16 6:11 ` Ingo Molnar 2013-10-16 12:42 ` Steven Rostedt 2013-10-16 11:40 ` Frederic Weisbecker 1 sibling, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2013-10-16 6:11 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng * Steven Rostedt <rostedt@goodmis.org> wrote: > Since the NMI iretq nesting has been fixed, there's no reason that > an NMI handler can not take a page fault for vmalloc'd code. No locks > are taken in that code path, and the software now handles nested NMIs > when the fault re-enables NMIs on iretq. > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > warn on triggers a vmalloc fault for some reason, then we can go into > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > the variable to make it happen "once"). > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> Would be nice to see the warning quoted that triggered this. Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 6:11 ` Ingo Molnar @ 2013-10-16 12:42 ` Steven Rostedt 2013-10-16 12:51 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2013-10-16 12:42 UTC (permalink / raw) To: Ingo Molnar Cc: LKML, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, 16 Oct 2013 08:11:18 +0200 Ingo Molnar <mingo@kernel.org> wrote: > > * Steven Rostedt <rostedt@goodmis.org> wrote: > > > Since the NMI iretq nesting has been fixed, there's no reason that > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > are taken in that code path, and the software now handles nested NMIs > > when the fault re-enables NMIs on iretq. > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > warn on triggers a vmalloc fault for some reason, then we can go into > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > the variable to make it happen "once"). > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > Would be nice to see the warning quoted that triggered this. Sure, want me to add this to the change log? =============== [ 15.069144] BUG: unable to handle kernel [ 15.073635] paging request at 1649736d [ 15.076379] IP: [<c200402a>] print_context_stack+0x4a/0xa0 [ 15.082529] *pde = 00000000 [ 15.085758] Thread overran stack, or stack corrupted [ 15.091303] Oops: 0000 [#1] SMP [ 15.094932] Modules linked in: atomisp_css2400b0_v2(+) lm3554 ov2722 imx1x5 atmel_mxt_ts vxd392 videobuf_vmalloc videobuf_core bcm_bt_lpm bcm43241 kct_daemon(O) [ 15.111093] CPU: 2 PID: 2443 Comm: Compiler Tainted: G W O 3.10.1+ #1 [ 15.119075] task: f213f980 ti: f0c42000 task.ti: f0c42000 [ 15.125116] EIP: 0060:[<c200402a>] EFLAGS: 00210087 CPU: 2 [ 15.131255] EIP is at print_context_stack+0x4a/0xa0 [ 15.136712] EAX: 16497ffc EBX: 1649736d ECX: 986736d8 EDX: 1649736d [ 15.143722] ESI: 00000000 EDI: ffffe000 EBP: f0c4220c ESP: f0c421ec [ 15.150732] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068 [ 15.156771] CR0: 80050033 CR2: 1649736d CR3: 31245000 CR4: 001007d0 [ 15.163781] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 [ 15.170789] DR6: ffff0ff0 DR7: 00000400 [ 15.175076] Stack: [ 15.177324] 16497ffc 16496000 986736d8 ffffe000 986736d8 1649736d c282c148 16496000 [ 15.186067] f0c4223c c20033b0 c282c148 c29ceecf 00000000 f0c4222c 986736d8 f0c4222c [ 15.194810] 00000000 c29ceecf 00000000 00000000 f0c42260 c20041a7 f0c4229c c282c148 [ 15.203549] Call Trace: [ 15.206295] [<c20033b0>] dump_trace+0x70/0xf0 [ 15.211274] [<c20041a7>] show_trace_log_lvl+0x47/0x60 [ 15.217028] [<c2003482>] show_stack_log_lvl+0x52/0xd0 [ 15.222782] [<c2004201>] show_stack+0x21/0x50 [ 15.227762] [<c281b38b>] dump_stack+0x16/0x18 [ 15.232742] [<c2037cff>] warn_slowpath_common+0x5f/0x80 [ 15.238693] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 15.244156] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 15.249621] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 15.255472] [<c2037d3d>] warn_slowpath_null+0x1d/0x20 [ 15.261228] [<c282553a>] vmalloc_fault+0x5a/0xcf [ 15.266497] [<c282592f>] __do_page_fault+0x2cf/0x4a0 [ 15.272154] [<c25e13e0>] ? logger_aio_write+0x230/0x230 [ 15.278106] [<c2039c94>] ? console_unlock+0x314/0x440 ... // [ 16.885364] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 16.891217] [<c2825b08>] do_page_fault+0x8/0x10 [ 16.896387] [<c2823066>] error_code+0x5a/0x60 [ 16.901367] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 16.907219] [<c208d6a0>] ? print_modules+0x20/0x90 [ 16.912685] [<c2037cfa>] warn_slowpath_common+0x5a/0x80 [ 16.918634] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 16.924097] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 16.929562] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 16.935415] [<c2037d3d>] warn_slowpath_null+0x1d/0x20 [ 16.941169] [<c282553a>] vmalloc_fault+0x5a/0xcf [ 16.946437] [<c282592f>] __do_page_fault+0x2cf/0x4a0 [ 16.952095] [<c25e13e0>] ? logger_aio_write+0x230/0x230 [ 16.958046] [<c2039c94>] ? console_unlock+0x314/0x440 [ 16.963800] [<c2003e62>] ? sys_modify_ldt+0x2/0x160 [ 16.969362] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 16.975215] [<c2825b08>] do_page_fault+0x8/0x10 [ 16.980386] [<c2823066>] error_code+0x5a/0x60 [ 16.985366] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 16.991215] [<c208d6a0>] ? print_modules+0x20/0x90 [ 16.996673] [<c2037cfa>] warn_slowpath_common+0x5a/0x80 [ 17.002622] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 17.008086] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 17.013550] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.019403] [<c2037d3d>] warn_slowpath_null+0x1d/0x20 [ 17.025159] [<c282553a>] vmalloc_fault+0x5a/0xcf [ 17.030428] [<c282592f>] __do_page_fault+0x2cf/0x4a0 [ 17.036085] [<c25e13e0>] ? logger_aio_write+0x230/0x230 [ 17.042037] [<c2039c94>] ? console_unlock+0x314/0x440 [ 17.047790] [<c2003e62>] ? sys_modify_ldt+0x2/0x160 [ 17.053352] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.059205] [<c2825b08>] do_page_fault+0x8/0x10 [ 17.064375] [<c2823066>] error_code+0x5a/0x60 [ 17.069354] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.075204] [<c208d6a0>] ? print_modules+0x20/0x90 [ 17.080669] [<c2037cfa>] warn_slowpath_common+0x5a/0x80 [ 17.086619] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 17.092082] [<c282553a>] ? vmalloc_fault+0x5a/0xcf [ 17.097546] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.103399] [<c2037d3d>] warn_slowpath_null+0x1d/0x20 [ 17.109154] [<c282553a>] vmalloc_fault+0x5a/0xcf [ 17.114422] [<c282592f>] __do_page_fault+0x2cf/0x4a0 [ 17.120080] [<c206b93d>] ? update_group_power+0x1fd/0x240 [ 17.126224] [<c227827b>] ? number.isra.2+0x32b/0x330 [ 17.131880] [<c20679bc>] ? update_curr+0xac/0x190 [ 17.137247] [<c227827b>] ? number.isra.2+0x32b/0x330 [ 17.142905] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.148755] [<c2825b08>] do_page_fault+0x8/0x10 [ 17.153926] [<c2823066>] error_code+0x5a/0x60 [ 17.158905] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.164760] [<c208d1a9>] ? module_address_lookup+0x29/0xb0 [ 17.170999] [<c208dddb>] kallsyms_lookup+0x9b/0xb0 [ 17.176462] [<c208de1d>] __sprint_symbol+0x2d/0xd0 [ 17.181926] [<c22790cc>] ? sprintf+0x1c/0x20 [ 17.186804] [<c208def4>] sprint_symbol+0x14/0x20 [ 17.192063] [<c208df1e>] __print_symbol+0x1e/0x40 [ 17.197430] [<c25e00d7>] ? ashmem_shrink+0x77/0xf0 [ 17.202895] [<c25e13e0>] ? logger_aio_write+0x230/0x230 [ 17.208845] [<c205bdf5>] ? up+0x25/0x40 [ 17.213242] [<c2039cb7>] ? console_unlock+0x337/0x440 [ 17.218998] [<c2818236>] ? printk+0x38/0x3a [ 17.223782] [<c20006d0>] __show_regs+0x70/0x190 [ 17.228954] [<c200353a>] show_regs+0x3a/0x1b0 [ 17.233931] [<c2818236>] ? printk+0x38/0x3a [ 17.238717] [<c2824182>] arch_trigger_all_cpu_backtrace_handler+0x62/0x80 [ 17.246413] [<c2823919>] nmi_handle.isra.0+0x39/0x60 [ 17.252071] [<c2823a29>] do_nmi+0xe9/0x3f0 -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 12:42 ` Steven Rostedt @ 2013-10-16 12:51 ` Ingo Molnar 2013-10-16 13:01 ` Borislav Petkov ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Ingo Molnar @ 2013-10-16 12:51 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng * Steven Rostedt <rostedt@goodmis.org> wrote: > On Wed, 16 Oct 2013 08:11:18 +0200 > Ingo Molnar <mingo@kernel.org> wrote: > > > > > * Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > Since the NMI iretq nesting has been fixed, there's no reason that > > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > > are taken in that code path, and the software now handles nested NMIs > > > when the fault re-enables NMIs on iretq. > > > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > > warn on triggers a vmalloc fault for some reason, then we can go into > > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > > the variable to make it happen "once"). > > > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > Would be nice to see the warning quoted that triggered this. > > Sure, want me to add this to the change log? Yeah, that would be helpful - but only the stack trace portion I suspect, to make it clear what caused the fault. The one posted in the thread shows: [ 17.148755] [<c2825b08>] do_page_fault+0x8/0x10 [ 17.153926] [<c2823066>] error_code+0x5a/0x60 [ 17.158905] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 [ 17.164760] [<c208d1a9>] ? module_address_lookup+0x29/0xb0 [ 17.170999] [<c208dddb>] kallsyms_lookup+0x9b/0xb0 [ 17.186804] [<c208def4>] sprint_symbol+0x14/0x20 [ 17.192063] [<c208df1e>] __print_symbol+0x1e/0x40 [ 17.197430] [<c25e00d7>] ? ashmem_shrink+0x77/0xf0 [ 17.202895] [<c25e13e0>] ? logger_aio_write+0x230/0x230 [ 17.208845] [<c205bdf5>] ? up+0x25/0x40 [ 17.213242] [<c2039cb7>] ? console_unlock+0x337/0x440 [ 17.218998] [<c2818236>] ? printk+0x38/0x3a [ 17.223782] [<c20006d0>] __show_regs+0x70/0x190 [ 17.228954] [<c200353a>] show_regs+0x3a/0x1b0 [ 17.233931] [<c2818236>] ? printk+0x38/0x3a [ 17.238717] [<c2824182>] arch_trigger_all_cpu_backtrace_handler+0x62/0x80 [ 17.246413] [<c2823919>] nmi_handle.isra.0+0x39/0x60 [ 17.252071] [<c2823a29>] do_nmi+0xe9/0x3f0 So kallsyms_lookup() faulted, while the NMI watchdog triggered a show_regs()? How is that possible? Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 12:51 ` Ingo Molnar @ 2013-10-16 13:01 ` Borislav Petkov 2013-10-16 13:03 ` Steven Rostedt 2013-10-17 0:29 ` Liu, Chuansheng 2 siblings, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2013-10-16 13:01 UTC (permalink / raw) To: Ingo Molnar Cc: Steven Rostedt, LKML, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 02:51:11PM +0200, Ingo Molnar wrote: > The one posted in the thread shows: > > [ 17.148755] [<c2825b08>] do_page_fault+0x8/0x10 > [ 17.153926] [<c2823066>] error_code+0x5a/0x60 > [ 17.158905] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 > [ 17.164760] [<c208d1a9>] ? module_address_lookup+0x29/0xb0 > [ 17.170999] [<c208dddb>] kallsyms_lookup+0x9b/0xb0 > [ 17.186804] [<c208def4>] sprint_symbol+0x14/0x20 > [ 17.192063] [<c208df1e>] __print_symbol+0x1e/0x40 > [ 17.197430] [<c25e00d7>] ? ashmem_shrink+0x77/0xf0 > [ 17.202895] [<c25e13e0>] ? logger_aio_write+0x230/0x230 > [ 17.208845] [<c205bdf5>] ? up+0x25/0x40 > [ 17.213242] [<c2039cb7>] ? console_unlock+0x337/0x440 > [ 17.218998] [<c2818236>] ? printk+0x38/0x3a > [ 17.223782] [<c20006d0>] __show_regs+0x70/0x190 > [ 17.228954] [<c200353a>] show_regs+0x3a/0x1b0 > [ 17.233931] [<c2818236>] ? printk+0x38/0x3a > [ 17.238717] [<c2824182>] arch_trigger_all_cpu_backtrace_handler+0x62/0x80 > [ 17.246413] [<c2823919>] nmi_handle.isra.0+0x39/0x60 > [ 17.252071] [<c2823a29>] do_nmi+0xe9/0x3f0 Btw, you probably should drop all the numbers and leave only the function names in the stack trace. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 12:51 ` Ingo Molnar 2013-10-16 13:01 ` Borislav Petkov @ 2013-10-16 13:03 ` Steven Rostedt 2013-10-17 0:29 ` Liu, Chuansheng 2 siblings, 0 replies; 18+ messages in thread From: Steven Rostedt @ 2013-10-16 13:03 UTC (permalink / raw) To: Ingo Molnar Cc: LKML, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, 16 Oct 2013 14:51:11 +0200 Ingo Molnar <mingo@kernel.org> wrote: > So kallsyms_lookup() faulted, while the NMI watchdog triggered a > show_regs()? How is that possible? It was a vmalloc fault. Do modules keep their symbol tables in a vmalloced area? If so, I think the get_ksymbol() can fault when searching for a module address. -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 12:51 ` Ingo Molnar 2013-10-16 13:01 ` Borislav Petkov 2013-10-16 13:03 ` Steven Rostedt @ 2013-10-17 0:29 ` Liu, Chuansheng 2 siblings, 0 replies; 18+ messages in thread From: Liu, Chuansheng @ 2013-10-17 0:29 UTC (permalink / raw) To: Ingo Molnar, Steven Rostedt Cc: LKML, Thomas Gleixner, H. Peter Anvin, Frederic Weisbecker, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi > -----Original Message----- > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo > Molnar > Sent: Wednesday, October 16, 2013 8:51 PM > To: Steven Rostedt > Cc: LKML; Thomas Gleixner; H. Peter Anvin; Frederic Weisbecker; Andrew > Morton; paulmck@linux.vnet.ibm.com; Peter Zijlstra; x86@kernel.org; Wang, > Xiaoming; Li, Zhuangzhi; Liu, Chuansheng > Subject: Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault > > > * Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Wed, 16 Oct 2013 08:11:18 +0200 > > Ingo Molnar <mingo@kernel.org> wrote: > > > > > > > > * Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > > Since the NMI iretq nesting has been fixed, there's no reason that > > > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > > > are taken in that code path, and the software now handles nested NMIs > > > > when the fault re-enables NMIs on iretq. > > > > > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > > > warn on triggers a vmalloc fault for some reason, then we can go into > > > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > > > the variable to make it happen "once"). > > > > > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > > > Would be nice to see the warning quoted that triggered this. > > > > Sure, want me to add this to the change log? > > Yeah, that would be helpful - but only the stack trace portion I suspect, > to make it clear what caused the fault. > > The one posted in the thread shows: > > [ 17.148755] [<c2825b08>] do_page_fault+0x8/0x10 > [ 17.153926] [<c2823066>] error_code+0x5a/0x60 > [ 17.158905] [<c2825b00>] ? __do_page_fault+0x4a0/0x4a0 > [ 17.164760] [<c208d1a9>] ? module_address_lookup+0x29/0xb0 > [ 17.170999] [<c208dddb>] kallsyms_lookup+0x9b/0xb0 > [ 17.186804] [<c208def4>] sprint_symbol+0x14/0x20 > [ 17.192063] [<c208df1e>] __print_symbol+0x1e/0x40 > [ 17.197430] [<c25e00d7>] ? ashmem_shrink+0x77/0xf0 > [ 17.202895] [<c25e13e0>] ? logger_aio_write+0x230/0x230 > [ 17.208845] [<c205bdf5>] ? up+0x25/0x40 > [ 17.213242] [<c2039cb7>] ? console_unlock+0x337/0x440 > [ 17.218998] [<c2818236>] ? printk+0x38/0x3a > [ 17.223782] [<c20006d0>] __show_regs+0x70/0x190 > [ 17.228954] [<c200353a>] show_regs+0x3a/0x1b0 > [ 17.233931] [<c2818236>] ? printk+0x38/0x3a > [ 17.238717] [<c2824182>] > arch_trigger_all_cpu_backtrace_handler+0x62/0x80 > [ 17.246413] [<c2823919>] nmi_handle.isra.0+0x39/0x60 > [ 17.252071] [<c2823a29>] do_nmi+0xe9/0x3f0 > > So kallsyms_lookup() faulted, while the NMI watchdog triggered a > show_regs()? How is that possible? Not NMI watchdog triggered show_regs(), when we call arch_trigger_all_cpu_backtrace(), the NMI handler arch_trigger_all_cpu_backtrace_handler() will call show_regs(). > > Thanks, > > Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-15 20:39 [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault Steven Rostedt 2013-10-16 6:11 ` Ingo Molnar @ 2013-10-16 11:40 ` Frederic Weisbecker 2013-10-16 12:45 ` Steven Rostedt 1 sibling, 1 reply; 18+ messages in thread From: Frederic Weisbecker @ 2013-10-16 11:40 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Tue, Oct 15, 2013 at 04:39:06PM -0400, Steven Rostedt wrote: > Since the NMI iretq nesting has been fixed, there's no reason that > an NMI handler can not take a page fault for vmalloc'd code. No locks > are taken in that code path, and the software now handles nested NMIs > when the fault re-enables NMIs on iretq. > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > warn on triggers a vmalloc fault for some reason, then we can go into > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > the variable to make it happen "once"). > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> Thanks! For now we probably indeed want this patch. But I hope it's only for the short term. I still think that allowing faults in NMIs is very nasty, as we expect NMIs to never be disturbed. I'm not even sure if that interacts correctly with the rcu_nmi_enter() and preempt_count & NMI_MASK things. Not sure how perf is ready for that either (now hardware events can be interrupted by fault trace events). So I hope we can think about something else for the long term. > --- > arch/x86/mm/fault.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index 3aaeffc..78926c6 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -268,8 +268,6 @@ static noinline __kprobes int vmalloc_fault(unsigned long address) > if (!(address >= VMALLOC_START && address < VMALLOC_END)) > return -1; > > - WARN_ON_ONCE(in_nmi()); > - > /* > * Synchronize this task's top level page-table > * with the 'reference' page table. > -- > 1.8.1.4 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 11:40 ` Frederic Weisbecker @ 2013-10-16 12:45 ` Steven Rostedt 2013-10-16 13:08 ` Frederic Weisbecker 0 siblings, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2013-10-16 12:45 UTC (permalink / raw) To: Frederic Weisbecker Cc: LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, 16 Oct 2013 13:40:37 +0200 Frederic Weisbecker <fweisbec@gmail.com> wrote: > On Tue, Oct 15, 2013 at 04:39:06PM -0400, Steven Rostedt wrote: > > Since the NMI iretq nesting has been fixed, there's no reason that > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > are taken in that code path, and the software now handles nested NMIs > > when the fault re-enables NMIs on iretq. > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > warn on triggers a vmalloc fault for some reason, then we can go into > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > the variable to make it happen "once"). > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > Thanks! For now we probably indeed want this patch. But I hope it's only > for the short term. Why? > > I still think that allowing faults in NMIs is very nasty, as we expect NMIs to never > be disturbed. We do faults (well, breakpoints really) in NMI to enable tracing. > I'm not even sure if that interacts correctly with the rcu_nmi_enter() > and preempt_count & NMI_MASK things. Not sure how perf is ready for that either (now > hardware events can be interrupted by fault trace events). I'm a bit confused. What doesn't interact correctly with rcu_nmi_enter()? > > So I hope we can think about something else for the long term. I still don't understand what's wrong with it. As long as the faulting code does not grab any locks there shouldn't be anything wrong with faulting in NMI. For vmalloc, it is just updating page tables. -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 12:45 ` Steven Rostedt @ 2013-10-16 13:08 ` Frederic Weisbecker 2013-10-16 13:14 ` Steven Rostedt 2013-10-16 19:36 ` Paul E. McKenney 0 siblings, 2 replies; 18+ messages in thread From: Frederic Weisbecker @ 2013-10-16 13:08 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 08:45:18AM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 13:40:37 +0200 > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > On Tue, Oct 15, 2013 at 04:39:06PM -0400, Steven Rostedt wrote: > > > Since the NMI iretq nesting has been fixed, there's no reason that > > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > > are taken in that code path, and the software now handles nested NMIs > > > when the fault re-enables NMIs on iretq. > > > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > > warn on triggers a vmalloc fault for some reason, then we can go into > > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > > the variable to make it happen "once"). > > > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > Thanks! For now we probably indeed want this patch. But I hope it's only > > for the short term. > > Why? > > > > > I still think that allowing faults in NMIs is very nasty, as we expect NMIs to never > > be disturbed. > > We do faults (well, breakpoints really) in NMI to enable tracing. > > > I'm not even sure if that interacts correctly with the rcu_nmi_enter() > > and preempt_count & NMI_MASK things. Not sure how perf is ready for that either (now > > hardware events can be interrupted by fault trace events). > > I'm a bit confused. What doesn't interact correctly with > rcu_nmi_enter()? Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the wrong way. Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects us against that. > > > > > So I hope we can think about something else for the long term. > > I still don't understand what's wrong with it. As long as the faulting > code does not grab any locks there shouldn't be anything wrong with > faulting in NMI. For vmalloc, it is just updating page tables. NMI code is written with the idea that it can't be interrupted. May be that paranoia (again), you know. And I can't point you any problem in practice. I just think that allowing such a thing is asking for troubles. But I'm ok with your patch, it fixes a real bug and as long as we don't have a better solution, we should keep that. BTW, does faulting in NMIs re-enable NMIs? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 13:08 ` Frederic Weisbecker @ 2013-10-16 13:14 ` Steven Rostedt 2013-10-16 13:28 ` Frederic Weisbecker 2013-10-16 19:36 ` Paul E. McKenney 1 sibling, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2013-10-16 13:14 UTC (permalink / raw) To: Frederic Weisbecker Cc: LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, 16 Oct 2013 15:08:57 +0200 Frederic Weisbecker <fweisbec@gmail.com> wrote: > Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen > between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the > wrong way. > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > us against that. I will say that we should probably warn if it's any fault other than a vmalloc fault. A vmalloc fault should only happen in kernel space, and should not be happening from user code. > > > > > > > > > So I hope we can think about something else for the long term. > > > > I still don't understand what's wrong with it. As long as the faulting > > code does not grab any locks there shouldn't be anything wrong with > > faulting in NMI. For vmalloc, it is just updating page tables. > > NMI code is written with the idea that it can't be interrupted. May be that > paranoia (again), you know. And I can't point you any problem in practice. > I just think that allowing such a thing is asking for troubles. The WARN_ON() that I removed is from vmalloc fault. I don't see an issue with NMIs faulting via vmalloc. For any other page fault, sure, I would be concerned about it. But what's wrong with an NMI running module code? > > But I'm ok with your patch, it fixes a real bug and as long as we don't have > a better solution, we should keep that. > > BTW, does faulting in NMIs re-enable NMIs? Yes, but we now have code to handle that :-) -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 13:14 ` Steven Rostedt @ 2013-10-16 13:28 ` Frederic Weisbecker 2013-10-16 13:37 ` Steven Rostedt 0 siblings, 1 reply; 18+ messages in thread From: Frederic Weisbecker @ 2013-10-16 13:28 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 09:14:37AM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 15:08:57 +0200 > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > > Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen > > between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the > > wrong way. > > > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > > us against that. > > I will say that we should probably warn if it's any fault other than a > vmalloc fault. A vmalloc fault should only happen in kernel space, and > should not be happening from user code. The NMI can interrupt userspace. When the fault happens, it sees that context tracking state is set to userspace (NMIs and interrupts in general don't exit that state, hence the in_interrupt() check that returns when user_exit/enter is called) so it calls user_enter(). But anyway we should be protected against that. > > > > > > > > > > > > > > So I hope we can think about something else for the long term. > > > > > > I still don't understand what's wrong with it. As long as the faulting > > > code does not grab any locks there shouldn't be anything wrong with > > > faulting in NMI. For vmalloc, it is just updating page tables. > > > > NMI code is written with the idea that it can't be interrupted. May be that > > paranoia (again), you know. And I can't point you any problem in practice. > > I just think that allowing such a thing is asking for troubles. > > The WARN_ON() that I removed is from vmalloc fault. I don't see an > issue with NMIs faulting via vmalloc. For any other page fault, sure, I > would be concerned about it. But what's wrong with an NMI running > module code? I won't argue further as none of us is going to change his opinion on this :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 13:28 ` Frederic Weisbecker @ 2013-10-16 13:37 ` Steven Rostedt 2013-10-16 19:39 ` Paul E. McKenney 0 siblings, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2013-10-16 13:37 UTC (permalink / raw) To: Frederic Weisbecker Cc: LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, paulmck, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, 16 Oct 2013 15:28:15 +0200 Frederic Weisbecker <fweisbec@gmail.com> wrote: > On Wed, Oct 16, 2013 at 09:14:37AM -0400, Steven Rostedt wrote: > > On Wed, 16 Oct 2013 15:08:57 +0200 > > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > > > > > Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen > > > between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the > > > wrong way. > > > > > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > > > us against that. > > > > I will say that we should probably warn if it's any fault other than a > > vmalloc fault. A vmalloc fault should only happen in kernel space, and > > should not be happening from user code. > > The NMI can interrupt userspace. When the fault happens, it sees that context tracking > state is set to userspace (NMIs and interrupts in general don't exit that state, hence > the in_interrupt() check that returns when user_exit/enter is called) so it calls user_enter(). > But anyway we should be protected against that. IIRC, NMI itself is safe to use rcu_read_lock(), at least I remember Paul making sure that stuff was lockless and NMI safe. > > The WARN_ON() that I removed is from vmalloc fault. I don't see an > > issue with NMIs faulting via vmalloc. For any other page fault, sure, I > > would be concerned about it. But what's wrong with an NMI running > > module code? > > I won't argue further as none of us is going to change his opinion on this :) Sure sure, yet another argument continues with two sides stubbornly refusing to negotiate about a looming future (de)fault! -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 13:37 ` Steven Rostedt @ 2013-10-16 19:39 ` Paul E. McKenney 0 siblings, 0 replies; 18+ messages in thread From: Paul E. McKenney @ 2013-10-16 19:39 UTC (permalink / raw) To: Steven Rostedt Cc: Frederic Weisbecker, LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 09:37:12AM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 15:28:15 +0200 > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > On Wed, Oct 16, 2013 at 09:14:37AM -0400, Steven Rostedt wrote: > > > On Wed, 16 Oct 2013 15:08:57 +0200 > > > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > > > > > > > > Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen > > > > between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the > > > > wrong way. > > > > > > > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > > > > us against that. > > > > > > I will say that we should probably warn if it's any fault other than a > > > vmalloc fault. A vmalloc fault should only happen in kernel space, and > > > should not be happening from user code. > > > > The NMI can interrupt userspace. When the fault happens, it sees that context tracking > > state is set to userspace (NMIs and interrupts in general don't exit that state, hence > > the in_interrupt() check that returns when user_exit/enter is called) so it calls user_enter(). > > But anyway we should be protected against that. > > IIRC, NMI itself is safe to use rcu_read_lock(), at least I remember > Paul making sure that stuff was lockless and NMI safe. Yep, even preemptible RCU. This relies on the fact that we cannot be preempted within either an NMI handler or an exception handler. > > > The WARN_ON() that I removed is from vmalloc fault. I don't see an > > > issue with NMIs faulting via vmalloc. For any other page fault, sure, I > > > would be concerned about it. But what's wrong with an NMI running > > > module code? > > > > I won't argue further as none of us is going to change his opinion on this :) > > Sure sure, yet another argument continues with two sides stubbornly > refusing to negotiate about a looming future (de)fault! I figure some good hard testing will bring the truth of the matter to light. The arguing parties might well then wish that they had compromised so as to avoid the hard sharp truth, but by then it will be too late. ;-) Thanx, Paul ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 13:08 ` Frederic Weisbecker 2013-10-16 13:14 ` Steven Rostedt @ 2013-10-16 19:36 ` Paul E. McKenney 2013-10-16 19:57 ` Steven Rostedt 2013-10-17 13:59 ` Frederic Weisbecker 1 sibling, 2 replies; 18+ messages in thread From: Paul E. McKenney @ 2013-10-16 19:36 UTC (permalink / raw) To: Frederic Weisbecker Cc: Steven Rostedt, LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 03:08:57PM +0200, Frederic Weisbecker wrote: > On Wed, Oct 16, 2013 at 08:45:18AM -0400, Steven Rostedt wrote: > > On Wed, 16 Oct 2013 13:40:37 +0200 > > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > > > On Tue, Oct 15, 2013 at 04:39:06PM -0400, Steven Rostedt wrote: > > > > Since the NMI iretq nesting has been fixed, there's no reason that > > > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > > > are taken in that code path, and the software now handles nested NMIs > > > > when the fault re-enables NMIs on iretq. > > > > > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > > > warn on triggers a vmalloc fault for some reason, then we can go into > > > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > > > the variable to make it happen "once"). > > > > > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > > > Thanks! For now we probably indeed want this patch. But I hope it's only > > > for the short term. > > > > Why? > > > > > > > > I still think that allowing faults in NMIs is very nasty, as we expect NMIs to never > > > be disturbed. > > > > We do faults (well, breakpoints really) in NMI to enable tracing. > > > > > I'm not even sure if that interacts correctly with the rcu_nmi_enter() > > > and preempt_count & NMI_MASK things. Not sure how perf is ready for that either (now > > > hardware events can be interrupted by fault trace events). > > > > I'm a bit confused. What doesn't interact correctly with > > rcu_nmi_enter()? > > Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen > between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the > wrong way. I can attest to this! NMIs check for being nested within process/irq-based non-idle sojourns, but not the other way around. The result is that RCU will be ignoring you during that time, and not even disabling interrupts will save you. It will check rdtp->dynticks, see that its value is even, and register a quiescent state on behalf of the hapless CPU. > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > us against that. Here you are relying on the exception being treated as an interrupt, correct? Thanx, Paul > > > So I hope we can think about something else for the long term. > > > > I still don't understand what's wrong with it. As long as the faulting > > code does not grab any locks there shouldn't be anything wrong with > > faulting in NMI. For vmalloc, it is just updating page tables. > > NMI code is written with the idea that it can't be interrupted. May be that > paranoia (again), you know. And I can't point you any problem in practice. > I just think that allowing such a thing is asking for troubles. > > But I'm ok with your patch, it fixes a real bug and as long as we don't have > a better solution, we should keep that. > > BTW, does faulting in NMIs re-enable NMIs? > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 19:36 ` Paul E. McKenney @ 2013-10-16 19:57 ` Steven Rostedt 2013-10-18 11:54 ` Paul E. McKenney 2013-10-17 13:59 ` Frederic Weisbecker 1 sibling, 1 reply; 18+ messages in thread From: Steven Rostedt @ 2013-10-16 19:57 UTC (permalink / raw) To: paulmck Cc: Frederic Weisbecker, LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, 16 Oct 2013 12:36:32 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote: > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > > us against that. > > Here you are relying on the exception being treated as an interrupt, > correct? I don't think so. It's relying on nmi_enter() also makes in_interrupt() return true. Like I said before. An NMI interrupting userspace should be no different than an interrupt interrupting userspace. They both can trigger vmalloc faults, and we should be able to deal with it. -- Steve ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 19:57 ` Steven Rostedt @ 2013-10-18 11:54 ` Paul E. McKenney 0 siblings, 0 replies; 18+ messages in thread From: Paul E. McKenney @ 2013-10-18 11:54 UTC (permalink / raw) To: Steven Rostedt Cc: Frederic Weisbecker, LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 03:57:30PM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 12:36:32 -0700 > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote: > > > > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > > > us against that. > > > > Here you are relying on the exception being treated as an interrupt, > > correct? > > I don't think so. It's relying on nmi_enter() also makes in_interrupt() > return true. Got it, never mind! Thanx, Paul > Like I said before. An NMI interrupting userspace should be no > different than an interrupt interrupting userspace. They both can > trigger vmalloc faults, and we should be able to deal with it. > > -- Steve > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault 2013-10-16 19:36 ` Paul E. McKenney 2013-10-16 19:57 ` Steven Rostedt @ 2013-10-17 13:59 ` Frederic Weisbecker 1 sibling, 0 replies; 18+ messages in thread From: Frederic Weisbecker @ 2013-10-17 13:59 UTC (permalink / raw) To: Paul E. McKenney Cc: Steven Rostedt, LKML, Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Morton, Peter Zijlstra, x86, Wang, Xiaoming, Li, Zhuangzhi, Liu, Chuansheng On Wed, Oct 16, 2013 at 12:36:32PM -0700, Paul E. McKenney wrote: > On Wed, Oct 16, 2013 at 03:08:57PM +0200, Frederic Weisbecker wrote: > > On Wed, Oct 16, 2013 at 08:45:18AM -0400, Steven Rostedt wrote: > > > On Wed, 16 Oct 2013 13:40:37 +0200 > > > Frederic Weisbecker <fweisbec@gmail.com> wrote: > > > > > > > On Tue, Oct 15, 2013 at 04:39:06PM -0400, Steven Rostedt wrote: > > > > > Since the NMI iretq nesting has been fixed, there's no reason that > > > > > an NMI handler can not take a page fault for vmalloc'd code. No locks > > > > > are taken in that code path, and the software now handles nested NMIs > > > > > when the fault re-enables NMIs on iretq. > > > > > > > > > > Not only that, if the vmalloc_fault() WARN_ON_ONCE() is hit, and that > > > > > warn on triggers a vmalloc fault for some reason, then we can go into > > > > > an infinite loop (the WARN_ON_ONCE() does the WARN() before updating > > > > > the variable to make it happen "once"). > > > > > > > > > > Reported-by: "Liu, Chuansheng" <chuansheng.liu@intel.com> > > > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > > > > > Thanks! For now we probably indeed want this patch. But I hope it's only > > > > for the short term. > > > > > > Why? > > > > > > > > > > > I still think that allowing faults in NMIs is very nasty, as we expect NMIs to never > > > > be disturbed. > > > > > > We do faults (well, breakpoints really) in NMI to enable tracing. > > > > > > > I'm not even sure if that interacts correctly with the rcu_nmi_enter() > > > > and preempt_count & NMI_MASK things. Not sure how perf is ready for that either (now > > > > hardware events can be interrupted by fault trace events). > > > > > > I'm a bit confused. What doesn't interact correctly with > > > rcu_nmi_enter()? > > > > Faults can call rcu_user_exit() / rcu_user_enter(). This is not supposed to happen > > between rcu_nmi_enter() and rcu_nmi_exit(). rdtp->dynticks would be incremented in the > > wrong way. > > I can attest to this! NMIs check for being nested within > process/irq-based non-idle sojourns, but not the other way around. > The result is that RCU will be ignoring you during that time, and not > even disabling interrupts will save you. It will check rdtp->dynticks, > see that its value is even, and register a quiescent state on behalf of > the hapless CPU. Fortunately, we are avoiding this with the in_interrupt() check on user_enter() and user_exit(). Their goal is precisely to deal with traps/faults happening on interrupts :) > > > Ah but we have an in_interrupt() check in context_tracking_user_enter() that protects > > us against that. > > Here you are relying on the exception being treated as an interrupt, > correct? >From an RCU point of view yeah. In these cases the exception is either protected under rcu_irq_* and rcu_nmi* APIs, depending on where it happened. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-10-18 11:55 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-10-15 20:39 [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault Steven Rostedt 2013-10-16 6:11 ` Ingo Molnar 2013-10-16 12:42 ` Steven Rostedt 2013-10-16 12:51 ` Ingo Molnar 2013-10-16 13:01 ` Borislav Petkov 2013-10-16 13:03 ` Steven Rostedt 2013-10-17 0:29 ` Liu, Chuansheng 2013-10-16 11:40 ` Frederic Weisbecker 2013-10-16 12:45 ` Steven Rostedt 2013-10-16 13:08 ` Frederic Weisbecker 2013-10-16 13:14 ` Steven Rostedt 2013-10-16 13:28 ` Frederic Weisbecker 2013-10-16 13:37 ` Steven Rostedt 2013-10-16 19:39 ` Paul E. McKenney 2013-10-16 19:36 ` Paul E. McKenney 2013-10-16 19:57 ` Steven Rostedt 2013-10-18 11:54 ` Paul E. McKenney 2013-10-17 13:59 ` Frederic Weisbecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®