Groups > linux.kernel > #1261307 > unrolled thread

Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing and continuing a trace

Started by	Namhyung Kim <namhyung@kernel.org>
First post	2015-11-03 09:50 +0100
Last post	2015-11-04 03:00 +0100
Articles	2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

  Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing  and continuing a trace Namhyung Kim <namhyung@kernel.org> - 2015-11-03 09:50 +0100
    Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing  and continuing a trace Tom Zanussi <tom.zanussi@linux.intel.com> - 2015-11-04 03:00 +0100

#1261307 — Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing and continuing a trace

From	Namhyung Kim <namhyung@kernel.org>
Date	2015-11-03 09:50 +0100
Subject	Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing and continuing a trace
Message-ID	<qqFrY-jV-5@gated-at.bofh.it>

On Thu, Oct 22, 2015 at 01:14:17PM -0500, Tom Zanussi wrote:
> Allow users to append 'pause' or 'continue' to an existing trigger in
> order to have it paused or to have a paused trace continue.
> 
> This expands the hist trigger syntax from this:
>     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending \
>           [ if filter] > event/trigger
> 
> to this:
> 
>     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause or cont \
>           [ if filter] > event/trigger

What if 'cont' is used for a non-existing hist?  I think it should
fail with -ENOENT but it seems that it'd create a new hist trigger..

Thanks,
Namhyung


> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  kernel/trace/trace.c             |  7 ++++++-
>  kernel/trace/trace_events_hist.c | 26 +++++++++++++++++++++++---
>  2 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index efa8d05..a223959 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3802,6 +3802,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            [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, and the value of a\n"
> @@ -3816,7 +3817,11 @@ static const char readme_msg[] =
>  	"\t    used to specify more or fewer than the default 2048 entries\n"
>  	"\t    for the hashtable size.\n\n"
>  	"\t    Reading the 'hist' file for the event will dump the hash\n"
> -	"\t    table in its entirety to stdout."
> +	"\t    table in its entirety to stdout.\n\n"
> +	"\t    The 'pause' parameter can be used to pause an existing hist\n"
> +	"\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"
>  #endif
>  ;
>  
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 4fc3136..8753c3b 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -78,6 +78,8 @@ struct hist_trigger_attrs {
>  	char		*keys_str;
>  	char		*vals_str;
>  	char		*sort_key_str;
> +	bool		pause;
> +	bool		cont;
>  	unsigned int	map_bits;
>  };
>  
> @@ -184,6 +186,11 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
>  			attrs->vals_str = kstrdup(str, GFP_KERNEL);
>  		else if (!strncmp(str, "sort", strlen("sort")))
>  			attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
> +		else if (!strncmp(str, "pause", strlen("pause")))
> +			attrs->pause = true;
> +		else if (!strncmp(str, "continue", strlen("continue")) ||
> +			 !strncmp(str, "cont", strlen("cont")))
> +			attrs->cont = true;
>  		else if (!strncmp(str, "size", strlen("size"))) {
>  			int map_bits = parse_map_size(str);
>  
> @@ -843,7 +850,10 @@ static int event_hist_trigger_print(struct seq_file *m,
>  	if (data->filter_str)
>  		seq_printf(m, " if %s", data->filter_str);
>  
> -	seq_puts(m, " [active]");
> +	if (data->paused)
> +		seq_puts(m, " [paused]");
> +	else
> +		seq_puts(m, " [active]");
>  
>  	seq_putc(m, '\n');
>  
> @@ -882,16 +892,25 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
>  				 struct event_trigger_data *data,
>  				 struct trace_event_file *file)
>  {
> +	struct hist_trigger_data *hist_data = data->private_data;
>  	struct event_trigger_data *test;
>  	int ret = 0;
>  
>  	list_for_each_entry_rcu(test, &file->triggers, list) {
>  		if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
> -			ret = -EEXIST;
> +			if (hist_data->attrs->pause)
> +				test->paused = true;
> +			else if (hist_data->attrs->cont)
> +				test->paused = false;
> +			else
> +				ret = -EEXIST;
>  			goto out;
>  		}
>  	}
>  
> +	if (hist_data->attrs->pause)
> +		data->paused = true;
> +
>  	if (data->ops->init) {
>  		ret = data->ops->init(data->ops, data);
>  		if (ret < 0)
> @@ -984,7 +1003,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
>  	 * triggers registered a failure too.
>  	 */
>  	if (!ret) {
> -		ret = -ENOENT;
> +		if (!(attrs->pause || attrs->cont))
> +			ret = -ENOENT;
>  		goto out_free;
>  	} else if (ret < 0)
>  		goto out_free;
> -- 
> 1.9.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1262029

From	Tom Zanussi <tom.zanussi@linux.intel.com>
Date	2015-11-04 03:00 +0100
Message-ID	<qqVwK-2hf-13@gated-at.bofh.it>
In reply to	#1261307

On Tue, 2015-11-03 at 17:38 +0900, Namhyung Kim wrote:
> On Thu, Oct 22, 2015 at 01:14:17PM -0500, Tom Zanussi wrote:
> > Allow users to append 'pause' or 'continue' to an existing trigger in
> > order to have it paused or to have a paused trace continue.
> > 
> > This expands the hist trigger syntax from this:
> >     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending \
> >           [ if filter] > event/trigger
> > 
> > to this:
> > 
> >     # echo hist:keys=xxx:vals=yyy:sort=zzz.descending:pause or cont \
> >           [ if filter] > event/trigger
> 
> What if 'cont' is used for a non-existing hist?  I think it should
> fail with -ENOENT but it seems that it'd create a new hist trigger..
> 

You're right - 'pause' is the only one that should create a new trigger,
not 'cont' or 'clear'.

Thanks,

Tom

> Thanks,
> Namhyung
> 
> 
> > 
> > Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> > Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > ---
> >  kernel/trace/trace.c             |  7 ++++++-
> >  kernel/trace/trace_events_hist.c | 26 +++++++++++++++++++++++---
> >  2 files changed, 29 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index efa8d05..a223959 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3802,6 +3802,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            [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, and the value of a\n"
> > @@ -3816,7 +3817,11 @@ static const char readme_msg[] =
> >  	"\t    used to specify more or fewer than the default 2048 entries\n"
> >  	"\t    for the hashtable size.\n\n"
> >  	"\t    Reading the 'hist' file for the event will dump the hash\n"
> > -	"\t    table in its entirety to stdout."
> > +	"\t    table in its entirety to stdout.\n\n"
> > +	"\t    The 'pause' parameter can be used to pause an existing hist\n"
> > +	"\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"
> >  #endif
> >  ;
> >  
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index 4fc3136..8753c3b 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -78,6 +78,8 @@ struct hist_trigger_attrs {
> >  	char		*keys_str;
> >  	char		*vals_str;
> >  	char		*sort_key_str;
> > +	bool		pause;
> > +	bool		cont;
> >  	unsigned int	map_bits;
> >  };
> >  
> > @@ -184,6 +186,11 @@ static struct hist_trigger_attrs *parse_hist_trigger_attrs(char *trigger_str)
> >  			attrs->vals_str = kstrdup(str, GFP_KERNEL);
> >  		else if (!strncmp(str, "sort", strlen("sort")))
> >  			attrs->sort_key_str = kstrdup(str, GFP_KERNEL);
> > +		else if (!strncmp(str, "pause", strlen("pause")))
> > +			attrs->pause = true;
> > +		else if (!strncmp(str, "continue", strlen("continue")) ||
> > +			 !strncmp(str, "cont", strlen("cont")))
> > +			attrs->cont = true;
> >  		else if (!strncmp(str, "size", strlen("size"))) {
> >  			int map_bits = parse_map_size(str);
> >  
> > @@ -843,7 +850,10 @@ static int event_hist_trigger_print(struct seq_file *m,
> >  	if (data->filter_str)
> >  		seq_printf(m, " if %s", data->filter_str);
> >  
> > -	seq_puts(m, " [active]");
> > +	if (data->paused)
> > +		seq_puts(m, " [paused]");
> > +	else
> > +		seq_puts(m, " [active]");
> >  
> >  	seq_putc(m, '\n');
> >  
> > @@ -882,16 +892,25 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
> >  				 struct event_trigger_data *data,
> >  				 struct trace_event_file *file)
> >  {
> > +	struct hist_trigger_data *hist_data = data->private_data;
> >  	struct event_trigger_data *test;
> >  	int ret = 0;
> >  
> >  	list_for_each_entry_rcu(test, &file->triggers, list) {
> >  		if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
> > -			ret = -EEXIST;
> > +			if (hist_data->attrs->pause)
> > +				test->paused = true;
> > +			else if (hist_data->attrs->cont)
> > +				test->paused = false;
> > +			else
> > +				ret = -EEXIST;
> >  			goto out;
> >  		}
> >  	}
> >  
> > +	if (hist_data->attrs->pause)
> > +		data->paused = true;
> > +
> >  	if (data->ops->init) {
> >  		ret = data->ops->init(data->ops, data);
> >  		if (ret < 0)
> > @@ -984,7 +1003,8 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
> >  	 * triggers registered a failure too.
> >  	 */
> >  	if (!ret) {
> > -		ret = -ENOENT;
> > +		if (!(attrs->pause || attrs->cont))
> > +			ret = -ENOENT;
> >  		goto out_free;
> >  	} else if (ret < 0)
> >  		goto out_free;
> > -- 
> > 1.9.3
> > 


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

[toc] | [prev] | [standalone]

csiph-web

Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing and continuing a trace

Contents

#1261307 — Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing and continuing a trace

#1262029