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
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
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
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
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
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
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
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]
> > >
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
> >
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
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
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.
> >
> >
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();
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]
>
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
> >
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
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
[
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
18 matches
Mail list logo