lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1437596699.22524.90.camel@tzanussi-mobl.amr.corp.intel.com>
Date:	Wed, 22 Jul 2015 15:24:59 -0500
From:	Tom Zanussi <tom.zanussi@...ux.intel.com>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc:	rostedt@...dmis.org, daniel.wagner@...-carit.de,
	namhyung@...nel.org, josh@...htriplett.org, andi@...stfloor.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 13/22] tracing: Add hist trigger support for clearing
 a trace

On Wed, 2015-07-22 at 22:50 +0900, Masami Hiramatsu wrote:
> On 2015/07/17 2:22, Tom Zanussi wrote:
> > Allow users to append 'clear' to an existing trigger in order to have
> > the hash table cleared.
> > 
> > This expands the hist trigger syntax from this:
> >     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont \
> >            [ if filter] > event/trigger
> > 
> > to this:
> > 
> >     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause/cont/clear \
> >           [ if filter] > event/trigger
> 
> By the way, since pause/cont/clear is not the trigger but the commands
> (which executed immediately), I think it should be a write fops of
> "hist" special file.
> 
> e.g. to clear the histogram, write 0 to hist
> 
> # echo 0 > event/hist
> 
> And pause/cont will be 1 and 2.
> 
> # echo 1 > event/hist  <- pause
> and
> # echo 2 > event/hist  <- continue
> 
> What would you think ?
> 

I do think it makes sense, and like the idea.  But if we allow multiple
histograms per event, does it still work?  (I mean in that case, we'd
like to pause/continue/clear them individually, right?)

Tom

> Thanks,
> 
> 
> > 
> > Signed-off-by: Tom Zanussi <tom.zanussi@...ux.intel.com>
> > ---
> >  kernel/trace/trace.c             |  4 +++-
> >  kernel/trace/trace_events_hist.c | 25 ++++++++++++++++++++++++-
> >  2 files changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 547bbc8..27daa28 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3791,7 +3791,7 @@ static const char readme_msg[] =
> >  	"\t            [:values=<field1[,field2,...]]\n"
> >  	"\t            [:sort=field1,field2,...]\n"
> >  	"\t            [:size=#entries]\n"
> > -	"\t            [:pause][:continue]\n"
> > +	"\t            [:pause][:continue][:clear]\n"
> >  	"\t            [if <filter>]\n\n"
> >  	"\t    When a matching event is hit, an entry is added to a hash\n"
> >  	"\t    table using the key(s) and value(s) named.  Keys and values\n"
> > @@ -3826,6 +3826,8 @@ static const char readme_msg[] =
> >  	"\t    trigger or to start a hist trigger but not log any events\n"
> >  	"\t    until told to do so.  'continue' can be used to start or\n"
> >  	"\t    restart a paused hist trigger.\n\n"
> > +	"\t    The 'clear' param will clear the contents of a running hist\n"
> > +	"\t    trigger and leave its current paused/active state.\n\n"
> >  #endif
> >  ;
> >  
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index 3ae58e7..d8259fe 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -80,6 +80,7 @@ struct hist_trigger_attrs {
> >  	char		*sort_key_str;
> >  	bool		pause;
> >  	bool		cont;
> > +	bool		clear;
> >  	unsigned int	map_bits;
> >  };
> >  
> > @@ -188,6 +189,8 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
> >  			attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
> >  		else if (!strncmp(str, "pause", strlen("pause")))
> >  			attrs->pause = true;
> > +		else if (!strncmp(str, "clear", strlen("clear")))
> > +			attrs->clear = true;
> >  		else if (!strncmp(str, "continue", strlen("continue")) ||
> >  			 !strncmp(str, "cont", strlen("cont")))
> >  			attrs->cont = true;
> > @@ -888,6 +891,24 @@ static struct event_trigger_ops *event_hist_get_trigger_ops(char *cmd,
> >  	return &event_hist_trigger_ops;
> >  }
> >  
> > +static void hist_clear(struct event_trigger_data *data)
> > +{
> > +	struct hist_trigger_data *hist_data = data->private_data;
> > +	bool paused;
> > +
> > +	paused = data->paused;
> > +	data->paused = true;
> > +
> > +	synchronize_sched();
> > +
> > +	tracing_map_clear(hist_data->map);
> > +
> > +	atomic64_set(&hist_data->total_hits, 0);
> > +	atomic64_set(&hist_data->drops, 0);
> > +
> > +	data->paused = paused;
> > +}
> > +
> >  static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
> >  				 struct event_trigger_data *data,
> >  				 struct trace_event_file *file)
> > @@ -902,6 +923,8 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
> >  				test->paused = true;
> >  			else if (hist_data->attrs->cont)
> >  				test->paused = false;
> > +			else if (hist_data->attrs->clear)
> > +				hist_clear(test);
> >  			else
> >  				ret = -EEXIST;
> >  			goto out;
> > @@ -1003,7 +1026,7 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
> >  	 * triggers registered a failure too.
> >  	 */
> >  	if (!ret) {
> > -		if (!(attrs->pause || attrs->cont))
> > +		if (!(attrs->pause || attrs->cont || attrs->clear))
> >  			ret = -ENOENT;
> >  		goto out_free;
> >  	} else if (ret < 0)
> > 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ