Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH v24 03/10] trace-cmd: Add trace-cmd library APIs for ftrace clock name
Date: Wed, 21 Oct 2020 21:31:35 -0400
Message-ID: <20201021213135.2022f4e0@oasis.local.home> (raw)
In-Reply-To: <20201021212626.1c3436c4@oasis.local.home>

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

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-09 14:03 [PATCH v24 00/10] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 01/10] trace-cmd: [POC] PTP-like algorithm for host - guest timestamp synchronization Tzvetomir Stoyanov (VMware)
2020-10-15 21:24   ` Steven Rostedt
2020-10-26  7:38     ` Tzvetomir Stoyanov
2020-10-09 14:03 ` [PATCH v24 02/10] trace-cmd: Debug scripts for " Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 03/10] trace-cmd: Add trace-cmd library APIs for ftrace clock name Tzvetomir Stoyanov (VMware)
2020-10-22  1:26   ` Steven Rostedt
2020-10-22  1:31     ` Steven Rostedt [this message]
2020-10-09 14:03 ` [PATCH v24 04/10] trace-cmd: Move VM related logic in a separate file Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 05/10] trace-cmd: Add clock parameter to timestamp synchronization plugins Tzvetomir Stoyanov (VMware)
2020-10-22  1:39   ` Steven Rostedt
2020-10-09 14:03 ` [PATCH v24 06/10] trace-cmd: Add role " Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 07/10] trace-cmd: Add host / guest role in timestamp synchronization context Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 08/10] trace-cmd: Add guest CPU count PID in tracecmd_time_sync struct Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 09/10] trace-cmd: Fixed bitmask logic tracecmd_tsync_proto_getall() Tzvetomir Stoyanov (VMware)
2020-10-09 14:03 ` [PATCH v24 10/10] trace-cmd [POC]: Add KVM timestamp synchronization plugin 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=20201021213135.2022f4e0@oasis.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

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git