[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1446601952.17120.167.camel@tzanussi-mobl.amr.corp.intel.com>
Date: Tue, 03 Nov 2015 19:52:32 -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 24/28] tracing: Add support for multiple hist
triggers per event
On Tue, 2015-11-03 at 09:34 +0900, Namhyung Kim wrote:
> 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 <tom.zanussi@...ux.intel.com>
> > Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> > ---
> > 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?
>
Yeah, I think you're right - adding multiple triggers should use >> (and
> should truncate as mentioned).
>
> > +
> > + 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. :)
Yep, thanks.
Tom
> 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
> >
--
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