Date: Wed, 9 Mar 2016 08:52:50 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: Chunyu Hu <chuhu@redhat.com>
Cc: liwan@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] tracing: make tracer_flags use the right set_flag
 callback
Message-ID: <20160309085250.6dcba5e9@gandalf.local.home>
In-Reply-To: <304748287.27041685.1457530725205.JavaMail.zimbra@redhat.com>
References: <1457444222-8654-1-git-send-email-chuhu@redhat.com>
	<20160308095443.409de468@gandalf.local.home>
	<20160308101451.69f9bc18@gandalf.local.home>
	<587988151.26798768.1457452530893.JavaMail.zimbra@redhat.com>
	<20160308112312.2fdc0dd2@gandalf.local.home>
	<304748287.27041685.1457530725205.JavaMail.zimbra@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3556
Lines: 103

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

> ----- Original Message -----
> > From: "Steven Rostedt" <rostedt@goodmis.org>
> > To: "Chunyu Hu" <chuhu@redhat.com>
> > Cc: liwan@redhat.com, linux-kernel@vger.kernel.org
> > 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@redhat.com> 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
> > 
> >   
>