lkml.org 
[lkml]   [2015]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v9 12/22] tracing: Add hist trigger support for pausing and continuing a trace
From
Date
Hi Masami,

On Wed, 2015-07-22 at 17:20 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> On 2015/07/17 2:22, 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
>
> Since the only one hist trigger can be set on one event, it seems
> that we don't need keys for pause/cont/clear (e.g. hist:pause is enough).
> Anyway, I've found an odd behavior.

Right, because currently there is only one hist trigger per event, the
key is ignored, which also accounts for the 'odd' behavior.

But rather than saying it's expected and document it, I think the
conclusion from the other comments are that we'll be allowing multiple
hist triggers per event, and in that case, they need to be uniquely
identifiable by both key and filter.

>
> [root@localhost tracing]# echo 'hist:keys=parent_pid' > events/sched/sched_process_fork/trigger
> [root@localhost tracing]# echo 'hist:keys=common_pid:pause' > events/sched/sched_process_fork/trigger
> [root@localhost tracing]# cat events/sched/sched_process_fork/trigger
> hist:keys=parent_pid:vals=hitcount:sort=hitcount:size=2048 [paused]
>
> So, the second "pause" command can work with different keys.
> Moreover, I can remove it with different keys.
>

Right, this goes away once we have code that deals with multiple
histograms per event, which I'll go ahead and implement rather than
document the confusion...

Tom

> [root@localhost tracing]# echo '!hist:keys=child_pid' > events/sched/sched_process_fork/trigger
> [root@localhost tracing]# cat events/sched/sched_process_fork/trigger
> # Available triggers:
> # traceon traceoff snapshot stacktrace enable_event disable_event enable_hist disable_hist hist
>
> Thank you,
>
> >
> > Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> > ---
> > kernel/trace/trace.c | 5 +++++
> > kernel/trace/trace_events_hist.c | 26 +++++++++++++++++++++++---
> > 2 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 5dd1fc4..547bbc8 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -3791,6 +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 [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"
> > @@ -3821,6 +3822,10 @@ static const char readme_msg[] =
> > "\t on. The default if unspecified is 'hitcount' and the.\n"
> > "\t default sort order is 'ascending'. To sort in the opposite\n"
> > "\t direction, append .descending' to the sort key.\n\n"
> > + "\t The 'pause' param 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 6bf224f..3ae58e7 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;
> >
>
>




\
 
 \ /
  Last update: 2015-07-22 22:41    [W:0.190 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site