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