lkml.org 
[lkml]   [2012]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/3] trace-cmd: setting plugin to 'nop' clears data before its recorded
From
Date
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




\
 
 \ /
  Last update: 2012-04-06 14:27    [W:0.047 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site