linux-kernel - Re: [PATCH v11 10/28] tracing: Add hist trigger support for multiple values ('vals=' param)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151102074244.GC17941@sejong>
Date:	Mon, 2 Nov 2015 16:42:44 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Tom Zanussi <tom.zanussi@...ux.intel.com>
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 10/28] tracing: Add hist trigger support for multiple
 values ('vals=' param)

On Thu, Oct 22, 2015 at 01:14:14PM -0500, Tom Zanussi wrote:
> Allow users to specify trace event fields to use in aggregated sums
> via a new 'vals=' keyword.  Before this addition, the only aggregated
> sum supported was the implied value 'hitcount'.  With this addition,
> 'hitcount' is also supported as an explicit value field, as is any
> numeric trace event field.
> 
> This expands the hist trigger syntax from this:
> 
>   # echo hist:keys=xxx [ if filter] > event/trigger
> 
> to this:
> 
>   # echo hist:keys=xxx:vals=yyy [ if filter] > event/trigger
> 
> Signed-off-by: Tom Zanussi <tom.zanussi@...ux.intel.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> ---

Just nipicks..


>  kernel/trace/trace.c             | 12 ++++---
>  kernel/trace/trace_events_hist.c | 75 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 81 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6d5870b..98817d5 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3799,14 +3799,16 @@ static const char readme_msg[] =
>  #ifdef CONFIG_HIST_TRIGGERS
>  	"      hist trigger\t- If set, event hits are aggregated into a hash table\n"
>  	"\t    Format: hist:keys=<field1>\n"
> +	"\t            [:values=<field1[,field2,...]]\n"

				<field1>[,<field2>,...]
or
				<field1[,field2,...]>


>  	"\t            [:size=#entries]\n"
>  	"\t            [if <filter>]\n\n"
>  	"\t    When a matching event is hit, an entry is added to a hash\n"
> -	"\t    table using the key named, and the value of a sum called\n"
> -	"\t    'hitcount' is incremented.  Keys correspond to fields in the\n"
> -	"\t    event's format description.  Keys can be any field.  The\n"
> -	"\t    'size' parameter can be  used to specify more or fewer than\n"
> -	"\t    the default 2048 entries for the hashtable size.\n\n"
> +	"\t    table using the key(s) and value(s) named, and the value of a\n"
> +	"\t    sum called 'hitcount' is incremented.  Keys and values\n"
> +	"\t    correspond to fields in the event's format description.  Keys\n"
> +	"\t    can be any field.  Values must correspond to numeric fields.\n"
> +	"\t    The 'size' parameter can be  used to specify more or fewer\n"
> +	"\t    than the default 2048 entries for the hashtable size.\n\n"
>  	"\t    Reading the 'hist' file for the event will dump the hash\n"
>  	"\t    table in its entirety to stdout."
>  #endif

[SNIP]
> @@ -534,6 +593,12 @@ hist_trigger_entry_print(struct seq_file *m,
>  	seq_printf(m, " hitcount: %10llu",
>  		   tracing_map_read_sum(elt, HITCOUNT_IDX));
>  
> +	for (i = 1; i < hist_data->n_vals; i++) {
> +		seq_printf(m, "  %s: %10llu",
> +			   hist_data->fields[i]->field->name,
> +			   tracing_map_read_sum(elt, i));
> +	}
> +
>  	seq_puts(m, "\n");
>  }
>  
> @@ -641,7 +706,15 @@ static int event_hist_trigger_print(struct seq_file *m,
>  	}
>  
>  	seq_puts(m, ":vals=");
> -	seq_puts(m, "hitcount");
> +
> +	for (i = 0; i < hist_data->n_vals; i++) {
> +		if (i == 0)

s/0/HITCOUNT_IDX/ ?

Thanks,
Namhyung


> +			seq_puts(m, "hitcount");
> +		else {
> +			seq_puts(m, ",");
> +			hist_field_print(m, hist_data->fields[i]);
> +		}
> +	}
>  
>  	seq_puts(m, ":sort=");
>  	seq_puts(m, "hitcount");
> -- 
> 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/