Re: [GIT PULL] ring-buffer: Replace this_cpu_*() with __this_cpu_*()

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 19 Mar 2015 18:34:44 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 19 Mar 2015 15:16:25 -0700
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > So I don't think the ring-buffer change is necessarily _wrong_, but if
> > this is a performance issue, why don't we just fix it up for the
> > generic case rather than for just one user?
> 
> I totally agree with your analysis, but it's up to Christoph to come up
> with an answer to your questions.
> 

I will add that the ring buffer issue is not just a performance
problem. It is a correctness problem. The generic
preempt_disable/enable() functions can be traced by the function
tracer, where as the preempt_disable/enable_notrace() versions are not.

As tracing is very invasive, and can cause unnecessary recursions,
there are protection mechanisms to prevent something like that
happening. The issue that this patch addresses is that the recursion
protection is the code that happens to be causing the recursion!

 some_function()
   function_tracer()
     ring_buffer_reserve()
       trace_recursive_lock()
         this_cpu_read()
           preempt_disable()
             function_tracer()
               ring_buffer_reserve()
                 trace_recursion_lock()
                    (etc)

The reason this did not happen is that the function_tracer() also has
its own recursion protection that uses current->trace_recursion to
prevent that from happening. But if there was some function tracing
that did not check recursion and calls into the ring buffer, that could
crash the system.

-- Steve