From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95461C2B9F4 for ; Tue, 22 Jun 2021 21:25:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7C78761352 for ; Tue, 22 Jun 2021 21:25:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229667AbhFVV1t (ORCPT ); Tue, 22 Jun 2021 17:27:49 -0400 Received: from mail3-relais-sop.national.inria.fr ([192.134.164.104]:53040 "EHLO mail3-relais-sop.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229612AbhFVV1s (ORCPT ); Tue, 22 Jun 2021 17:27:48 -0400 IronPort-HdrOrdr: =?us-ascii?q?A9a23=3A3YbhaanUYQScgZAC/MOGX2WJwnDpDfIr3DAb?= =?us-ascii?q?v31ZSRFFG/Fw9vre+MjzuiWetN98YhsdcJW7WZVoIkmskKKdg7NwAV7KZmCPhI?= =?us-ascii?q?LrFvAA0WKI+VPd8kPFmtK1mZ0QEZRWOZnASWJ3isv3+2CDfuoIytPvys+Vuds?= =?us-ascii?q?=3D?= X-IronPort-AV: E=Sophos;i="5.83,292,1616454000"; d="scan'208";a="385853157" Received: from 173.121.68.85.rev.sfr.net (HELO hadrien) ([85.68.121.173]) by mail3-relais-sop.national.inria.fr with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Jun 2021 23:25:28 +0200 Date: Tue, 22 Jun 2021 23:25:23 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@hadrien To: Steven Rostedt cc: Linux Trace Devel Subject: Re: [PATCH] trace-cmd split: Copy trace_clock from input handler to output handler In-Reply-To: <20210622171338.6447f199@gandalf.local.home> Message-ID: References: <20210622171338.6447f199@gandalf.local.home> User-Agent: Alpine 2.22 (DEB 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-trace-devel@vger.kernel.org On Tue, 22 Jun 2021, Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" > > When splitting a trace file, the clock is needed to create a output file > from the input file. But the output descriptor clock is never set, and the > clock returned from retrieving the descriptor is NULL (unless you are > root, in which case the code will read the machine trace clock instead, Thanks for the fix. I should have mentioned another difference between the machine where it worked and the machine where it didn't, which is that I was root on the former and not root on the latter. > and continue as if nothing was wrong). This caused a failure, and once > again, trace-cmd split, errored out without failure, but just did not > append all the data. > > Although this fixes the commit listed below, the problem was there before, > as the code before that commit tried to read the clock from the file > system, but just wouldn't error out if it couldn't read it. > > Add a new function to retrieve the trace_clock from the input handle, to > be used to pass it to the output handle before appending the CPU data. Just to confirm, is it still possible to run trace-cmd split without being root? thanks, julia > Reported-by: Julia Lawall > Fixes: 72670886 ("trace-cmd: Save only the selected clock in the trace.dat file") > Signed-off-by: Steven Rostedt (VMware) > --- > lib/trace-cmd/include/private/trace-cmd-private.h | 3 ++- > lib/trace-cmd/trace-input.c | 13 +++++++++++++ > lib/trace-cmd/trace-output.c | 2 +- > tracecmd/trace-split.c | 1 + > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h > index 7194cb30..6440084d 100644 > --- a/lib/trace-cmd/include/private/trace-cmd-private.h > +++ b/lib/trace-cmd/include/private/trace-cmd-private.h > @@ -87,7 +87,8 @@ tracecmd_plugin_context_output(struct trace_plugin_context *trace_context); > > void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet); > bool tracecmd_get_quiet(struct tracecmd_output *handle); > -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock); > +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock); > +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle); > > static inline int tracecmd_host_bigendian(void) > { > diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c > index 655bd654..ac57bc4f 100644 > --- a/lib/trace-cmd/trace-input.c > +++ b/lib/trace-cmd/trace-input.c > @@ -4075,6 +4075,19 @@ size_t tracecmd_get_options_offset(struct tracecmd_input *handle) > return handle->options_start; > } > > +/** > + * tracecmd_get_trace_clock - return the saved trace clock > + * @handle: input handle for the trace.dat file > + * > + * Returns a string of the clock that was saved in the trace.dat file. > + * The string should not be freed, as it points to the internal > + * structure data. > + */ > +const char *tracecmd_get_trace_clock(struct tracecmd_input *handle) > +{ > + return handle->trace_clock; > +} > + > /** > * tracecmd_get_show_data_func - return the show data func > * @handle: input handle for the trace.dat file > diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c > index 78a25350..a8de107c 100644 > --- a/lib/trace-cmd/trace-output.c > +++ b/lib/trace-cmd/trace-output.c > @@ -122,7 +122,7 @@ void tracecmd_set_quiet(struct tracecmd_output *handle, bool set_quiet) > handle->quiet = set_quiet; > } > > -void tracecmd_set_out_clock(struct tracecmd_output *handle, char *clock) > +void tracecmd_set_out_clock(struct tracecmd_output *handle, const char *clock) > { > if (handle && clock) > handle->trace_clock = strdup(clock); > diff --git a/tracecmd/trace-split.c b/tracecmd/trace-split.c > index 8366d128..10d0943d 100644 > --- a/tracecmd/trace-split.c > +++ b/tracecmd/trace-split.c > @@ -384,6 +384,7 @@ static double parse_file(struct tracecmd_input *handle, > for (cpu = 0; cpu < cpus; cpu ++) > cpu_list[cpu] = cpu_data[cpu].file; > > + tracecmd_set_out_clock(ohandle, tracecmd_get_trace_clock(handle)); > tracecmd_append_cpu_data(ohandle, cpus, cpu_list); > > current = end; > -- > 2.29.2 > >