Re: [PATCH 1/2] tracing: make tracer_flags use the right set_flag callback

[Steven Rostedt]

On Wed, 9 Mar 2016 08:38:45 -0500 (EST)
Chunyu Hu <chuhu@xxxxxxxxxx> wrote:

> ----- Original Message -----
> > From: "Steven Rostedt" <rostedt@xxxxxxxxxxx>
> > To: "Chunyu Hu" <chuhu@xxxxxxxxxx>
> > Cc: liwan@xxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> > Sent: Wednesday, March 9, 2016 12:23:12 AM
> > Subject: Re: [PATCH 1/2] tracing: make tracer_flags use the right set_flag callback
> > 
> > On Tue, 8 Mar 2016 10:55:30 -0500 (EST)
> > Chunyu Hu <chuhu@xxxxxxxxxx> wrote:
> > 
> >   
> > > Sure! But my patch/build work are all manual, so I guess I can't keep with
> > > your speed, but i will do. Thx.  
> > 
> > Here's the latest version of your patch ;-)  
> 
> Thanks. It's amazing, all the call trace  in ftrace.c are gone. I did
> the ltp ftrace stress tests and some regression tests on it. I didn't hit 
> any call trace. Thanks for the modification of the code. Just hit several
> dmesg like: kernel: failed to start wakeup tracer. 
> 
>  
> BTW, may I ask the question that what's the difference of func_stack_trace
> and stacktrace? And Is there a case that the tracing_on can't be setup? I
> haven't touch many parts of the code. Thanks ;)


stacktrace works with events, func_stack_trace works with the function
tracer. They are separate for two reasons. One, they have completely
different infrastructures. Two, function tracing is far more invasive,
and enabling func_stack_trace without function filtering will pretty
much bring your system to a screaming halt.

I'm not sure what you mean about the tracing_on question. The
tracing_on file only connects to the tracing_on functionality in the
kernel, which when clear will stop the ring buffers from recording data.

-- Steve

> >  	for (i = 0; i < tr->nr_topts; i++) {
> > -		/*
> > -		 * Check if these flags have already been added.
> > -		 * Some tracers share flags.
> > -		 */
> > -		if (tr->topts[i].tracer->flags == tracer->flags)
> > +		/* Make sure there's no duplicate flags. */
> > +		if (WARN_ON_ONCE(tr->topts[i].tracer->flags == tracer->flags))  
> 
> Thanks! It's really a safer way to throw out this warn if there is duplicate.
> 
> >  			return;
> >  	}
> >  
> > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > index 8414fa40bf27..b4cae47f283e 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -345,6 +345,7 @@ struct tracer_opt {
> >  struct tracer_flags {
> >  	u32			val;
> >  	struct tracer_opt	*opts;
> > +	struct tracer		*trace;
> >  };
> >  
> >  /* Makes more easy to define a tracer opt */
> > diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> > index fcd41a166405..5a095c2e4b69 100644
> > --- a/kernel/trace/trace_functions.c
> > +++ b/kernel/trace/trace_functions.c
> > @@ -219,6 +219,8 @@ static void tracing_stop_function_trace(struct
> > trace_array *tr)
> >  	unregister_ftrace_function(tr->ops);
> >  }
> >  
> > +static struct tracer function_trace;
> > +
> >  static int
> >  func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
> >  {
> > @@ -228,6 +230,10 @@ func_set_flag(struct trace_array *tr, u32 old_flags, u32
> > bit, int set)
> >  		if (!!set == !!(func_flags.val & TRACE_FUNC_OPT_STACK))
> >  			break;
> >  
> > +		/* We can change this flag when not running. */
> > +		if (tr->current_trace != &function_trace)
> > +			break;
> > +  
> 
> At first I thought it should have some relation with the funcgraph trace. so i 
> guessed was wrong. Thx.
> 
> >  		unregister_ftrace_function(tr->ops);
> >  
> >  		if (set) {
> > --
> > 1.8.3.1
> > 
> >   
>