Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
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" 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
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 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" > > > > > Signed-off-by: Steven Rostedt > > > > > > > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
> -Original Message- > From: Ingo Molnar [mailto:mingo.kernel@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; paul...@linux.vnet.ibm.com; Peter Zijlstra; x...@kernel.org; Wang, > Xiaoming; Li, Zhuangzhi; Liu, Chuansheng > Subject: Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault > > > * Steven Rostedt wrote: > > > On Wed, 16 Oct 2013 08:11:18 +0200 > > Ingo Molnar wrote: > > > > > > > > * 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" > > > > Signed-off-by: Steven Rostedt > > > > > > 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] [] do_page_fault+0x8/0x10 > [ 17.153926] [] error_code+0x5a/0x60 > [ 17.158905] [] ? __do_page_fault+0x4a0/0x4a0 > [ 17.164760] [] ? module_address_lookup+0x29/0xb0 > [ 17.170999] [] kallsyms_lookup+0x9b/0xb0 > [ 17.186804] [] sprint_symbol+0x14/0x20 > [ 17.192063] [] __print_symbol+0x1e/0x40 > [ 17.197430] [] ? ashmem_shrink+0x77/0xf0 > [ 17.202895] [] ? logger_aio_write+0x230/0x230 > [ 17.208845] [] ? up+0x25/0x40 > [ 17.213242] [] ? console_unlock+0x337/0x440 > [ 17.218998] [] ? printk+0x38/0x3a > [ 17.223782] [] __show_regs+0x70/0x190 > [ 17.228954] [] show_regs+0x3a/0x1b0 > [ 17.233931] [] ? printk+0x38/0x3a > [ 17.238717] [] > arch_trigger_all_cpu_backtrace_handler+0x62/0x80 > [ 17.246413] [] nmi_handle.isra.0+0x39/0x60 > [ 17.252071] [] 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, 16 Oct 2013 12:36:32 -0700 "Paul E. McKenney" 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, Oct 16, 2013 at 09:37:12AM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 15:28:15 +0200 > Frederic Weisbecker 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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
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 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" > > > > Signed-off-by: Steven Rostedt > > > > > > 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? > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, 16 Oct 2013 15:28:15 +0200 Frederic Weisbecker 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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, Oct 16, 2013 at 09:14:37AM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 15:08:57 +0200 > Frederic Weisbecker 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 :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, 16 Oct 2013 15:08:57 +0200 Frederic Weisbecker 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, Oct 16, 2013 at 08:45:18AM -0400, Steven Rostedt wrote: > On Wed, 16 Oct 2013 13:40:37 +0200 > Frederic Weisbecker 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" > > > Signed-off-by: Steven Rostedt > > > > 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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, 16 Oct 2013 14:51:11 +0200 Ingo Molnar 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, Oct 16, 2013 at 02:51:11PM +0200, Ingo Molnar wrote: > The one posted in the thread shows: > > [ 17.148755] [] do_page_fault+0x8/0x10 > [ 17.153926] [] error_code+0x5a/0x60 > [ 17.158905] [] ? __do_page_fault+0x4a0/0x4a0 > [ 17.164760] [] ? module_address_lookup+0x29/0xb0 > [ 17.170999] [] kallsyms_lookup+0x9b/0xb0 > [ 17.186804] [] sprint_symbol+0x14/0x20 > [ 17.192063] [] __print_symbol+0x1e/0x40 > [ 17.197430] [] ? ashmem_shrink+0x77/0xf0 > [ 17.202895] [] ? logger_aio_write+0x230/0x230 > [ 17.208845] [] ? up+0x25/0x40 > [ 17.213242] [] ? console_unlock+0x337/0x440 > [ 17.218998] [] ? printk+0x38/0x3a > [ 17.223782] [] __show_regs+0x70/0x190 > [ 17.228954] [] show_regs+0x3a/0x1b0 > [ 17.233931] [] ? printk+0x38/0x3a > [ 17.238717] [] arch_trigger_all_cpu_backtrace_handler+0x62/0x80 > [ 17.246413] [] nmi_handle.isra.0+0x39/0x60 > [ 17.252071] [] 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. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
* Steven Rostedt wrote: > On Wed, 16 Oct 2013 08:11:18 +0200 > Ingo Molnar wrote: > > > > > * 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" > > > Signed-off-by: Steven Rostedt > > > > 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] [] do_page_fault+0x8/0x10 [ 17.153926] [] error_code+0x5a/0x60 [ 17.158905] [] ? __do_page_fault+0x4a0/0x4a0 [ 17.164760] [] ? module_address_lookup+0x29/0xb0 [ 17.170999] [] kallsyms_lookup+0x9b/0xb0 [ 17.186804] [] sprint_symbol+0x14/0x20 [ 17.192063] [] __print_symbol+0x1e/0x40 [ 17.197430] [] ? ashmem_shrink+0x77/0xf0 [ 17.202895] [] ? logger_aio_write+0x230/0x230 [ 17.208845] [] ? up+0x25/0x40 [ 17.213242] [] ? console_unlock+0x337/0x440 [ 17.218998] [] ? printk+0x38/0x3a [ 17.223782] [] __show_regs+0x70/0x190 [ 17.228954] [] show_regs+0x3a/0x1b0 [ 17.233931] [] ? printk+0x38/0x3a [ 17.238717] [] arch_trigger_all_cpu_backtrace_handler+0x62/0x80 [ 17.246413] [] nmi_handle.isra.0+0x39/0x60 [ 17.252071] [] do_nmi+0xe9/0x3f0 So kallsyms_lookup() faulted, while the NMI watchdog triggered a show_regs()? How is that possible? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, 16 Oct 2013 13:40:37 +0200 Frederic Weisbecker 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" > > Signed-off-by: Steven Rostedt > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
On Wed, 16 Oct 2013 08:11:18 +0200 Ingo Molnar wrote: > > * 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" > > Signed-off-by: Steven Rostedt > > 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: [] print_context_stack+0x4a/0xa0 [ 15.082529] *pde = [ 15.085758] Thread overran stack, or stack corrupted [ 15.091303] Oops: [#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: GW O 3.10.1+ #1 [ 15.119075] task: f213f980 ti: f0c42000 task.ti: f0c42000 [ 15.125116] EIP: 0060:[] 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: EDI: e000 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: DR1: DR2: DR3: [ 15.170789] DR6: 0ff0 DR7: 0400 [ 15.175076] Stack: [ 15.177324] 16497ffc 16496000 986736d8 e000 986736d8 1649736d c282c148 16496000 [ 15.186067] f0c4223c c20033b0 c282c148 c29ceecf f0c4222c 986736d8 f0c4222c [ 15.194810] c29ceecf f0c42260 c20041a7 f0c4229c c282c148 [ 15.203549] Call Trace: [ 15.206295] [] dump_trace+0x70/0xf0 [ 15.211274] [] show_trace_log_lvl+0x47/0x60 [ 15.217028] [] show_stack_log_lvl+0x52/0xd0 [ 15.222782] [] show_stack+0x21/0x50 [ 15.227762] [] dump_stack+0x16/0x18 [ 15.232742] [] warn_slowpath_common+0x5f/0x80 [ 15.238693] [] ? vmalloc_fault+0x5a/0xcf [ 15.244156] [] ? vmalloc_fault+0x5a/0xcf [ 15.249621] [] ? __do_page_fault+0x4a0/0x4a0 [ 15.255472] [] warn_slowpath_null+0x1d/0x20 [ 15.261228] [] vmalloc_fault+0x5a/0xcf [ 15.266497] [] __do_page_fault+0x2cf/0x4a0 [ 15.272154] [] ? logger_aio_write+0x230/0x230 [ 15.278106] [] ? console_unlock+0x314/0x440 ... // [ 16.885364] [] ? __do_page_fault+0x4a0/0x4a0 [ 16.891217] [] do_page_fault+0x8/0x10 [ 16.896387] [] error_code+0x5a/0x60 [ 16.901367] [] ? __do_page_fault+0x4a0/0x4a0 [ 16.907219] [] ? print_modules+0x20/0x90 [ 16.912685] [] warn_slowpath_common+0x5a/0x80 [ 16.918634] [] ? vmalloc_fault+0x5a/0xcf [ 16.924097] [] ? vmalloc_fault+0x5a/0xcf [ 16.929562] [] ? __do_page_fault+0x4a0/0x4a0 [ 16.935415] [] warn_slowpath_null+0x1d/0x20 [ 16.941169] [] vmalloc_fault+0x5a/0xcf [ 16.946437] [] __do_page_fault+0x2cf/0x4a0 [ 16.952095] [] ? logger_aio_write+0x230/0x230 [ 16.958046] [] ? console_unlock+0x314/0x440 [ 16.963800] [] ? sys_modify_ldt+0x2/0x160 [ 16.969362] [] ? __do_page_fault+0x4a0/0x4a0 [ 16.975215] [] do_page_fault+0x8/0x10 [ 16.980386] [] error_code+0x5a/0x60 [ 16.985366] [] ? __do_page_fault+0x4a0/0x4a0 [ 16.991215] [] ? print_modules+0x20/0x90 [ 16.996673] [] warn_slowpath_common+0x5a/0x80 [ 17.002622] [] ? vmalloc_fault+0x5a/0xcf [ 17.008086] [] ? vmalloc_fault+0x5a/0xcf [ 17.013550] [] ? __do_page_fault+0x4a0/0x4a0 [ 17.019403] [] warn_slowpath_null+0x1d/0x20 [ 17.025159] [] vmalloc_fault+0x5a/0xcf [ 17.030428] [] __do_page_fault+0x2cf/0x4a0 [ 17.036085] [] ? logger_aio_write+0x230/0x230 [ 17.042037] [] ? console_unlock+0x314/0x440 [ 17.047790] [] ? sys_modify_ldt+0x2/0x160 [ 17.053352] [] ? __do_page_fault+0x4a0/0x4a0 [ 17.059205] [] do_page_fault+0x8/0x10 [ 17.064375] [] error_code+0x5a/0x60 [ 17.069354] [] ? __do_page_fault+0x4a0/0x4a0 [ 17.075204] [] ? print_modules+0x20/0x90 [ 17.080669] [] warn_slowpath_common+0x5a/0x80 [ 17.086619] [] ? vmalloc_fault+0x5a/0xcf [ 17.092082] [] ? vmalloc_fault+0x5a/0xcf [ 17.097546] [] ? __do_page_fault+0x4a0/0x4a0 [ 17.103399] [] warn_slowpath_null+0x1d/0x20 [ 17.109154] [] vmalloc_fault+0x5a/0xcf [ 17.114422] [] __do_page_fault+0x2cf/0x4a0 [ 17.120080] [] ? update_group_power+0x1fd/0x240 [ 17.126224] [] ? number.isra.2+0x32b/0x330 [ 17.131880] [] ? update_curr+0xac/0x190 [ 17.137247] [] ? number.isra.2+0x32b/0x330 [ 17.14
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
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" > Signed-off-by: Steven Rostedt 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Remove WARN_ON(in_nmi()) from vmalloc_fault
* 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" > Signed-off-by: Steven Rostedt Would be nice to see the warning quoted that triggered this. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
