On Sat, Jan 15, 2022 at 5:20 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Fri, 10 Dec 2021 12:54:34 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
>
> More nitpicking about spacing ;-)
>
> > Trace file version 7 format is based on sections. To fit the latency
> > trace data in this structure, a new section and option for it is
> > defined:
> > BUFFER_TEXT
>
> space
>
> > It is similar to the BUFFER section which holds the flyrecord binary
> > data, but has a latency specific design for text data. The BUFFER_TEXT
> > section has:
> > - section header, as all other sections
> > - compression of the trace data, optional
> > - corresponding trace option, pointing to the section
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx>
> > ---
> > .../include/private/trace-cmd-private.h | 1 +
> > lib/trace-cmd/trace-output.c | 24 ++++++++++++++++---
> > 2 files changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h
> > index 047fc26f..d8ee9b74 100644
> > --- a/lib/trace-cmd/include/private/trace-cmd-private.h
> > +++ b/lib/trace-cmd/include/private/trace-cmd-private.h
> > @@ -145,6 +145,7 @@ enum {
> > TRACECMD_OPTION_KALLSYMS,
> > TRACECMD_OPTION_PRINTK,
> > TRACECMD_OPTION_CMDLINES,
> > + TRACECMD_OPTION_BUFFER_TEXT,
> > TRACECMD_OPTION_MAX,
> > };
> >
> > diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
> > index 44050dc8..47227728 100644
> > --- a/lib/trace-cmd/trace-output.c
> > +++ b/lib/trace-cmd/trace-output.c
> > @@ -1874,7 +1874,9 @@ out_add_buffer_option_v7(struct tracecmd_output *handle, const char *name,
> >
> > struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, int cpus)
> > {
> > + enum tracecmd_section_flags flags = 0;
> > struct tracecmd_output *handle;
> > + tsize_t offset;
> > char *path;
> >
> > handle = tracecmd_output_create(output_file);
> > @@ -1891,7 +1893,8 @@ struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, in
> >
> > if (tracecmd_write_cpus(handle, cpus) < 0)
> > goto out_free;
> > -
> > + if (tracecmd_write_buffer_info(handle) < 0)
> > + goto out_free;
> > if (tracecmd_write_options(handle) < 0)
> > goto out_free;
> >
> > @@ -1901,23 +1904,38 @@ struct tracecmd_output *tracecmd_create_file_latency(const char *output_file, in
> > goto out_free;
> > }
> >
> > - if (do_write_check(handle, "latency ", 10))
> > + if (!HAS_SECTIONS(handle) && do_write_check(handle, "latency ", 10))
> > goto out_free;
> >
> > path = get_tracing_file(handle, "trace");
> > if (!path)
> > goto out_free;
> >
> > + offset = do_lseek(handle, 0, SEEK_CUR);
> > + if (HAS_SECTIONS(handle) &&
> > + !out_add_buffer_option_v7(handle, "", TRACECMD_OPTION_BUFFER_TEXT, offset, 0, NULL))
> > + goto out_free;
> > +
> > + offset = out_write_section_header(handle, TRACECMD_OPTION_BUFFER_TEXT,
> > + "buffer latency", flags, false);
> > +
> > copy_file(handle, path);
> > + if (out_update_section_header(handle, offset))
> > + goto out_free;
> >
> > put_tracing_file(path);
> >
> > handle->file_state = TRACECMD_FILE_CPU_LATENCY;
> >
> > + if (HAS_SECTIONS(handle))
> > + tracecmd_write_options(handle);
> > +
> > return handle;
>
>
> >
> > out_free:
> > - tracecmd_output_close(handle);
> > + if (handle)
> > + tracecmd_output_close(handle);
> > + unlink(output_file);
>
> Hmm, how does the above play a role in this patch?
>
> That is, what about this new BUFFER_TEXT required this change?
> I mean, output_file is being removed now, but I don't see anything in
> the rest of the patch to warrant that?
>
That looks like a leftover from a previous version of this patch.
> -- Steve
>
>
> > return NULL;
> > }
> >
>
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
|