Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <1581636974.2374.2.camel@gmail.com>
Subject: Re: [PATCH 0/3] tracing: synth_event_trace fixes
From:   Tom Zanussi <tzanussi@gmail.com>
To:     rostedt@goodmis.org
Cc:     artem.bityutskiy@linux.intel.com, mhiramat@kernel.org,
        linux-kernel@vger.kernel.org
Date:   Thu, 13 Feb 2020 17:36:14 -0600
In-Reply-To: <cover.1581630377.git.zanussi@kernel.org>
References: <cover.1581630377.git.zanussi@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Steve,

I apparently tested the wrong patches, and while I think patch 1 and 3
are ok, I'm seeing a problem with patch 2 (then endian changes).  Will
send a v2 as soon as I can.

Tom

On Thu, 2020-02-13 at 16:16 -0600, Tom Zanussi wrote:
> Hi Steve,
> 
> Sorry, it took me some time to get a 32-bit x86 system up and running
> here in order to build and test things on i386.  These patches pass
> both selftests and the synth_event_gen_test testing, although the bug
> where (null) prints after every integer field in the trace output is
> still there and is there even before these or yesterday's patches - I
> have a suspicion it's been there for awhile but nobody looked at
> synthetic event trace output on i386.  In any case, I'm going to
> continue looking into that - it's a weird situation where nothing
> gets
> put in the final %s in the format string on i386 so shows as (null),
> even though it looks like it's there.  Anyway..
> 
> Here are 3 bugfix patches, the first of which fixes the bug seen by
> the test robot, and the other two are patches that fix a couple
> things
> I noticed when doing the first patch.
> 
> The previous patch I sent, changing u64 to long for the test robot
> bug
> did fix that problem too, but on i386 systems that would reduce every
> field to 32 bits, which isn't what we want either.  The new patch
> doesn't change the code in synth_event_trace() - it still uses u64
> just like synth_event_trace_array() which takes an array of u64.
> Without any further information such as a format string, I don't know
> of a better way to deal with the varargs version, other than require
> it get passed what it expects, u64 params.
> 
> The second patch adds the same endianness fix as for
> trace_event_raw_event_synth(), and the last one just adds back a
> missing check fot synth_event_trace() and synth_event_trace_array().
> 
> Thanks,
> 
> Tom
> 
> The following changes since commit
> 359c92c02bfae1a6f1e8e37c298e518fd256642c:
> 
>   Merge tag 'dax-fixes-5.6-rc1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm (2020-02-
> 11 16:52:08 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-
> trace.git ftrace/synth-event-gen-fixes2-v1
> 
> Tom Zanussi (3):
>   tracing: Make sure synth_event_trace() example always uses u64
>   tracing: Make synth_event trace functions endian-correct
>   tracing: Check that number of vals matches number of synth  event
>     fields
> 
>  kernel/trace/synth_event_gen_test.c | 34 +++++++++----------
>  kernel/trace/trace_events_hist.c    | 68
> ++++++++++++++++++++++++++++++++++---
>  2 files changed, 81 insertions(+), 21 deletions(-)
>