Re: [PATCH v9 13/22] tracing: Add hist trigger support for clearing a trace

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

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 ?

Thanks,


> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.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)
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com