From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754003AbbGWTg1 (ORCPT ); Thu, 23 Jul 2015 15:36:27 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:38870 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753580AbbGWTgY (ORCPT ); Thu, 23 Jul 2015 15:36:24 -0400 References: <1436522587-136825-1-git-send-email-hekuang@huawei.com> <1436522587-136825-2-git-send-email-hekuang@huawei.com> <20150717103215.7d285953@gandalf.local.home> <20150717141311.41e4c30a@gandalf.local.home> From: Alex =?utf-8?Q?Benn=C3=A9e?= To: Steven Rostedt Cc: He Kuang , 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 Subject: Re: [RFC PATCH v4 1/3] tracing/events: Fix wrong sample output by storing array length instead of size In-reply-to: <20150717141311.41e4c30a@gandalf.local.home> Date: Thu, 23 Jul 2015 20:36:20 +0100 Message-ID: <878ua6u0nf.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Steven Rostedt writes: > On Fri, 17 Jul 2015 10:32:15 -0400 > Steven Rostedt 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)" > 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 > Signed-off-by: Steven Rostedt > --- > 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)) > ); Reviewed-by: Alex Bennée -- Alex Bennée