Re: [PATCHv2] tracing - avoid soft lockup in trace_pipe

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 2011-03-21 at 13:10 +0100, Jiri Olsa wrote:
> On Mon, Mar 14, 2011 at 07:06:21PM +0100, Jiri Olsa wrote:
> > On Mon, Mar 14, 2011 at 11:20:57AM -0400, Steven Rostedt wrote:
> > > On Mon, 2011-03-14 at 16:07 +0100, Jiri Olsa wrote:
> > > > So we get back the the while loop with no data added and with
> > > > TRACE_TYPE_HANDLED return code, which will keep us in the loop.
> > > > 
> > > > Not sure this is bug or feature :)
> > > > 
> > > > 
> > > Most bugs are features ;)
> > > 
> > > honestly, I'd love to rip out all the "bin", "hex" etc formats as they
> > > are pretty much deprecated.
> > > 
> > > But as you pointed out that this can keep us in the inner loop, we need
> > > to fix that. I prefer to fix it within the nop print function. Perhaps
> > > we need to fill in something. "type: %d", iter->ent->type ?
> > 
> > sounds ok :) attaching v2 patch
> hi, any feedback?

Ah, sorry, as this was a reply to the original thread, it didn't pop up
immediately.

> > 
> > 
> > ---
> > running following commands:
> > 
> >   # enable the binary option
> >   echo 1 > ./options/bin
> >   # disable context info option
> >   echo 0 > ./options/context-info 
> >   # tracing only events
> >   echo 1 > ./events/enable
> >   cat trace_pipe
> > 
> > is causing lockup (in NON preemptive kernels) inside
> > tracing_read_pipe function.
> > 
> > The reason are:
> >  - bin/hex/raw output functions for events are set to
> >    trace_nop_print function, which prints nothing and
> >    returns TRACE_TYPE_HANDLED value
> >  - LOST EVENT trace do not handle trace_seq overflow
> > 
> > These reasons force the while loop in tracing_read_pipe
> > function never to break.
> > 
> > The attached patch fixies handling of lost event trace, and
> > changes trace_nop_print to print minimal info, which is needed
> > for the correct tracing_read_pipe processing.
> > 
> > v2 changes:
> >  - omit the cond_resched changes by trace_nop_print changes
> >  - WARN changed to WARN_ONCE and added info to be able
> >    to find out the culprit

So this replaces both patches, correct?

Thanks,

-- Steve