Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753048AbbGVUZD (ORCPT ); Wed, 22 Jul 2015 16:25:03 -0400 Received: from mga02.intel.com ([134.134.136.20]:34710 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751865AbbGVUZB (ORCPT ); Wed, 22 Jul 2015 16:25:01 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,525,1432623600"; d="scan'208";a="610997490" Message-ID: <1437596699.22524.90.camel@tzanussi-mobl.amr.corp.intel.com> Subject: Re: [PATCH v9 13/22] tracing: Add hist trigger support for clearing a trace From: Tom Zanussi To: Masami Hiramatsu Cc: rostedt@goodmis.org, daniel.wagner@bmw-carit.de, namhyung@kernel.org, josh@joshtriplett.org, andi@firstfloor.org, linux-kernel@vger.kernel.org Date: Wed, 22 Jul 2015 15:24:59 -0500 In-Reply-To: <55AF9F9B.5080003@hitachi.com> References: <5c71e4c084d18cec2cd28d73878413c592686f47.1437066836.git.tom.zanussi@linux.intel.com> <55AF9F9B.5080003@hitachi.com> 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 Content-Length: 4976 Lines: 143 On Wed, 2015-07-22 at 22:50 +0900, Masami Hiramatsu wrote: > 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 ? > I do think it makes sense, and like the idea. But if we allow multiple histograms per event, does it still work? (I mean in that case, we'd like to pause/continue/clear them individually, right?) Tom > Thanks, > > > > > > Signed-off-by: Tom Zanussi > > --- > > 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= > "\t [:sort=field1,field2,...]\n" > > "\t [:size=#entries]\n" > > - "\t [:pause][:continue]\n" > > + "\t [:pause][:continue][:clear]\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. 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) > > > > -- 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/