linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
Cc: Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] trace-cmd: Add validation for reading and writing trace.dat files
Date: Wed, 24 Feb 2021 18:08:11 -0500	[thread overview]
Message-ID: <20210224180811.4f5accd4@gandalf.local.home> (raw)
In-Reply-To: <CAPpZLN7Ghth=9uxpu2oYAPbAHO0N2SBYbw0502fEWedEyZPzQg@mail.gmail.com>

On Wed, 24 Feb 2021 07:22:09 +0200
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> > > ---  
> >  
> > >  /* --- Opening and Reading the trace.dat file --- */
> > >
> > > +enum  {
> > > +     TRACECMD_FILE_INIT,
> > > +     TRACECMD_FILE_HEADERS,
> > > +     TRACECMD_FILE_FTRACE_EVENTS,
> > > +     TRACECMD_FILE_ALL_EVENTS,
> > > +     TRACECMD_FILE_KALLSYMS,
> > > +     TRACECMD_FILE_PRINTK,
> > > +     TRACECMD_FILE_CMD_LINES,
> > > +     TRACECMD_FILE_CPU_COUNT,
> > > +     TRACECMD_FILE_OPTIONS,
> > > +     TRACECMD_FILE_CPU_LATENCY,
> > > +     TRACECMD_FILE_CPU_FLYRECORD,  
> >
> > I still really don't think we want to make LATENCY and FLYRECORD states.
> > Because they are not a state of the trace.dat file, but a type.  
> 
> I considered these as states, as any of the previous states, that indicate
> what kind of data is currently in the file. We can replace both with a single
> TRACECMD_FILE_CPU_DATA state, and use the old TRACECMD_FL_
> flags to find what kind of tracing data is in the file.

I just realized that the flags were only used by trace-input and not
trace-output (which is what I was thinking it was). But I guess once you
get to the latency or flyrecord state, that would be the last state for
reading.

> 
> >
> > Unless we document here that they are the last states of the file, and once
> > reached, the state can not change.  
> 
> This is true for the current tarce.dat file format - they are the last states
> and as for the all other states - once reached, previously written data
> cannot be changed. May be I cannot understand your point here, may
> be you mean that once these final states are reached, the tracing data
> is still not written in the file (read from the file) ? In that case may be it
> is more logical to add additional state, to indicate that all trace data is
> in the file, regardless of its type (cpu / latency) ?

I think because the states are used for both reading and writing, I was
thinking the flags were too, but looking at the previous code, I see it's
not. My concern was for the writing of the file, and that appears to not be
an issue.

That said, there's some minor fixes in the patch I'll reply with a separate
email.

-- Steve




  reply	other threads:[~2021-02-24 23:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-19  5:31 [PATCH v2 0/2] Fix listener and add trace file validation Tzvetomir Stoyanov (VMware)
2021-02-19  5:31 ` [PATCH v2 1/2] trace-cmd: Add validation for reading and writing trace.dat files Tzvetomir Stoyanov (VMware)
2021-02-23 22:18   ` Steven Rostedt
2021-02-24  5:22     ` Tzvetomir Stoyanov
2021-02-24 23:08       ` Steven Rostedt [this message]
2021-02-24 23:18   ` Steven Rostedt
2021-02-26  4:02     ` Tzvetomir Stoyanov
2021-02-26  5:02       ` Steven Rostedt
2021-02-19  5:31 ` [PATCH v2 2/2] trace-cmd: Fix broken listener and add error checks Tzvetomir Stoyanov (VMware)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210224180811.4f5accd4@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=tz.stoyanov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).