[GIT PULL] tracing: use raw spinlocks for trace_vprintk

[GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Steven Rostedt]

Ingo,

Please pull the latest tip/tracing/ftrace tree, which can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/tracing/ftrace


Steven Rostedt (1):
      tracing: use raw spinlocks for trace_vprintk

----
 kernel/trace/trace.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
---------------------------
commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
Author: Steven Rostedt <srostedt@redhat.com>
Date:   Tue Mar 10 17:16:35 2009 -0400

    tracing: use raw spinlocks for trace_vprintk
    
    Impact: prevent locking up by lockdep tracer
    
    The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
    call back into lockdep without locking up.
    
    Signed-off-by: Steven Rostedt <srostedt@redhat.com>

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8c6a902..4c97947 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1176,7 +1176,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
  */
 int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
 {
-	static DEFINE_SPINLOCK(trace_buf_lock);
+	static raw_spinlock_t trace_buf_lock =
+		(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
 	static u32 trace_buf[TRACE_BUF_SIZE];
 
 	struct ring_buffer_event *event;
@@ -1201,7 +1202,9 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
 	if (unlikely(atomic_read(&data->disabled)))
 		goto out;
 
-	spin_lock_irqsave(&trace_buf_lock, flags);
+	/* Lockdep uses trace_printk for lock tracing */
+	local_irq_save(flags);
+	__raw_spin_lock(&trace_buf_lock);
 	len = vbin_printf(trace_buf, TRACE_BUF_SIZE, fmt, args);
 
 	if (len > TRACE_BUF_SIZE || len < 0)
@@ -1220,7 +1223,8 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
 	ring_buffer_unlock_commit(tr->buffer, event);
 
 out_unlock:
-	spin_unlock_irqrestore(&trace_buf_lock, flags);
+	__raw_spin_unlock(&trace_buf_lock);
+	local_irq_restore(flags);
 
 out:
 	ftrace_preempt_enable(resched);

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Peter Zijlstra]

On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:

> commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> Author: Steven Rostedt <srostedt@redhat.com>
> Date:   Tue Mar 10 17:16:35 2009 -0400
> 
>     tracing: use raw spinlocks for trace_vprintk
>     
>     Impact: prevent locking up by lockdep tracer
>     
>     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
>     call back into lockdep without locking up.

Hmm, I did this when I posted the lockdep tracepoints, so someone then
did a bad copy/paste job when renaming ftrace_printk or something?

See efed792d6738964f399a508ef9e831cd60fa4657

>     Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8c6a902..4c97947 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1176,7 +1176,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
>   */
>  int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
>  {
> -	static DEFINE_SPINLOCK(trace_buf_lock);
> +	static raw_spinlock_t trace_buf_lock =
> +		(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
>  	static u32 trace_buf[TRACE_BUF_SIZE];
>  
>  	struct ring_buffer_event *event;
> @@ -1201,7 +1202,9 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
>  	if (unlikely(atomic_read(&data->disabled)))
>  		goto out;
>  
> -	spin_lock_irqsave(&trace_buf_lock, flags);
> +	/* Lockdep uses trace_printk for lock tracing */
> +	local_irq_save(flags);

Shouldn't you also use raw_local_irq_save() and friends?

> +	__raw_spin_lock(&trace_buf_lock);
>  	len = vbin_printf(trace_buf, TRACE_BUF_SIZE, fmt, args);
>  
>  	if (len > TRACE_BUF_SIZE || len < 0)
> @@ -1220,7 +1223,8 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
>  	ring_buffer_unlock_commit(tr->buffer, event);
>  
>  out_unlock:
> -	spin_unlock_irqrestore(&trace_buf_lock, flags);
> +	__raw_spin_unlock(&trace_buf_lock);
> +	local_irq_restore(flags);
>  
>  out:
>  	ftrace_preempt_enable(resched);
> 

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Frederic Weisbecker]

On Wed, Mar 11, 2009 at 07:59:24AM +0100, Peter Zijlstra wrote:
> On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> 
> > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > Author: Steven Rostedt <srostedt@redhat.com>
> > Date:   Tue Mar 10 17:16:35 2009 -0400
> > 
> >     tracing: use raw spinlocks for trace_vprintk
> >     
> >     Impact: prevent locking up by lockdep tracer
> >     
> >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> >     call back into lockdep without locking up.
> 
> Hmm, I did this when I posted the lockdep tracepoints, so someone then
> did a bad copy/paste job when renaming ftrace_printk or something?
> 
> See efed792d6738964f399a508ef9e831cd60fa4657



Must be my bad :-s
I think I lost this modification that was done on the old trace_vprintf
between two iterations of the bprintk patchset.

BTW, Ingo reported one or two monthes ago that ftrace_printk was not NMI safe
because of this spinlock.

He suggested to drop the spinlock and then make trace_buf per_cpu.

By disabling the irq we prevent from race with maskable irqs. And in
case of racy accesses to trace_buf because of an nmi, then the buffer
might be mixed up but it must be harmless compared to a hardlockup that
can occur now. On the worst case, the trace will be weird and that's it.

Frederic.


> >     Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 8c6a902..4c97947 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -1176,7 +1176,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
> >   */
> >  int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> >  {
> > -	static DEFINE_SPINLOCK(trace_buf_lock);
> > +	static raw_spinlock_t trace_buf_lock =
> > +		(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
> >  	static u32 trace_buf[TRACE_BUF_SIZE];
> >  
> >  	struct ring_buffer_event *event;
> > @@ -1201,7 +1202,9 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> >  	if (unlikely(atomic_read(&data->disabled)))
> >  		goto out;
> >  
> > -	spin_lock_irqsave(&trace_buf_lock, flags);
> > +	/* Lockdep uses trace_printk for lock tracing */
> > +	local_irq_save(flags);
> 
> Shouldn't you also use raw_local_irq_save() and friends?
> 
> > +	__raw_spin_lock(&trace_buf_lock);
> >  	len = vbin_printf(trace_buf, TRACE_BUF_SIZE, fmt, args);
> >  
> >  	if (len > TRACE_BUF_SIZE || len < 0)
> > @@ -1220,7 +1223,8 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> >  	ring_buffer_unlock_commit(tr->buffer, event);
> >  
> >  out_unlock:
> > -	spin_unlock_irqrestore(&trace_buf_lock, flags);
> > +	__raw_spin_unlock(&trace_buf_lock);
> > +	local_irq_restore(flags);
> >  
> >  out:
> >  	ftrace_preempt_enable(resched);
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Steven Rostedt]

On Wed, 11 Mar 2009, Peter Zijlstra wrote:

> On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> 
> > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > Author: Steven Rostedt <srostedt@redhat.com>
> > Date:   Tue Mar 10 17:16:35 2009 -0400
> > 
> >     tracing: use raw spinlocks for trace_vprintk
> >     
> >     Impact: prevent locking up by lockdep tracer
> >     
> >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> >     call back into lockdep without locking up.
> 
> Hmm, I did this when I posted the lockdep tracepoints, so someone then
> did a bad copy/paste job when renaming ftrace_printk or something?
> 
> See efed792d6738964f399a508ef9e831cd60fa4657
> 
> >     Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 8c6a902..4c97947 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -1176,7 +1176,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
> >   */
> >  int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> >  {
> > -	static DEFINE_SPINLOCK(trace_buf_lock);
> > +	static raw_spinlock_t trace_buf_lock =
> > +		(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
> >  	static u32 trace_buf[TRACE_BUF_SIZE];
> >  
> >  	struct ring_buffer_event *event;
> > @@ -1201,7 +1202,9 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> >  	if (unlikely(atomic_read(&data->disabled)))
> >  		goto out;
> >  
> > -	spin_lock_irqsave(&trace_buf_lock, flags);
> > +	/* Lockdep uses trace_printk for lock tracing */
> > +	local_irq_save(flags);
> 
> Shouldn't you also use raw_local_irq_save() and friends?

I found that the raw_local_irq_save() causes a lot more headaches with 
lockdep complaining that irqs are disabled and it did not know about it. 
Those checks seem everywhere, so I only use the raw_local_irq_save when I 
absolutely need to. Here, it works without it.

-- Steve


> 
> > +	__raw_spin_lock(&trace_buf_lock);
> >  	len = vbin_printf(trace_buf, TRACE_BUF_SIZE, fmt, args);
> >  
> >  	if (len > TRACE_BUF_SIZE || len < 0)
> > @@ -1220,7 +1223,8 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> >  	ring_buffer_unlock_commit(tr->buffer, event);
> >  
> >  out_unlock:
> > -	spin_unlock_irqrestore(&trace_buf_lock, flags);
> > +	__raw_spin_unlock(&trace_buf_lock);
> > +	local_irq_restore(flags);
> >  
> >  out:
> >  	ftrace_preempt_enable(resched);
> > 
> 

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Steven Rostedt]

On Wed, 11 Mar 2009, Frederic Weisbecker wrote:

> On Wed, Mar 11, 2009 at 07:59:24AM +0100, Peter Zijlstra wrote:
> > On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> > 
> > > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > > Author: Steven Rostedt <srostedt@redhat.com>
> > > Date:   Tue Mar 10 17:16:35 2009 -0400
> > > 
> > >     tracing: use raw spinlocks for trace_vprintk
> > >     
> > >     Impact: prevent locking up by lockdep tracer
> > >     
> > >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> > >     call back into lockdep without locking up.
> > 
> > Hmm, I did this when I posted the lockdep tracepoints, so someone then
> > did a bad copy/paste job when renaming ftrace_printk or something?
> > 
> > See efed792d6738964f399a508ef9e831cd60fa4657
> 
> 
> 
> Must be my bad :-s
> I think I lost this modification that was done on the old trace_vprintf
> between two iterations of the bprintk patchset.
> 
> BTW, Ingo reported one or two monthes ago that ftrace_printk was not NMI safe
> because of this spinlock.
> 
> He suggested to drop the spinlock and then make trace_buf per_cpu.
> 
> By disabling the irq we prevent from race with maskable irqs. And in
> case of racy accesses to trace_buf because of an nmi, then the buffer
> might be mixed up but it must be harmless compared to a hardlockup that
> can occur now. On the worst case, the trace will be weird and that's it.

But the lock is only used in this function, and the function can not 
recurse. It is NMI safe. see below.

> 
> Frederic.
> 
> 
> > >     Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> > > 
> > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > index 8c6a902..4c97947 100644
> > > --- a/kernel/trace/trace.c
> > > +++ b/kernel/trace/trace.c
> > > @@ -1176,7 +1176,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
> > >   */
> > >  int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> > >  {
> > > -	static DEFINE_SPINLOCK(trace_buf_lock);
> > > +	static raw_spinlock_t trace_buf_lock =
> > > +		(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
> > >  	static u32 trace_buf[TRACE_BUF_SIZE];
> > >  
> > >  	struct ring_buffer_event *event;
> > > @@ -1201,7 +1202,9 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> > >  	if (unlikely(atomic_read(&data->disabled)))
> > >  		goto out;

The above disable is exactly for NMIs. We should have preemption disabled 
here, and we disable this per cpu. If an NMI comes in after this point, it 
will exit the function without taking the lock. If it runs on another CPU, 
we really don't care. That's what NMIs are for ;-)

-- Steve



> > >  
> > > -	spin_lock_irqsave(&trace_buf_lock, flags);
> > > +	/* Lockdep uses trace_printk for lock tracing */
> > > +	local_irq_save(flags);
> > 
> > Shouldn't you also use raw_local_irq_save() and friends?
> > 
> > > +	__raw_spin_lock(&trace_buf_lock);
> > >  	len = vbin_printf(trace_buf, TRACE_BUF_SIZE, fmt, args);
> > >  
> > >  	if (len > TRACE_BUF_SIZE || len < 0)
> > > @@ -1220,7 +1223,8 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> > >  	ring_buffer_unlock_commit(tr->buffer, event);
> > >  
> > >  out_unlock:
> > > -	spin_unlock_irqrestore(&trace_buf_lock, flags);
> > > +	__raw_spin_unlock(&trace_buf_lock);
> > > +	local_irq_restore(flags);
> > >  
> > >  out:
> > >  	ftrace_preempt_enable(resched);
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@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: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Steven Rostedt]

On Wed, 11 Mar 2009, Steven Rostedt wrote:
> 
> The above disable is exactly for NMIs. We should have preemption disabled 
> here, and we disable this per cpu. If an NMI comes in after this point, it 
> will exit the function without taking the lock. If it runs on another CPU, 
> we really don't care. That's what NMIs are for ;-)

I have not had my first cup of coffee this morning yet. I meant to say:

  That's what spinlocks are for ;-)


Bah!

-- Steve

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Frederic Weisbecker]

On Wed, Mar 11, 2009 at 10:00:51AM -0400, Steven Rostedt wrote:
> 
> On Wed, 11 Mar 2009, Frederic Weisbecker wrote:
> 
> > On Wed, Mar 11, 2009 at 07:59:24AM +0100, Peter Zijlstra wrote:
> > > On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> > > 
> > > > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > > > Author: Steven Rostedt <srostedt@redhat.com>
> > > > Date:   Tue Mar 10 17:16:35 2009 -0400
> > > > 
> > > >     tracing: use raw spinlocks for trace_vprintk
> > > >     
> > > >     Impact: prevent locking up by lockdep tracer
> > > >     
> > > >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> > > >     call back into lockdep without locking up.
> > > 
> > > Hmm, I did this when I posted the lockdep tracepoints, so someone then
> > > did a bad copy/paste job when renaming ftrace_printk or something?
> > > 
> > > See efed792d6738964f399a508ef9e831cd60fa4657
> > 
> > 
> > 
> > Must be my bad :-s
> > I think I lost this modification that was done on the old trace_vprintf
> > between two iterations of the bprintk patchset.
> > 
> > BTW, Ingo reported one or two monthes ago that ftrace_printk was not NMI safe
> > because of this spinlock.
> > 
> > He suggested to drop the spinlock and then make trace_buf per_cpu.
> > 
> > By disabling the irq we prevent from race with maskable irqs. And in
> > case of racy accesses to trace_buf because of an nmi, then the buffer
> > might be mixed up but it must be harmless compared to a hardlockup that
> > can occur now. On the worst case, the trace will be weird and that's it.
> 
> But the lock is only used in this function, and the function can not 
> recurse. It is NMI safe. see below.
> 
> > 
> > Frederic.
> > 
> > 
> > > >     Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> > > > 
> > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > > index 8c6a902..4c97947 100644
> > > > --- a/kernel/trace/trace.c
> > > > +++ b/kernel/trace/trace.c
> > > > @@ -1176,7 +1176,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
> > > >   */
> > > >  int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> > > >  {
> > > > -	static DEFINE_SPINLOCK(trace_buf_lock);
> > > > +	static raw_spinlock_t trace_buf_lock =
> > > > +		(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
> > > >  	static u32 trace_buf[TRACE_BUF_SIZE];
> > > >  
> > > >  	struct ring_buffer_event *event;
> > > > @@ -1201,7 +1202,9 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> > > >  	if (unlikely(atomic_read(&data->disabled)))
> > > >  		goto out;
> 
> The above disable is exactly for NMIs. We should have preemption disabled 
> here, and we disable this per cpu. If an NMI comes in after this point, it 
> will exit the function without taking the lock. If it runs on another CPU, 
> we really don't care. That's what NMIs are for ;-)


Aaah, ok :-)

 
> -- Steve
> 
> 
> 
> > > >  
> > > > -	spin_lock_irqsave(&trace_buf_lock, flags);
> > > > +	/* Lockdep uses trace_printk for lock tracing */
> > > > +	local_irq_save(flags);
> > > 
> > > Shouldn't you also use raw_local_irq_save() and friends?
> > > 
> > > > +	__raw_spin_lock(&trace_buf_lock);
> > > >  	len = vbin_printf(trace_buf, TRACE_BUF_SIZE, fmt, args);
> > > >  
> > > >  	if (len > TRACE_BUF_SIZE || len < 0)
> > > > @@ -1220,7 +1223,8 @@ int trace_vprintk(unsigned long ip, int depth, const char *fmt, va_list args)
> > > >  	ring_buffer_unlock_commit(tr->buffer, event);
> > > >  
> > > >  out_unlock:
> > > > -	spin_unlock_irqrestore(&trace_buf_lock, flags);
> > > > +	__raw_spin_unlock(&trace_buf_lock);
> > > > +	local_irq_restore(flags);
> > > >  
> > > >  out:
> > > >  	ftrace_preempt_enable(resched);
> > > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@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: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> 
> > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > Author: Steven Rostedt <srostedt@redhat.com>
> > Date:   Tue Mar 10 17:16:35 2009 -0400
> > 
> >     tracing: use raw spinlocks for trace_vprintk
> >     
> >     Impact: prevent locking up by lockdep tracer
> >     
> >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> >     call back into lockdep without locking up.
> 
> Hmm, I did this when I posted the lockdep tracepoints, so someone then
> did a bad copy/paste job when renaming ftrace_printk or something?
> 
> See efed792d6738964f399a508ef9e831cd60fa4657

What's the conclusion in this thread? I'm holding the pull 
until there's agreement.

	Ingo

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Steven Rostedt]

On Wed, 11 Mar 2009, Ingo Molnar wrote:

> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> > 
> > > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > > Author: Steven Rostedt <srostedt@redhat.com>
> > > Date:   Tue Mar 10 17:16:35 2009 -0400
> > > 
> > >     tracing: use raw spinlocks for trace_vprintk
> > >     
> > >     Impact: prevent locking up by lockdep tracer
> > >     
> > >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> > >     call back into lockdep without locking up.
> > 
> > Hmm, I did this when I posted the lockdep tracepoints, so someone then
> > did a bad copy/paste job when renaming ftrace_printk or something?
> > 
> > See efed792d6738964f399a508ef9e831cd60fa4657
> 
> What's the conclusion in this thread? I'm holding the pull 
> until there's agreement.

I believe the conclusion is that Peter's changes got removed when Frederic 
removed the old printk version. But his new version did not have the 
changes.

My changes are basically the same as Peter's except that I did not do 
anything with the local_irq_save, since I do not think those are needed.

Peter,

Are you fine with the change? I guess it's up to you now.

-- Steve

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Peter Zijlstra]

On Wed, 2009-03-11 at 14:32 -0400, Steven Rostedt wrote:
> On Wed, 11 Mar 2009, Ingo Molnar wrote:
> 
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> > > 
> > > > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > > > Author: Steven Rostedt <srostedt@redhat.com>
> > > > Date:   Tue Mar 10 17:16:35 2009 -0400
> > > > 
> > > >     tracing: use raw spinlocks for trace_vprintk
> > > >     
> > > >     Impact: prevent locking up by lockdep tracer
> > > >     
> > > >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> > > >     call back into lockdep without locking up.
> > > 
> > > Hmm, I did this when I posted the lockdep tracepoints, so someone then
> > > did a bad copy/paste job when renaming ftrace_printk or something?
> > > 
> > > See efed792d6738964f399a508ef9e831cd60fa4657
> > 
> > What's the conclusion in this thread? I'm holding the pull 
> > until there's agreement.
> 
> I believe the conclusion is that Peter's changes got removed when Frederic 
> removed the old printk version. But his new version did not have the 
> changes.
> 
> My changes are basically the same as Peter's except that I did not do 
> anything with the local_irq_save, since I do not think those are needed.
> 
> Peter,
> 
> Are you fine with the change? I guess it's up to you now.

Yeah, although I'd rather see the raw_local_irq_save() there too, but
the patch as it stands solves the problem.

Re: [GIT PULL] tracing: use raw spinlocks for trace_vprintk

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, 2009-03-11 at 14:32 -0400, Steven Rostedt wrote:
> > On Wed, 11 Mar 2009, Ingo Molnar wrote:
> > 
> > > 
> > > * Peter Zijlstra <peterz@infradead.org> wrote:
> > > 
> > > > On Tue, 2009-03-10 at 21:26 -0400, Steven Rostedt wrote:
> > > > 
> > > > > commit 80370cb758e7ca2692cd9fb5e413d970b1f4b2b2
> > > > > Author: Steven Rostedt <srostedt@redhat.com>
> > > > > Date:   Tue Mar 10 17:16:35 2009 -0400
> > > > > 
> > > > >     tracing: use raw spinlocks for trace_vprintk
> > > > >     
> > > > >     Impact: prevent locking up by lockdep tracer
> > > > >     
> > > > >     The lockdep tracer uses trace_vprintk and thus trace_vprintk can not
> > > > >     call back into lockdep without locking up.
> > > > 
> > > > Hmm, I did this when I posted the lockdep tracepoints, so someone then
> > > > did a bad copy/paste job when renaming ftrace_printk or something?
> > > > 
> > > > See efed792d6738964f399a508ef9e831cd60fa4657
> > > 
> > > What's the conclusion in this thread? I'm holding the pull 
> > > until there's agreement.
> > 
> > I believe the conclusion is that Peter's changes got removed when Frederic 
> > removed the old printk version. But his new version did not have the 
> > changes.
> > 
> > My changes are basically the same as Peter's except that I did not do 
> > anything with the local_irq_save, since I do not think those are needed.
> > 
> > Peter,
> > 
> > Are you fine with the change? I guess it's up to you now.
> 
> Yeah, although I'd rather see the raw_local_irq_save() there too, but
> the patch as it stands solves the problem.

Pulled - thanks guys!

	Ingo