Messages in this thread | | | Subject | Re: [PATCH 3/3] trace-cmd: setting plugin to 'nop' clears data before its recorded | From | Steven Rostedt <> | Date | Fri, 06 Apr 2012 08:24:53 -0400 |
| |
On Fri, 2012-04-06 at 08:06 -0400, Mark Asselstine wrote:
> Works for me. I see you make use of gboolean elsewhere so I assume you prefer > the use of this rather than using int?
Sure.
> > > Also, and this has nothing to do with you or your patches, I simply hate > > the fact that I called these plugins. They should have been called > > "tracers" but I think I'm stuck with it. The term now is ambiguous as it > > means both a tracer (as it does here) and it means actual plugins that > > you can load into trace-cmd itself. > > > > To stop the proliferation do you want me to name the bool > 'disable_current_tracer' rather than 'disable_plugin' or 'plugin'?
disable_tracer would due.
> > > > > { > > > > > > disable_tracing(); > > > > > > - set_plugin("nop"); > > > > > > reset_events(); > > > > > > /* Force close and reset of ftrace pid file */ > > > > > > @@ -911,6 +910,12 @@ static void disable_all(void) > > > > > > clear_trace(); > > > > > > } > > > > > > +static void disable_all(void) > > > +{ > > > + disable_all_but_plugin(); > > > + set_plugin("nop"); > > > +} > > > + > > > > > > static void > > > update_sched_event(struct event_list **event, const char *file, > > > > > > const char *pid_filter, const char *field_filter) > > > > > > @@ -2227,7 +2232,7 @@ void trace_record (int argc, char **argv) > > > > > > } > > > > > > if (!keep) > > > > > > - disable_all(); > > > + disable_all_but_plugin(); > > > > > > printf("Kernel buffer statistics:\n" > > > > > > " Note: \"entries\" are the entries left in the > > > kernel ring buffer and are not\n"> > > > @@ -2245,6 +2250,8 @@ void trace_record (int argc, char **argv) > > > > > > record_data(date2ts); > > > delete_thread_data(); > > > > > > + if (!keep) > > > + set_plugin("nop"); > > > > > > if (keep) > > > > > > exit(0); > > > > I think the above would have been better if you did: > > > > if (keep) > > exit(0); > > else > > set_plugin(nop); > > > > -- Steve > > Wow, I wasn't being too bright there was I? Agreed and the 'else' isn't > necessary either, so this is a good cleanup. > > I will get this fixed up later today. Do you want a V2 of the full patch series > or just an update of this one patch?
You can just update this patch with a v2 on it. I'm currently traveling home from a conference and wont be back to work till Tuesday. Please, take your time.
Thanks!
-- Steve
| |