[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1446602020.17120.168.camel@tzanussi-mobl.amr.corp.intel.com>
Date: Tue, 03 Nov 2015 19:53:40 -0600
From: Tom Zanussi <tom.zanussi@...ux.intel.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: rostedt@...dmis.org, daniel.wagner@...-carit.de,
masami.hiramatsu.pt@...achi.com, josh@...htriplett.org,
andi@...stfloor.org, mathieu.desnoyers@...icios.com,
peterz@...radead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing
and continuing a trace
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@...ux.intel.com>
> > Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.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@...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