From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752722AbbKCAev (ORCPT ); Mon, 2 Nov 2015 19:34:51 -0500 Received: from mail-pa0-f50.google.com ([209.85.220.50]:33010 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751280AbbKCAes (ORCPT ); Mon, 2 Nov 2015 19:34:48 -0500 Date: Tue, 3 Nov 2015 09:34:37 +0900 From: Namhyung Kim To: Tom Zanussi 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 Subject: Re: [PATCH v11 24/28] tracing: Add support for multiple hist triggers per event Message-ID: <20151103003437.GC11498@danjae.kornet> References: <34f2d91cadfd4c7450d612172b4bbf5211a5a8c0.1445530672.git.tom.zanussi@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <34f2d91cadfd4c7450d612172b4bbf5211a5a8c0.1445530672.git.tom.zanussi@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 22, 2015 at 01:14:28PM -0500, Tom Zanussi wrote: > Allow users to define any number of hist triggers per trace event. > Any number of hist triggers may be added for a given event, which may > differ by key, value, or filter. > > Reading the event's 'hist' file will display the output of all the > hist triggers defined on an event concatenated in the order they were > defined. > > Signed-off-by: Tom Zanussi > Tested-by: Masami Hiramatsu > --- > Documentation/trace/events.txt | 151 +++++++++++++++++++++++++++++++++++++-- > kernel/trace/trace.c | 8 ++- > kernel/trace/trace_events_hist.c | 138 ++++++++++++++++++++++++++--------- > 3 files changed, 256 insertions(+), 41 deletions(-) > > diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt > index b3aa47e..6c64cb7 100644 > --- a/Documentation/trace/events.txt > +++ b/Documentation/trace/events.txt > @@ -532,12 +532,14 @@ The following commands are supported: > > 'hist' triggers add a 'hist' file to each event's subdirectory. > Reading the 'hist' file for the event will dump the hash table in > - its entirety to stdout. Each printed hash table entry is a simple > - list of the keys and values comprising the entry; keys are printed > - first and are delineated by curly braces, and are followed by the > - set of value fields for the entry. By default, numeric fields are > - displayed as base-10 integers. This can be modified by appending > - any of the following modifiers to the field name: > + its entirety to stdout. If there are multiple hist triggers > + attached to an event, there will be a table for each trigger in the > + output. Each printed hash table entry is a simple list of the keys > + and values comprising the entry; keys are printed first and are > + delineated by curly braces, and are followed by the set of value > + fields for the entry. By default, numeric fields are displayed as > + base-10 integers. This can be modified by appending any of the > + following modifiers to the field name: > > .hex display a number as a hex value > .sym display an address as a symbol > @@ -1629,3 +1631,140 @@ The following commands are supported: > . > . > . > + > + The following example demonstrates how multiple hist triggers can be > + attached to a given event. This capability can be useful for > + creating a set of different summaries derived from the same set of > + events, or for comparing the effects of different filters, among > + other things. > + > + # echo 'hist:keys=skbaddr.hex:vals=len if len < 0' > \ > + /sys/kernel/debug/tracing/events/net/netif_receive_skb/trigger > + # echo 'hist:keys=skbaddr.hex:vals=len if len > 4096' > \ > + /sys/kernel/debug/tracing/events/net/netif_receive_skb/trigger > + # echo 'hist:keys=skbaddr.hex:vals=len if len == 256' > \ > + /sys/kernel/debug/tracing/events/net/netif_receive_skb/trigger > + # echo 'hist:keys=skbaddr.hex:vals=len' > \ > + /sys/kernel/debug/tracing/events/net/netif_receive_skb/trigger > + # echo 'hist:keys=len:vals=common_preempt_count' > \ > + /sys/kernel/debug/tracing/events/net/netif_receive_skb/trigger AFAIK other tracefs files honor the truncation flag so open for writing would destroy other hist triggers. What do you think? > + > + The above set of commands create four triggers differing only in > + their filters, along with a completely different though fairly > + nonsensical trigger. [SNIP] > @@ -1289,22 +1368,18 @@ static int event_hist_trigger_func(struct event_command *cmd_ops, > > trigger_data->private_data = hist_data; > > + if (param) { /* if param is non-empty, it's supposed to be a filter */ > + ret = cmd_ops->set_filter(param, trigger_data, file); Maybe you want to check ->set_filter being NULL first. :) Thanks, Namhyung > + if (ret < 0) > + goto out_free; > + } > + > if (glob[0] == '!') { > cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file); > ret = 0; > goto out_free; > } > > - if (!param) /* if param is non-empty, it's supposed to be a filter */ > - goto out_reg; > - > - if (!cmd_ops->set_filter) > - goto out_reg; > - > - ret = cmd_ops->set_filter(param, trigger_data, file); > - if (ret < 0) > - goto out_free; > - out_reg: > ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file); > /* > * The above returns on success the # of triggers registered, > @@ -1337,7 +1412,7 @@ static struct event_command trigger_hist_cmd = { > .needs_rec = true, > .func = event_hist_trigger_func, > .reg = hist_register_trigger, > - .unreg = unregister_trigger, > + .unreg = hist_unregister_trigger, > .get_trigger_ops = event_hist_get_trigger_ops, > .set_filter = set_trigger_filter, > }; > @@ -1364,7 +1439,6 @@ hist_enable_trigger(struct event_trigger_data *data, void *rec) > test->paused = false; > else > test->paused = true; > - break; > } > } > } > -- > 1.9.3 >