[PATCH v4 0/3] Add array printing helpers to ftrace

[PATCH v4 0/3] Add array printing helpers to ftrace

[Javi Merino]

This series add a helper to the tracing framework to trace arrays.
Patch 1 adds them and patches 2 and 3 update the traceevent library to
parse them.  They've been tested with trace-cmd.

Changes since v3[0]:
  - use %zu to print size_t

Changes since v2:
  - Changed BUG() into a trace_seq_printf()
  - Add BUILD_BUG_ON() to chase mistakes in element sizes on build
  - Add patch 2 to avoid repeating code in patch 3.
  - print a warning in traeevent if the size of the array is not valid

[0] http://thread.gmane.org/gmane.linux.kernel/1869110

Dave Martin (1):
  tracing: Add array printing helpers

Javi Merino (2):
  tools lib traceevent: factor out allocating and processing args
  tools lib traceevent: Add support for __print_array()

 include/linux/ftrace_event.h       |   4 +
 include/trace/ftrace.h             |   9 +++
 kernel/trace/trace_output.c        |  44 ++++++++++
 tools/lib/traceevent/event-parse.c | 159 +++++++++++++++++++++++++++++--------
 tools/lib/traceevent/event-parse.h |   8 ++
 5 files changed, 193 insertions(+), 31 deletions(-)

-- 
1.9.1

[PATCH v4 1/3] tracing: Add array printing helpers

[Javi Merino]

From: Dave Martin <Dave.Martin@arm.com>

If a trace event contains an array, there is currently no standard
way to format this for text output.  Drivers are currently hacking
around this by a) local hacks that use the trace_seq functionailty
directly, or b) just not printing that information.  For fixed size
arrays, formatting of the elements can be open-coded, but this gets
cumbersome for arrays of non-trivial size.

These approaches result in non-standard content of the event format
description delivered to userspace, so userland tools needs to be
taught to understand and parse each array printing method
individually.

This patch implements common __print_<type>_array() helpers that
tracepoint implementations can use instead of reinventing them.  A
simple C-style syntax is used to delimit the array and its elements
{like,this}.

So that the helpers can be used with large static arrays as well as
dynamic arrays, they take a pointer and element count: they can be
used with __get_dynamic_array() for use with dynamic arrays.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 include/linux/ftrace_event.h |  4 ++++
 include/trace/ftrace.h       |  9 +++++++++
 kernel/trace/trace_output.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 0bebb5c348b8..5aa4a9269547 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -44,6 +44,10 @@ const char *ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
 const char *ftrace_print_hex_seq(struct trace_seq *p,
 				 const unsigned char *buf, int len);
 
+const char *ftrace_print_array_seq(struct trace_seq *p,
+				   const void *buf, int buf_len,
+				   size_t el_size);
+
 struct trace_iterator;
 struct trace_event;
 
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 139b5067345b..36afd0ed3458 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -263,6 +263,14 @@
 #undef __print_hex
 #define __print_hex(buf, buf_len) ftrace_print_hex_seq(p, buf, buf_len)
 
+#undef __print_array
+#define __print_array(array, count, el_size)				\
+	({								\
+		BUILD_BUG_ON(el_size != 8 && el_size != 16 &&		\
+			el_size != 32 && el_size != 64);		\
+		ftrace_print_array_seq(p, array, count, el_size);	\
+	})
+
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 static notrace enum print_line_t					\
@@ -674,6 +682,7 @@ static inline void ftrace_test_probe_##call(void)			\
 #undef __get_dynamic_array_len
 #undef __get_str
 #undef __get_bitmask
+#undef __print_array
 
 #undef TP_printk
 #define TP_printk(fmt, args...) "\"" fmt "\", "  __stringify(args)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index b77b9a697619..c06bd6e4ae0e 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -177,6 +177,50 @@ ftrace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len)
 }
 EXPORT_SYMBOL(ftrace_print_hex_seq);
 
+const char *
+ftrace_print_array_seq(struct trace_seq *p, const void *buf, int buf_len,
+		       size_t el_size)
+{
+	const char *ret = trace_seq_buffer_ptr(p);
+	const char *prefix = "";
+	void *ptr = (void *)buf;
+
+	trace_seq_putc(p, '{');
+
+	while (ptr < buf + buf_len) {
+		switch (el_size) {
+		case 8:
+			trace_seq_printf(p, "%s0x%x", prefix,
+					 *(u8 *)ptr);
+			break;
+		case 16:
+			trace_seq_printf(p, "%s0x%x", prefix,
+					 *(u16 *)ptr);
+			break;
+		case 32:
+			trace_seq_printf(p, "%s0x%x", prefix,
+					 *(u32 *)ptr);
+			break;
+		case 64:
+			trace_seq_printf(p, "%s0x%llx", prefix,
+					 *(u64 *)ptr);
+			break;
+		default:
+			trace_seq_printf(p, "BAD SIZE:%zu 0x%x", el_size,
+					 *(u8 *)ptr);
+			el_size = 8;
+		}
+		prefix = ",";
+		ptr += el_size / 8;
+	}
+
+	trace_seq_putc(p, '}');
+	trace_seq_putc(p, 0);
+
+	return ret;
+}
+EXPORT_SYMBOL(ftrace_print_array_seq);
+
 int ftrace_raw_output_prep(struct trace_iterator *iter,
 			   struct trace_event *trace_event)
 {
-- 
1.9.1

[PATCH v4 2/3] tools lib traceevent: factor out allocating and processing args

[Javi Merino]

The sequence of allocating the print_arg field, calling process_arg()
and verifying that the next event delimiter is repeated twice in
process_hex() and will also be used for process_int_array().  Factor it
out to a function to avoid writing the same code again and again.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <srostedt@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 tools/lib/traceevent/event-parse.c | 77 ++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index cf3a44bf1ec3..dabd8f5c6398 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2013,6 +2013,38 @@ process_entry(struct event_format *event __maybe_unused, struct print_arg *arg,
 	return EVENT_ERROR;
 }
 
+static int alloc_and_process_arg(struct event_format *event, char *next_token,
+				 struct print_arg **print_arg)
+{
+	struct print_arg *field;
+	enum event_type type;
+	char *token;
+	int ret = 0;
+
+	field = alloc_arg();
+	if (!field) {
+		do_warning_event(event, "%s: not enough memory!", __func__);
+		errno = ENOMEM;
+		return -1;
+	}
+
+	type = process_arg(event, field, &token);
+
+	if (test_type_token(type, token, EVENT_DELIM, next_token)) {
+		errno = EINVAL;
+		ret = -1;
+		free_arg(field);
+		goto out_free_token;
+	}
+
+	*print_arg = field;
+
+out_free_token:
+	free_token(token);
+
+	return ret;
+}
+
 static char *arg_eval (struct print_arg *arg);
 
 static unsigned long long
@@ -2485,49 +2517,20 @@ out_free:
 static enum event_type
 process_hex(struct event_format *event, struct print_arg *arg, char **tok)
 {
-	struct print_arg *field;
-	enum event_type type;
-	char *token = NULL;
-
 	memset(arg, 0, sizeof(*arg));
 	arg->type = PRINT_HEX;
 
-	field = alloc_arg();
-	if (!field) {
-		do_warning_event(event, "%s: not enough memory!", __func__);
-		goto out_free;
-	}
-
-	type = process_arg(event, field, &token);
-
-	if (test_type_token(type, token, EVENT_DELIM, ","))
-		goto out_free;
-
-	arg->hex.field = field;
-
-	free_token(token);
-
-	field = alloc_arg();
-	if (!field) {
-		do_warning_event(event, "%s: not enough memory!", __func__);
-		*tok = NULL;
-		return EVENT_ERROR;
-	}
-
-	type = process_arg(event, field, &token);
-
-	if (test_type_token(type, token, EVENT_DELIM, ")"))
-		goto out_free;
+	if (alloc_and_process_arg(event, ",", &arg->hex.field))
+		goto out;
 
-	arg->hex.size = field;
+	if (alloc_and_process_arg(event, ")", &arg->hex.size))
+		goto free_field;
 
-	free_token(token);
-	type = read_token_item(tok);
-	return type;
+	return read_token_item(tok);
 
- out_free:
-	free_arg(field);
-	free_token(token);
+free_field:
+	free_arg(arg->hex.field);
+out:
 	*tok = NULL;
 	return EVENT_ERROR;
 }
-- 
1.9.1

[PATCH v4 3/3] tools lib traceevent: Add support for __print_array()

[Javi Merino]

Trace can now generate traces with variable element size arrays.  Add
support to parse them.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <srostedt@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 tools/lib/traceevent/event-parse.c | 94 ++++++++++++++++++++++++++++++++++++++
 tools/lib/traceevent/event-parse.h |  8 ++++
 2 files changed, 102 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index dabd8f5c6398..9cb05c440821 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -757,6 +757,11 @@ static void free_arg(struct print_arg *arg)
 		free_arg(arg->hex.field);
 		free_arg(arg->hex.size);
 		break;
+	case PRINT_INT_ARRAY:
+		free_arg(arg->int_array.field);
+		free_arg(arg->int_array.size);
+		free_arg(arg->int_array.el_size);
+		break;
 	case PRINT_TYPE:
 		free(arg->typecast.type);
 		free_arg(arg->typecast.item);
@@ -2536,6 +2541,32 @@ out:
 }
 
 static enum event_type
+process_int_array(struct event_format *event, struct print_arg *arg, char **tok)
+{
+	memset(arg, 0, sizeof(*arg));
+	arg->type = PRINT_INT_ARRAY;
+
+	if (alloc_and_process_arg(event, ",", &arg->int_array.field))
+		goto out;
+
+	if (alloc_and_process_arg(event, ",", &arg->int_array.size))
+		goto free_field;
+
+	if (alloc_and_process_arg(event, ")", &arg->int_array.el_size))
+		goto free_size;
+
+	return read_token_item(tok);
+
+free_size:
+	free_arg(arg->int_array.size);
+free_field:
+	free_arg(arg->int_array.field);
+out:
+	*tok = NULL;
+	return EVENT_ERROR;
+}
+
+static enum event_type
 process_dynamic_array(struct event_format *event, struct print_arg *arg, char **tok)
 {
 	struct format_field *field;
@@ -2830,6 +2861,10 @@ process_function(struct event_format *event, struct print_arg *arg,
 		free_token(token);
 		return process_hex(event, arg, tok);
 	}
+	if (strcmp(token, "__print_array") == 0) {
+		free_token(token);
+		return process_int_array(event, arg, tok);
+	}
 	if (strcmp(token, "__get_str") == 0) {
 		free_token(token);
 		return process_str(event, arg, tok);
@@ -3358,6 +3393,7 @@ eval_num_arg(void *data, int size, struct event_format *event, struct print_arg
 		break;
 	case PRINT_FLAGS:
 	case PRINT_SYMBOL:
+	case PRINT_INT_ARRAY:
 	case PRINT_HEX:
 		break;
 	case PRINT_TYPE:
@@ -3768,6 +3804,55 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 		}
 		break;
 
+	case PRINT_INT_ARRAY: {
+		void *num;
+		int el_size;
+
+		if (arg->int_array.field->type == PRINT_DYNAMIC_ARRAY) {
+			unsigned long offset;
+			struct format_field *field =
+				arg->int_array.field->dynarray.field;
+			offset = pevent_read_number(pevent,
+						    data + field->offset,
+						    field->size);
+			num = data + (offset & 0xffff);
+		} else {
+			field = arg->int_array.field->field.field;
+			if (!field) {
+				str = arg->int_array.field->field.name;
+				field = pevent_find_any_field(event, str);
+				if (!field)
+					goto out_warning_field;
+				arg->int_array.field->field.field = field;
+			}
+			num = data + field->offset;
+		}
+		len = eval_num_arg(data, size, event, arg->int_array.size);
+		el_size = eval_num_arg(data, size, event,
+				       arg->int_array.el_size);
+		el_size /= 8;
+		for (i = 0; i < len; i++) {
+			if (i)
+				trace_seq_putc(s, ' ');
+
+			if (el_size == 1) {
+				trace_seq_printf(s, "%u", *(uint8_t *)num);
+			} else if (el_size == 2) {
+				trace_seq_printf(s, "%u", *(uint16_t *)num);
+			} else if (el_size == 4) {
+				trace_seq_printf(s, "%u", *(uint32_t *)num);
+			} else if (el_size == 8) {
+				trace_seq_printf(s, "%llu", *(uint64_t *)num);
+			} else {
+				trace_seq_printf(s, "BAD SIZE:%d 0x%x",
+						 el_size, *(uint8_t *)num);
+				el_size = 1;
+			}
+
+			num += el_size;
+		}
+		break;
+	}
 	case PRINT_TYPE:
 		break;
 	case PRINT_STRING: {
@@ -4931,6 +5016,15 @@ static void print_args(struct print_arg *args)
 		print_args(args->hex.size);
 		printf(")");
 		break;
+	case PRINT_INT_ARRAY:
+		printf("__print_array(");
+		print_args(args->int_array.field);
+		printf(", ");
+		print_args(args->int_array.size);
+		printf(", ");
+		print_args(args->int_array.el_size);
+		printf(")");
+		break;
 	case PRINT_STRING:
 	case PRINT_BSTRING:
 		printf("__get_str(%s)", args->string.string);
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 7a3873ff9a4f..5ac3e3c00389 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -245,6 +245,12 @@ struct print_arg_hex {
 	struct print_arg	*size;
 };
 
+struct print_arg_int_array {
+	struct print_arg	*field;
+	struct print_arg	*size;
+	struct print_arg	*el_size;
+};
+
 struct print_arg_dynarray {
 	struct format_field	*field;
 	struct print_arg	*index;
@@ -273,6 +279,7 @@ enum print_arg_type {
 	PRINT_FLAGS,
 	PRINT_SYMBOL,
 	PRINT_HEX,
+	PRINT_INT_ARRAY,
 	PRINT_TYPE,
 	PRINT_STRING,
 	PRINT_BSTRING,
@@ -292,6 +299,7 @@ struct print_arg {
 		struct print_arg_flags		flags;
 		struct print_arg_symbol		symbol;
 		struct print_arg_hex		hex;
+		struct print_arg_int_array	int_array;
 		struct print_arg_func		func;
 		struct print_arg_string		string;
 		struct print_arg_bitmask	bitmask;
-- 
1.9.1

Re: [PATCH v4 1/3] tracing: Add array printing helpers

[Steven Rostedt]

On Mon, 26 Jan 2015 12:11:49 +0000
Javi Merino <javi.merino@arm.com> wrote:

> From: Dave Martin <Dave.Martin@arm.com>
> 
> If a trace event contains an array, there is currently no standard
> way to format this for text output.  Drivers are currently hacking
> around this by a) local hacks that use the trace_seq functionailty
> directly, or b) just not printing that information.  For fixed size
> arrays, formatting of the elements can be open-coded, but this gets
> cumbersome for arrays of non-trivial size.
> 
> These approaches result in non-standard content of the event format
> description delivered to userspace, so userland tools needs to be
> taught to understand and parse each array printing method
> individually.
> 
> This patch implements common __print_<type>_array() helpers that
> tracepoint implementations can use instead of reinventing them.  A
> simple C-style syntax is used to delimit the array and its elements
> {like,this}.
> 
> So that the helpers can be used with large static arrays as well as
> dynamic arrays, they take a pointer and element count: they can be
> used with __get_dynamic_array() for use with dynamic arrays.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>  include/linux/ftrace_event.h |  4 ++++
>  include/trace/ftrace.h       |  9 +++++++++
>  kernel/trace/trace_output.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 0bebb5c348b8..5aa4a9269547 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -44,6 +44,10 @@ const char *ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
>  const char *ftrace_print_hex_seq(struct trace_seq *p,
>  				 const unsigned char *buf, int len);
>  
> +const char *ftrace_print_array_seq(struct trace_seq *p,
> +				   const void *buf, int buf_len,
> +				   size_t el_size);
> +
>  struct trace_iterator;
>  struct trace_event;
>  
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index 139b5067345b..36afd0ed3458 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -263,6 +263,14 @@
>  #undef __print_hex
>  #define __print_hex(buf, buf_len) ftrace_print_hex_seq(p, buf, buf_len)
>  
> +#undef __print_array
> +#define __print_array(array, count, el_size)				\
> +	({								\
> +		BUILD_BUG_ON(el_size != 8 && el_size != 16 &&		\
> +			el_size != 32 && el_size != 64);		\

I tried testing this patch by writing a print_array myself, and I kept
hitting this BUILD_BUG_ON, and was wondering WTF? Then it dawned on me.

el_size should not be based on bits, it should be based on bytes. I
passed in "sizeof()" which doesn't work with bits.

Please update to "el_size < 1 || el_size > 8".

and adjust the rest accordingly.

Thanks,

-- Steve


> +		ftrace_print_array_seq(p, array, count, el_size);	\
> +	})
> +
>  #undef DECLARE_EVENT_CLASS
>  #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
>  static notrace enum print_line_t					\
> @@ -674,6 +682,7 @@ static inline void ftrace_test_probe_##call(void)			\
>  #undef __get_dynamic_array_len
>  #undef __get_str
>  #undef __get_bitmask
> +#undef __print_array
>  
>  #undef TP_printk
>  #define TP_printk(fmt, args...) "\"" fmt "\", "  __stringify(args)
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index b77b9a697619..c06bd6e4ae0e 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -177,6 +177,50 @@ ftrace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len)
>  }
>  EXPORT_SYMBOL(ftrace_print_hex_seq);
>  
> +const char *
> +ftrace_print_array_seq(struct trace_seq *p, const void *buf, int buf_len,
> +		       size_t el_size)
> +{
> +	const char *ret = trace_seq_buffer_ptr(p);
> +	const char *prefix = "";
> +	void *ptr = (void *)buf;
> +
> +	trace_seq_putc(p, '{');
> +
> +	while (ptr < buf + buf_len) {
> +		switch (el_size) {
> +		case 8:
> +			trace_seq_printf(p, "%s0x%x", prefix,
> +					 *(u8 *)ptr);
> +			break;
> +		case 16:
> +			trace_seq_printf(p, "%s0x%x", prefix,
> +					 *(u16 *)ptr);
> +			break;
> +		case 32:
> +			trace_seq_printf(p, "%s0x%x", prefix,
> +					 *(u32 *)ptr);
> +			break;
> +		case 64:
> +			trace_seq_printf(p, "%s0x%llx", prefix,
> +					 *(u64 *)ptr);
> +			break;
> +		default:
> +			trace_seq_printf(p, "BAD SIZE:%zu 0x%x", el_size,
> +					 *(u8 *)ptr);
> +			el_size = 8;
> +		}
> +		prefix = ",";
> +		ptr += el_size / 8;
> +	}
> +
> +	trace_seq_putc(p, '}');
> +	trace_seq_putc(p, 0);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ftrace_print_array_seq);
> +
>  int ftrace_raw_output_prep(struct trace_iterator *iter,
>  			   struct trace_event *trace_event)
>  {

Re: [PATCH v4 1/3] tracing: Add array printing helpers

[Javi Merino]

On Wed, Jan 28, 2015 at 03:35:57AM +0000, Steven Rostedt wrote:
> On Mon, 26 Jan 2015 12:11:49 +0000
> Javi Merino <javi.merino@arm.com> wrote:
> 
> > From: Dave Martin <Dave.Martin@arm.com>
> > 
> > If a trace event contains an array, there is currently no standard
> > way to format this for text output.  Drivers are currently hacking
> > around this by a) local hacks that use the trace_seq functionailty
> > directly, or b) just not printing that information.  For fixed size
> > arrays, formatting of the elements can be open-coded, but this gets
> > cumbersome for arrays of non-trivial size.
> > 
> > These approaches result in non-standard content of the event format
> > description delivered to userspace, so userland tools needs to be
> > taught to understand and parse each array printing method
> > individually.
> > 
> > This patch implements common __print_<type>_array() helpers that
> > tracepoint implementations can use instead of reinventing them.  A
> > simple C-style syntax is used to delimit the array and its elements
> > {like,this}.
> > 
> > So that the helpers can be used with large static arrays as well as
> > dynamic arrays, they take a pointer and element count: they can be
> > used with __get_dynamic_array() for use with dynamic arrays.
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > Signed-off-by: Javi Merino <javi.merino@arm.com>
> > ---
> >  include/linux/ftrace_event.h |  4 ++++
> >  include/trace/ftrace.h       |  9 +++++++++
> >  kernel/trace/trace_output.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 57 insertions(+)
> > 
> > diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> > index 0bebb5c348b8..5aa4a9269547 100644
> > --- a/include/linux/ftrace_event.h
> > +++ b/include/linux/ftrace_event.h
> > @@ -44,6 +44,10 @@ const char *ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
> >  const char *ftrace_print_hex_seq(struct trace_seq *p,
> >  				 const unsigned char *buf, int len);
> >  
> > +const char *ftrace_print_array_seq(struct trace_seq *p,
> > +				   const void *buf, int buf_len,
> > +				   size_t el_size);
> > +
> >  struct trace_iterator;
> >  struct trace_event;
> >  
> > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> > index 139b5067345b..36afd0ed3458 100644
> > --- a/include/trace/ftrace.h
> > +++ b/include/trace/ftrace.h
> > @@ -263,6 +263,14 @@
> >  #undef __print_hex
> >  #define __print_hex(buf, buf_len) ftrace_print_hex_seq(p, buf, buf_len)
> >  
> > +#undef __print_array
> > +#define __print_array(array, count, el_size)				\
> > +	({								\
> > +		BUILD_BUG_ON(el_size != 8 && el_size != 16 &&		\
> > +			el_size != 32 && el_size != 64);		\
> 
> I tried testing this patch by writing a print_array myself, and I kept
> hitting this BUILD_BUG_ON, and was wondering WTF? Then it dawned on me.
> 
> el_size should not be based on bits, it should be based on bytes. I
> passed in "sizeof()" which doesn't work with bits.
> 
> Please update to "el_size < 1 || el_size > 8".
> 
> and adjust the rest accordingly.

Done, I'm testing it now.  I'll send a v5 later today.

Cheers,
Javi

Re: [PATCH v4 1/3] tracing: Add array printing helpers

[Javi Merino]

On Wed, Jan 28, 2015 at 11:26:09AM +0000, Javi Merino wrote:
> On Wed, Jan 28, 2015 at 03:35:57AM +0000, Steven Rostedt wrote:
> > On Mon, 26 Jan 2015 12:11:49 +0000
> > Javi Merino <javi.merino@arm.com> wrote:
> > 
> > > From: Dave Martin <Dave.Martin@arm.com>
> > > 
> > > If a trace event contains an array, there is currently no standard
> > > way to format this for text output.  Drivers are currently hacking
> > > around this by a) local hacks that use the trace_seq functionailty
> > > directly, or b) just not printing that information.  For fixed size
> > > arrays, formatting of the elements can be open-coded, but this gets
> > > cumbersome for arrays of non-trivial size.
> > > 
> > > These approaches result in non-standard content of the event format
> > > description delivered to userspace, so userland tools needs to be
> > > taught to understand and parse each array printing method
> > > individually.
> > > 
> > > This patch implements common __print_<type>_array() helpers that
> > > tracepoint implementations can use instead of reinventing them.  A
> > > simple C-style syntax is used to delimit the array and its elements
> > > {like,this}.
> > > 
> > > So that the helpers can be used with large static arrays as well as
> > > dynamic arrays, they take a pointer and element count: they can be
> > > used with __get_dynamic_array() for use with dynamic arrays.
> > > 
> > > Cc: Steven Rostedt <rostedt@goodmis.org>
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> > > Signed-off-by: Javi Merino <javi.merino@arm.com>
> > > ---
> > >  include/linux/ftrace_event.h |  4 ++++
> > >  include/trace/ftrace.h       |  9 +++++++++
> > >  kernel/trace/trace_output.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 57 insertions(+)
> > > 
> > > diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> > > index 0bebb5c348b8..5aa4a9269547 100644
> > > --- a/include/linux/ftrace_event.h
> > > +++ b/include/linux/ftrace_event.h
> > > @@ -44,6 +44,10 @@ const char *ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
> > >  const char *ftrace_print_hex_seq(struct trace_seq *p,
> > >  				 const unsigned char *buf, int len);
> > >  
> > > +const char *ftrace_print_array_seq(struct trace_seq *p,
> > > +				   const void *buf, int buf_len,
> > > +				   size_t el_size);
> > > +
> > >  struct trace_iterator;
> > >  struct trace_event;
> > >  
> > > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> > > index 139b5067345b..36afd0ed3458 100644
> > > --- a/include/trace/ftrace.h
> > > +++ b/include/trace/ftrace.h
> > > @@ -263,6 +263,14 @@
> > >  #undef __print_hex
> > >  #define __print_hex(buf, buf_len) ftrace_print_hex_seq(p, buf, buf_len)
> > >  
> > > +#undef __print_array
> > > +#define __print_array(array, count, el_size)				\
> > > +	({								\
> > > +		BUILD_BUG_ON(el_size != 8 && el_size != 16 &&		\
> > > +			el_size != 32 && el_size != 64);		\
> > 
> > I tried testing this patch by writing a print_array myself, and I kept
> > hitting this BUILD_BUG_ON, and was wondering WTF? Then it dawned on me.
> > 
> > el_size should not be based on bits, it should be based on bytes. I
> > passed in "sizeof()" which doesn't work with bits.

Ugh.  If you use sizeof() in print_array() then trace-cmd won't be
able to parse it since it doesn't have an implementation of sizeof().

  [thermal_power_allocator:thermal_power_allocator] function sizeof not defined
  Error: expected type 5 but read 0
*** Error in `./trace-cmd': double free or corruption (fasttop): 0x0000000000e04980 ***

Implementing a fully functional sizeof() in trace-cmd will be a huge
beast, I guess you will have to limit it to only some types.

> > Please update to "el_size < 1 || el_size > 8".
> > 
> > and adjust the rest accordingly.
> 
> Done, I'm testing it now.  I'll send a v5 later today.
> 
> Cheers,
> Javi

Re: [PATCH v4 1/3] tracing: Add array printing helpers

[Steven Rostedt]

On Wed, 28 Jan 2015 12:24:03 +0000
Javi Merino <javi.merino@arm.com> wrote:

> > > el_size should not be based on bits, it should be based on bytes. I
> > > passed in "sizeof()" which doesn't work with bits.
> 
> Ugh.  If you use sizeof() in print_array() then trace-cmd won't be
> able to parse it since it doesn't have an implementation of sizeof().
> 
>   [thermal_power_allocator:thermal_power_allocator] function sizeof not defined
>   Error: expected type 5 but read 0
> *** Error in `./trace-cmd': double free or corruption (fasttop): 0x0000000000e04980 ***
> 
> Implementing a fully functional sizeof() in trace-cmd will be a huge
> beast, I guess you will have to limit it to only some types.

Well it shouldn't crash, because people will be using things like
sizeof(). Just default it to 4 or 8 (word length).

We can work on a sizeof routine later. I have ideas on how to do it. Or
at least make it work for the common cases.

-- Steve

> 
> > > Please update to "el_size < 1 || el_size > 8".
> > > 
> > > and adjust the rest accordingly.
> > 
> > Done, I'm testing it now.  I'll send a v5 later today.
> > 
> > Cheers,
> > Javi