From mboxrd@z Thu Jan  1 00:00:00 1970
From: mathieu.desnoyers@polymtl.ca (Mathieu Desnoyers)
Date: Thu, 2 Apr 2009 00:51:04 -0400
Subject: [ltt-dev] Ftrace code in the 2.6.29 kernel
Message-ID: <20090402045104.GA21896@Krystal>

Hi Steven,

I am giving a look at the ftrace code, and I am a bit confused by the
way you handle reentrancy in ring_buffer.c. (this is the code in 2.6.29)
Please tell me if I missed important details :

1) you seem to have removed any sort of "nesting" check to allow NMI
handlers to run. Previously, I remember that you simply discarded the
event if a NMI handler appeared to run over the ring buffer code.

2) Assuming 1) is true, then __rb_reserve_next() called from
ring_buffer_lock_reserve() is protected by :

               local_irq_save(flags);
                __raw_spin_lock(&cpu_buffer->lock);

Which I think is the last thing you want to see in a NMI handler. It
sounds like this code is begging for a deadlock to occur if run in NMI
context. Or maybe you don't claim that this code supports NMI, but then
you should remove the following comment from ring_buffer.c :

rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
{
        /*
         * We only race with interrupts and NMIs on this CPU.

So basically, if an NMI nests over that code, or if an instrumented
fault happens within the ring_buffer code, this would generate an
infinite recursive call chain of trap/tracing/trap/tracing...

So this is why I think I might have missed a sanity check somewhere.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68