From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932785AbbKDBxp (ORCPT ); Tue, 3 Nov 2015 20:53:45 -0500 Received: from mga02.intel.com ([134.134.136.20]:25870 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752355AbbKDBxn (ORCPT ); Tue, 3 Nov 2015 20:53:43 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,240,1444719600"; d="scan'208";a="842546287" Message-ID: <1446602020.17120.168.camel@tzanussi-mobl.amr.corp.intel.com> Subject: Re: [PATCH v11 13/28] tracing: Add hist trigger support for pausing and continuing a trace From: Tom Zanussi To: Namhyung Kim Cc: rostedt@goodmis.org, daniel.wagner@bmw-carit.de, masami.hiramatsu.pt@hitachi.com, josh@joshtriplett.org, andi@firstfloor.org, mathieu.desnoyers@efficios.com, peterz@infradead.org, linux-kernel@vger.kernel.org Date: Tue, 03 Nov 2015 19:53:40 -0600 In-Reply-To: <20151103083849.GA20951@danjae.kornet> References: <508fbc262792510572b3c596ccafcd9c14900241.1445530672.git.tom.zanussi@linux.intel.com> <20151103083849.GA20951@danjae.kornet> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > Tested-by: Masami Hiramatsu > > --- > > 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= > "\t [:sort=field1,field2,...]\n" > > "\t [:size=#entries]\n" > > + "\t [:pause][:continue]\n" > > "\t [if ]\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 > >