Date: Fri, 17 Jul 2015 14:13:11 -0400
From: Steven Rostedt <rostedt@goodmis.org>
To: He Kuang <hekuang@huawei.com>
Cc: <ast@plumgrid.com>, <masami.hiramatsu.pt@hitachi.com>, <acme@kernel.org>,
        <a.p.zijlstra@chello.nl>, <mingo@redhat.com>, <namhyung@kernel.org>,
        <jolsa@kernel.org>, <wangnan0@huawei.com>, <pi3orama@163.com>,
        <linux-kernel@vger.kernel.org>,
        Alex =?UTF-8?B?QmVu?= =?UTF-8?B?bsOpZQ==?= 
	<alex.bennee@linaro.org>
Subject: Re: [RFC PATCH v4 1/3] tracing/events: Fix wrong sample output by
 storing array length instead of size
Message-ID: <20150717141311.41e4c30a@gandalf.local.home>
In-Reply-To: <20150717103215.7d285953@gandalf.local.home>
References: <1436522587-136825-1-git-send-email-hekuang@huawei.com>
	<1436522587-136825-2-git-send-email-hekuang@huawei.com>
	<20150717103215.7d285953@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4015
Lines: 105

On Fri, 17 Jul 2015 10:32:15 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:


> This change affects all callers of dymanic_array, not just bitmasks.
> 
> >  	__data_size += __item_length;
> >  
> >  #undef __string
> 
> BTW, if I revert commit ac01ce1410fc2 "tracing: Make
> ftrace_print_array_seq compute buf_len" it works again.
> 
> I'm going to look into this some more, and maybe the answer is to go
> back and just pass in buffer length here. I can't see what was broken
> before that change.

OK, the print_array() code is already being used by the thermal events
and can't be changed. But we can't make the proposed change because
that changes the user interface.

What we can change is the sample code!

-- Steve

>From 95de1e9721a2f9d05831a53d228e181a33001c55 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Date: Fri, 17 Jul 2015 14:03:26 -0400
Subject: [PATCH] tracing: Fix sample output of dynamic arrays

He Kuang noticed that the trace event samples for arrays was broken:

"The output result of trace_foo_bar event in traceevent samples is
 wrong. This problem can be reproduced as following:

  (Build kernel with SAMPLE_TRACE_EVENTS=m)

  $ insmod trace-events-sample.ko

  $ echo 1 > /sys/kernel/debug/tracing/events/sample-trace/foo_bar/enable

  $ cat /sys/kernel/debug/tracing/trace

  event-sample-980 [000] ....  43.649559: foo_bar: foo hello 21 0x15
  BIT1|BIT3|0x10 {0x1,0x6f6f6e53,0xff007970,0xffffffff} Snoopy
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 The array length is not right, should be {0x1}.
  (ffffffff,ffffffff)

  event-sample-980 [000] ....  44.653827: foo_bar: foo hello 22 0x16
  BIT2|BIT3|0x10
  {0x1,0x2,0x646e6147,0x666c61,0xffffffff,0xffffffff,0x750aeffe,0x7}
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 The array length is not right, should be {0x1,0x2}.
  Gandalf (ffffffff,ffffffff)"

This was caused by an update to have __print_array()'s second parameter
be the count of items in the array and not the size of the array.

As there is already users of __print_array(), it can not change. But
the sample code can and we can also improve on the documentation about
__print_array() and __get_dynamic_array_len().

Link: http://lkml.kernel.org/r/1436839171-31527-2-git-send-email-hekuang@huawei.com

Fixes: ac01ce1410fc2 ("tracing: Make ftrace_print_array_seq compute buf_len")
Reported-by: He Kuang <hekuang@huawei.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 samples/trace_events/trace-events-sample.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index 8965d1bb8811..125d6402f64f 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -168,7 +168,10 @@
  *
  *      For __dynamic_array(int, foo, bar) use __get_dynamic_array(foo)
  *            Use __get_dynamic_array_len(foo) to get the length of the array
- *            saved.
+ *            saved. Note, __get_dynamic_array_len() returns the total allocated
+ *            length of the dynamic array; __print_array() expects the second
+ *            parameter to be the number of elements. To get that, the array length
+ *            needs to be divided by the element size.
  *
  *      For __string(foo, bar) use __get_str(foo)
  *
@@ -288,7 +291,7 @@ TRACE_EVENT(foo_bar,
  *    This prints out the array that is defined by __array in a nice format.
  */
 		  __print_array(__get_dynamic_array(list),
-				__get_dynamic_array_len(list),
+				__get_dynamic_array_len(list) / sizeof(int),
 				sizeof(int)),
 		  __get_str(str), __get_bitmask(cpus))
 );
-- 
1.8.3.1

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