linux-kernel - Re: [PATCH v16 22/23] tracing: Add hist trigger 'log2' modifier

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 for Android: free password hash cracker in your pocket

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

Message-ID: <57044BA4.8000709@linux.intel.com>
Date:	Tue, 05 Apr 2016 18:35:00 -0500
From:	Tom Zanussi <tom.zanussi@...ux.intel.com>
To:	Namhyung Kim <namhyung@...nel.org>, Daniel Wagner <wagi@...om.org>
CC:	rostedt@...dmis.org, 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 v16 22/23] tracing: Add hist trigger 'log2' modifier

Hi Namhyung,

Sorry for the late reply, just catching up after Spring Break...

On 03/29/2016 10:17 AM, Namhyung Kim wrote:
> Hi Daniel,
>
> On Tue, Mar 29, 2016 at 12:01:40PM +0200, Daniel Wagner wrote:
>> Hi,
>>
>> On 03/03/2016 07:55 PM, Tom Zanussi wrote:
>>> When using '.log2' modifier, the output looks like:
>>>
>>>    # echo 'hist:key=bytes_req.log2' > kmalloc/trigger
>>>    # cat kmalloc/hist
>>>
>>>    { bytes_req: ~ 2^12 } hitcount:          1
>>>    { bytes_req: ~ 2^11 } hitcount:          1
>>>    { bytes_req: ~ 2^9  } hitcount:          2
>>>    { bytes_req: ~ 2^6  } hitcount:          3
>>>    { bytes_req: ~ 2^3  } hitcount:         13
>>>    { bytes_req: ~ 2^5  } hitcount:         19
>>>    { bytes_req: ~ 2^8  } hitcount:         49
>>>    { bytes_req: ~ 2^7  } hitcount:         57
>>>    { bytes_req: ~ 2^4  } hitcount:         74
>>
>>
>> I found a small inconsistency. My command line is
>>
>>    echo 'hist:key=latency.log2:sort=latency' > /sys/kernel/debug/tracing/events/test/latency_complete/trigger
>>
>> When looking at the output of 'hist' you see that the
>> 'sort=' is not what I provided.
>>
>> cat /sys/kernel/debug/tracing/events/test/latency_complete/hist
>> # event histogram
>> #
>> # trigger info: hist:keys=latency.log2:vals=hitcount:sort=latency.log2:size=2048 [active]
>> #
>> #
>
> Maybe we want to skip printing those flags for sort keys..
> What about this?
>

Looks fine to me.

Thanks,

Acked-by: Tom Zanussi <tom.zanussi@...ux.intel.com>

> Thanks,
> Namhyung
>
>
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 53e7d7bc67ca..464ef52117bd 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -1118,10 +1118,11 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
>   	return flags_str;
>   }
>
> -static void hist_field_print(struct seq_file *m, struct hist_field *hist_field)
> +static void hist_field_print(struct seq_file *m, struct hist_field *hist_field,
> +			     bool print_flags)
>   {
>   	seq_printf(m, "%s", hist_field->field->name);
> -	if (hist_field->flags) {
> +	if (hist_field->flags && print_flags) {
>   		const char *flags_str = get_hist_field_flags(hist_field);
>
>   		if (flags_str)
> @@ -1153,7 +1154,7 @@ static int event_hist_trigger_print(struct seq_file *m,
>   		if (key_field->flags & HIST_FIELD_STACKTRACE)
>   			seq_puts(m, "stacktrace");
>   		else
> -			hist_field_print(m, key_field);
> +			hist_field_print(m, key_field, true);
>   	}
>
>   	seq_puts(m, ":vals=");
> @@ -1163,7 +1164,7 @@ static int event_hist_trigger_print(struct seq_file *m,
>   			seq_puts(m, "hitcount");
>   		else {
>   			seq_puts(m, ",");
> -			hist_field_print(m, hist_data->fields[i]);
> +			hist_field_print(m, hist_data->fields[i], true);
>   		}
>   	}
>
> @@ -1182,7 +1183,7 @@ static int event_hist_trigger_print(struct seq_file *m,
>   		else {
>   			unsigned int idx = sort_key->field_idx;
>
> -			hist_field_print(m, hist_data->fields[idx]);
> +			hist_field_print(m, hist_data->fields[idx], false);
>   		}
>
>   		if (sort_key->descending)
>