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

[Namhyung Kim <namhyung@kernel.org>]

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?

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)