Re: [PATCH v24 03/10] trace-cmd: Add trace-cmd library APIs for ftrace clock name

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 21 Oct 2020 21:26:26 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri,  9 Oct 2020 17:03:31 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> 
> > Added enum with ftrace clock IDs and APIs to convert ftrace name to ID
> > and vice versa, as part of libtracecmd.
> > 
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  include/trace-cmd/trace-cmd.h | 16 +++++++++++++
> >  lib/trace-cmd/trace-util.c    | 45 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 61 insertions(+)
> > 
> > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> > index f9c1f843..393a2e7b 100644
> > --- a/include/trace-cmd/trace-cmd.h
> > +++ b/include/trace-cmd/trace-cmd.h
> > @@ -415,6 +415,22 @@ int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle,
> >  				unsigned int *sync_msg_id,
> >  				unsigned int *payload_size, char **payload);
> >  
> > +enum tracecmd_clocks {
> > +	TRACECMD_CLOCK_UNKNOWN	= 0,
> > +	TRACECMD_CLOCK_LOCAL	= 1,
> > +	TRACECMD_CLOCK_GLOBAL	= 1 << 1,
> > +	TRACECMD_CLOCK_COUNTER	= 1 << 2,
> > +	TRACECMD_CLOCK_UPTIME	= 1 << 3,
> > +	TRACECMD_CLOCK_PERF	= 1 << 4,
> > +	TRACECMD_CLOCK_MONO	= 1 << 5,
> > +	TRACECMD_CLOCK_MONO_RAW	= 1 << 6,
> > +	TRACECMD_CLOCK_BOOT	= 1 << 7,
> > +	TRACECMD_CLOCK_X86_TSC	= 1 << 8  
> 
> I'm curious to why you have this as a bitmask. We can only have on
> clock at a time, right?

I got to patch 5 and see that you do need this to be a bitmask.

When this is the case, the change log should state that. That is, the
change log should have something like:

The clock enum will be used in a bitmask such that the synchronization
protocol can pass a bitmask of supported clocks.

Remember, all patches should be "stand alone". That is, do not assume
that someone will have access to other patches when they are looking at
the current patch.

You may disregard the rest of this email.

-- Steve