linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Subject: Re: [PATCH v3 21/23] trace-cmd: Get current clock for host-guest tracing session
Date: Thu, 25 Mar 2021 07:13:55 +0200	[thread overview]
Message-ID: <CAPpZLN7Cj9Bzwmnc52jWFnGZHTAeTk+0v=oxHZ-+iJLmfJ4XEA@mail.gmail.com> (raw)
In-Reply-To: <20210324171556.47b8065f@gandalf.local.home>

On Wed, Mar 24, 2021 at 11:15 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 24 Mar 2021 15:04:16 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > In host-guest tracing session, all peers should use the same tracing
> > clock. If there is no user configured trace clock, the current logic
> > assumes "local" clock for the session. This could be wrong, as other
> > clock than "local" could be already configured on the host, before
> > running trace-cmd. The default clock for host-guest tracing session
> > should be rertieved from the host's "trace_clock" file.
>
> Actually this is wrong. If clock is not specified, you should check the
> first instance, because if the user does:
>
>  # trace-cmd start -B foo -p nop -C myclock
>
>  # trace-cmd record -B foo -e kvm -e sched -A Guest -e all
>
> It should not use the clock from the top instance, but instead the first
> instance.

Yes, the implementation uses the clock from the first non-guest
instance, the description of the patch is not correct.

>
> I'm trying to make sure tha trace-cmd does not affect or even use the top
> instance if it is not part of the command line.
>
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  tracecmd/trace-record.c | 40 ++++++++++++++++++++++++++++++----------
> >  1 file changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> > index f90fdbe4..2fc6723a 100644
> > --- a/tracecmd/trace-record.c
> > +++ b/tracecmd/trace-record.c
> > @@ -6455,11 +6455,12 @@ static void get_tsc_offset(struct common_record_context *ctx)
> >
> >  static void set_tsync_params(struct common_record_context *ctx)
> >  {
> > -     const char *clock = ctx->clock;
> >       struct buffer_instance *instance;
> >       int shift, mult;
> >       bool force_tsc = false;
> > +     char *clock = NULL;
> >
> > +     if (!ctx->clock) {
> >       /*
> >        * If no clock is configured &&
> >        * KVM time sync protocol is available &&
> > @@ -6468,18 +6469,35 @@ static void set_tsync_params(struct common_record_context *ctx)
> >        * force using the x86-tsc clock for this host-guest tracing session
> >        * and store TSC to nsec multiplier and shift.
> >        */
> > -     if (!clock && tsync_proto_is_supported("kvm") &&
> > -         clock_is_supported(NULL, TSC_CLOCK) &&
> > -         !get_tsc_nsec(&shift, &mult) && mult) {
> > -             clock = TSC_CLOCK;
> > -             ctx->tsc2nsec.mult = mult;
> > -             ctx->tsc2nsec.shift = shift;
> > -             ctx->tsc2nsec.offset = get_clock_now(TSC_CLOCK);
> > -             force_tsc = true;
> > +             if (tsync_proto_is_supported("kvm") &&
> > +                 clock_is_supported(NULL, TSC_CLOCK) &&
>
> So we want to test the first instance, not the top one.

I assume that if the top instance supports "x86-tsc" clock, then any
instance should support it also ? Is it possible that supported clocks
can be different in each instance ?

>
> -- Steve
>
> > +                 !get_tsc_nsec(&shift, &mult) && mult) {
> > +                     clock = strdup(TSC_CLOCK);
> > +                     if (!clock)
> > +                             die("Cannot not allocate clock");
> > +                     ctx->tsc2nsec.mult = mult;
> > +                     ctx->tsc2nsec.shift = shift;
> > +                     ctx->tsc2nsec.offset = get_clock_now(TSC_CLOCK);
> > +                     force_tsc = true;
> > +             } else {
> > +             /*
> > +              * Else, use the current clock of the first host instance
> > +              */
> > +                     for_all_instances(instance) {
> > +                             if (is_guest(instance))
> > +                                     continue;
> > +                             clock = tracefs_get_clock(instance->tracefs);
> > +                             break;

This is the loop that gets the clock from the first non-guest
instance, in case there is no "-C ..." option in the command line.

> > +                     }
> > +             }
> > +     } else {
> > +             clock = strdup(ctx->clock);
> > +             if (!clock)
> > +                     die("Cannot not allocate clock");
> >       }
> >
> >       if (!clock && !ctx->tsync_loop_interval)
> > -             return;
> > +             goto out;
> >       for_all_instances(instance) {
> >               if (clock && !(instance->flags & BUFFER_FL_HAS_CLOCK)) {
> >                       /* use the same clock in all tracing peers */
> > @@ -6501,6 +6519,8 @@ static void set_tsync_params(struct common_record_context *ctx)
> >               }
> >               instance->tsync_loop_interval = ctx->tsync_loop_interval;
> >       }
> > +out:
> > +     free(clock);
> >  }
> >
> >  /*
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

  reply	other threads:[~2021-03-25  5:15 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-24 13:03 [PATCH v3 00/23] TSC trace clock to nanosecond conversion Tzvetomir Stoyanov (VMware)
2021-03-24 13:03 ` [PATCH v3 01/23] trace-cmd: Add initial perf interface in trace-cmd library Tzvetomir Stoyanov (VMware)
2021-03-24 13:03 ` [PATCH v3 02/23] trace-cmd: Extend trace-cmd dump subcommand to display the clock Tzvetomir Stoyanov (VMware)
2021-03-24 17:57   ` Steven Rostedt
2021-03-24 18:02     ` Steven Rostedt
2021-03-24 13:03 ` [PATCH v3 03/23] trace-cmd: Save only the selected clock in the trace.dat file Tzvetomir Stoyanov (VMware)
2021-03-24 13:03 ` [PATCH v3 04/23] trace-cmd: Internal refactoring, move logic for local tep handler in its own function Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 05/23] trace-cmd: Add new local function to check if a trace clock is supported Tzvetomir Stoyanov (VMware)
2021-03-24 17:49   ` Steven Rostedt
2021-03-24 13:04 ` [PATCH v3 06/23] trace-cmd: Add new trace-cmd clock tsc2nsec Tzvetomir Stoyanov (VMware)
2021-03-24 15:05   ` Steven Rostedt
2021-03-24 15:20   ` Steven Rostedt
2021-03-24 15:38     ` Tzvetomir Stoyanov
2021-03-24 16:22       ` Steven Rostedt
2021-03-24 16:56         ` Tzvetomir Stoyanov
2021-03-24 20:55           ` Steven Rostedt
2021-03-24 18:32   ` Steven Rostedt
2021-03-24 18:33     ` Steven Rostedt
2021-03-24 13:04 ` [PATCH v3 07/23] trace-cmd: Define a new option for tsc2nsec conversion Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 08/23] trace-cmd: Save information for tsc to nanoseconds conversion in trace file Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 09/23] trace-cmd: Read information for tsc to nanoseconds conversion from " Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 10/23] trace-cmd: Save tsc2nsec clock in trace.dat file Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 11/23] trace-cmd: Append new options into guest trace file at the end of the tracing session Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 12/23] trace-cmd: Remove unneeded multiply in events timestamp reading Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 13/23] trace-cmd: Perform all timestamp corrections in a single function Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 14/23] trace-cmd: Convert tsc timestamps to nanosecods when reading trace data from a file Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 15/23] trace-cmd: Set order and priorities when applying timestamp corrections Tzvetomir Stoyanov (VMware)
2021-03-24 18:51   ` Steven Rostedt
2021-03-25  6:29     ` Tzvetomir Stoyanov
2021-03-24 13:04 ` [PATCH v3 16/23] trace-cmd: Add a new flag to disable any " Tzvetomir Stoyanov (VMware)
2021-03-24 14:24   ` Steven Rostedt
2021-03-24 15:20     ` Steven Rostedt
2021-03-24 13:04 ` [PATCH v3 17/23] trace-cmd: Change "--nodate" option to affect "--date" option only Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 18/23] trace-cmd: Add new parameter "--raw-ts" to "trace-cmd report" command Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 19/23] trace-cmd: Print times in TimeShift options as unsigned in trace-cmd dump Tzvetomir Stoyanov (VMware)
2021-03-24 15:33   ` Steven Rostedt
2021-03-24 13:04 ` [PATCH v3 20/23] trace-cmd: Use tsc clock for host-guest tracing, if available Tzvetomir Stoyanov (VMware)
2021-03-24 13:04 ` [PATCH v3 21/23] trace-cmd: Get current clock for host-guest tracing session Tzvetomir Stoyanov (VMware)
2021-03-24 21:15   ` Steven Rostedt
2021-03-25  5:13     ` Tzvetomir Stoyanov [this message]
2021-03-25 13:41       ` Steven Rostedt
2021-03-24 13:04 ` [PATCH v3 22/23] trace-cmd: Save the trace clocks in TRACECLOCK option Tzvetomir Stoyanov (VMware)
2021-03-24 21:24   ` Steven Rostedt
2021-03-25  5:35     ` Tzvetomir Stoyanov
2021-03-24 13:04 ` [PATCH v3 23/23] trace-cmd: Read at least 8 bytes trace-id option 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='CAPpZLN7Cj9Bzwmnc52jWFnGZHTAeTk+0v=oxHZ-+iJLmfJ4XEA@mail.gmail.com' \
    --to=tz.stoyanov@gmail.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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).