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.zanu...@linux.intel.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.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?


> +
> +  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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to