Re: [PATCH v11 24/28] tracing: Add support for multiple hist triggers per event

2015-11-03 Thread Tom Zanussi
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 
> > 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:
> >  
> >  .hexdisplay a number as a hex value
> > .symdisplay 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 th

Re: [PATCH v11 24/28] tracing: Add support for multiple hist triggers per event

2015-11-02 Thread Namhyung Kim
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:
>  
>  .hexdisplay a number as a hex value
>   .symdisplay 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

[PATCH v11 24/28] tracing: Add support for multiple hist triggers per event

2015-10-22 Thread Tom Zanussi
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:
 
 .hexdisplay a number as a hex value
.symdisplay 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
+
+  The above set of commands create four triggers differing only in
+  their filters, along with a completely different though fairly
+  nonsensical trigger.
+
+  Displaying the contents of the 'hist' file for the event shows the
+  contents of all five histograms:
+
+# cat /sys/kernel/debug/tracing/events/net/netif_receive_skb/hist
+
+# event histogram
+#
+# trigger info: 
hist:keys=len:vals=hitcount,common_preempt_count:sort=hitcount:size=2048 
[active]
+#
+
+{ len:176 } hitcount:  1  common_preempt_count:  0
+{ len:223 } hitcount:  1  common_preempt_count:  0
+{ len:   4854 } hitcount:  1  common_preempt_count:  0
+{ len:395 } hitcount:  1  common_preempt_count:  0
+{ len:177 } hitcount:  1  common_preempt_count:  0
+{ len:446 } hitcount:  1  common_preempt_count:  0
+{ len:   1601 } hitcount:  1  common_preempt_count:  0
+.
+.
+.
+{ len:   1280 } hitcount: 66  common_preempt_count:  0
+{ len:116 } hitcount: 81  common_preempt_count: 40
+{ len:708 } hitcount:112  common_preempt_count:  0
+{ len: 46 } hitcount:221  common_preempt_count:  0
+{ len:   1264 } hitcount:458  common_preempt_count:  0
+
+Totals:
+Hits: 1428
+Entries: 147
+Dropped: 0
+
+
+# event histogram
+#
+# trigger info: 
hist:keys=skbaddr.hex:vals=hitcount,len:sort=hitcount:size=2048 [active]
+#
+
+{ skbaddr: 8800baee5e00 } hitcount:  1  len:130
+{ skbaddr: 88005f3d5600 } hitcount:  1  len:   1280
+{ skbaddr: 88005f3d4900 } hitcount:  1  len:   1280
+{ skba