Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Sergey Senozhatsky
On (09/07/18 16:03), Peter Zijlstra wrote: > > > > I would even argue that placing printk_nmi_enter() between > > lockdep_off() and ftrace_nmi_enter() is wrong because if in the future > > printk_nmi_enter() were to do any ftrace tracing, it wont be caught, as > > it was by having it before lockde

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Peter Zijlstra
On Fri, Sep 07, 2018 at 10:01:28AM -0400, Steven Rostedt wrote: > On Fri, 7 Sep 2018 09:55:33 -0400 > Steven Rostedt wrote: > > > On Fri, 7 Sep 2018 15:45:32 +0200 > > Peter Zijlstra wrote: > > > > > Yes really, we should not muck with the IRQ state from NMI context. > > > > Right, and we di

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Peter Zijlstra
On Fri, Sep 07, 2018 at 09:55:33AM -0400, Steven Rostedt wrote: > On Fri, 7 Sep 2018 15:45:32 +0200 > Peter Zijlstra wrote: > > > Yes really, we should not muck with the IRQ state from NMI context. > > Right, and we didn't. Your patch didn't change anything, but allow for It does, it kills lock

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Steven Rostedt
On Fri, 7 Sep 2018 09:55:33 -0400 Steven Rostedt wrote: > On Fri, 7 Sep 2018 15:45:32 +0200 > Peter Zijlstra wrote: > > > Yes really, we should not muck with the IRQ state from NMI context. > > Right, and we didn't. Your patch didn't change anything, but allow for > printk_nmi_enter/exit() t

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Steven Rostedt
On Fri, 7 Sep 2018 15:45:32 +0200 Peter Zijlstra wrote: > Yes really, we should not muck with the IRQ state from NMI context. Right, and we didn't. Your patch didn't change anything, but allow for printk_nmi_enter/exit() to be traced by ftrace, but that's wrong to begin with because it ftrace_nm

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Steven Rostedt
On Fri, 7 Sep 2018 11:30:55 +0200 Peter Zijlstra wrote: > > That said, I am not against this change. Especially the inlining > > is a good move. Note that lockdep_off()/lockdep_on() must not > > be traced as well. > > Hard to trace an inline funcion; we could make it __always_inline to > feel

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Steven Rostedt
On Fri, 7 Sep 2018 17:35:38 +0900 Sergey Senozhatsky wrote: > Should't printk_nmi_enter()/printk_nmi_exit() still be notrace? > Like you and Steven said - it's still before ftrace_nmi_enter() > and should be notrace regardless. Correct. My patch fixes both issues. The patch that Peter is postin

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Peter Zijlstra
On Fri, Sep 07, 2018 at 09:41:48AM -0400, Steven Rostedt wrote: > On Fri, 7 Sep 2018 09:34:48 +0200 > Peter Zijlstra wrote: > > > On Wed, Sep 05, 2018 at 09:33:34PM -0400, Steven Rostedt wrote: > > > do_idle { > > > > > > [interrupts enabled] > > > > > > [interrupts disabled] > > >

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Steven Rostedt
On Fri, 7 Sep 2018 09:34:48 +0200 Peter Zijlstra wrote: > On Wed, Sep 05, 2018 at 09:33:34PM -0400, Steven Rostedt wrote: > > do_idle { > > > > [interrupts enabled] > > > > [interrupts disabled] > > TRACE_IRQS_OFF [lockdep says irqs off] > > [...] > > TRACE_IRQS_IRET > >

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Peter Zijlstra
On Fri, Sep 07, 2018 at 10:28:34AM +0200, Petr Mladek wrote: > On Fri 2018-09-07 09:45:31, Peter Zijlstra wrote: > > On Thu, Sep 06, 2018 at 11:31:51AM +0900, Sergey Senozhatsky wrote: > > > An alternative option, thus, could be re-instating back the rule that > > > lockdep_off/on should be the fir

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Sergey Senozhatsky
On (09/07/18 10:28), Petr Mladek wrote: > On Fri 2018-09-07 09:45:31, Peter Zijlstra wrote: > > On Thu, Sep 06, 2018 at 11:31:51AM +0900, Sergey Senozhatsky wrote: > > > An alternative option, thus, could be re-instating back the rule that > > > lockdep_off/on should be the first and the last thing

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Petr Mladek
On Fri 2018-09-07 09:45:31, Peter Zijlstra wrote: > On Thu, Sep 06, 2018 at 11:31:51AM +0900, Sergey Senozhatsky wrote: > > An alternative option, thus, could be re-instating back the rule that > > lockdep_off/on should be the first and the last thing we do in > > nmi_enter/nmi_exit. E.g. > > > >

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Peter Zijlstra
On Thu, Sep 06, 2018 at 11:31:51AM +0900, Sergey Senozhatsky wrote: > An alternative option, thus, could be re-instating back the rule that > lockdep_off/on should be the first and the last thing we do in > nmi_enter/nmi_exit. E.g. > > nmi_enter() > lockdep_off(); > printk_nmi_enter();

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-07 Thread Peter Zijlstra
On Wed, Sep 05, 2018 at 09:33:34PM -0400, Steven Rostedt wrote: > do_idle { > > [interrupts enabled] > > [interrupts disabled] > TRACE_IRQS_OFF [lockdep says irqs off] > [...] > TRACE_IRQS_IRET > test if pt_regs say return to interrupts enabled [yes] >

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-06 Thread Steven Rostedt
On Thu, 6 Sep 2018 11:31:51 +0900 Sergey Senozhatsky wrote: > Hello, > > On (09/05/18 21:33), Steven Rostedt wrote: > > do_idle { > > > > [interrupts enabled] > > > > [interrupts disabled] > > TRACE_IRQS_OFF [lockdep says irqs off] > > [...] > > TRACE_IRQS_IRET > >

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-06 Thread Petr Mladek
On Thu 2018-09-06 11:31:51, Sergey Senozhatsky wrote: > Hello, > > On (09/05/18 21:33), Steven Rostedt wrote: > > do_idle { > > > > [interrupts enabled] > > > > [interrupts disabled] > > TRACE_IRQS_OFF [lockdep says irqs off] > > [...] > > TRACE_IRQS_IRET > > test

Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-05 Thread Sergey Senozhatsky
Hello, On (09/05/18 21:33), Steven Rostedt wrote: > do_idle { > > [interrupts enabled] > > [interrupts disabled] > TRACE_IRQS_OFF [lockdep says irqs off] > [...] > TRACE_IRQS_IRET > test if pt_regs say return to interrupts enabled [yes] > TRACE_IR

[PATCH] printk/tracing: Do not trace printk_nmi_enter()

2018-09-05 Thread Steven Rostedt
[ I'm currently testing this patch, and will push this to mainline, I'm just looking for acks and review bys. Also, Peter, this is the bug that my patch that differentiates if lockdep is broken or if the code is broken when lockdep_assert_irqs_{dis,en}abled() triggers, and would have bee