| Message ID | 20190116191838.32127-6-tstoyanov@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | trace-cmd [POC]: Timestamps sync using PTP-like algorithm, relying on vsock events. | expand |
On Wed, 16 Jan 2019 21:18:36 +0200 Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote: > Store the user configured clock (if any) in the record context, > struct common_record_context. The clock is going to be used when > synchronizing timestamps with remote tracing machine. > --- > tracecmd/trace-record.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index f7349eb..900d0d4 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -227,6 +227,7 @@ struct common_record_context { > int topt; > int do_child; > int run_command; > + char *clock; > }; > > static void add_reset_file(const char *file, const char *val, int prio) > @@ -5183,6 +5184,7 @@ static void parse_record_options(int argc, > break; > case 'C': > ctx->instance->clock = optarg; > + ctx->clock = optarg; Hmm, this can cause an issue. The first thing I have to ask is, why can't we use ctx->instance->clock. I'm sure the answer is that we are adding more than one clock. But this begs the question, which clock should be use, if there are more than one clock to add? I think this patch is not needed. In the last patch, test if ctx->instance->clock exists, and if not, we search the other instances for a clock to use. Or something else that doesn't make which clock to use ambiguous. -- Steve > break; > case 'v': > neg_event = 1;
On Mon, Jan 21, 2019 at 11:19 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 16 Jan 2019 21:18:36 +0200 > Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote: > > > Store the user configured clock (if any) in the record context, > > struct common_record_context. The clock is going to be used when > > synchronizing timestamps with remote tracing machine. > > --- > > tracecmd/trace-record.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > > index f7349eb..900d0d4 100644 > > --- a/tracecmd/trace-record.c > > +++ b/tracecmd/trace-record.c > > @@ -227,6 +227,7 @@ struct common_record_context { > > int topt; > > int do_child; > > int run_command; > > + char *clock; > > }; > > > > static void add_reset_file(const char *file, const char *val, int prio) > > @@ -5183,6 +5184,7 @@ static void parse_record_options(int argc, > > break; > > case 'C': > > ctx->instance->clock = optarg; > > + ctx->clock = optarg; > > Hmm, this can cause an issue. The first thing I have to ask is, why > can't we use ctx->instance->clock. I'm sure the answer is that we are > adding more than one clock. But this begs the question, which clock > should be use, if there are more than one clock to add? > > I think this patch is not needed. In the last patch, test if > ctx->instance->clock exists, and if not, we search the other instances > for a clock to use. Or something else that doesn't make which clock to > use ambiguous. Ok, I'll remove it and will search for the first ctx->instance->clock instead. Current algorithm assumes that only one clock is used, and it is the same in both host and guest contexts. We should think how to handle the case with multiple instances, each one with different clock. I do not know, how instances are written in the trace.dat file, is there a different time offset option in the file, per instance ? > > -- Steve > > > break; > > case 'v': > > neg_event = 1; >
On Tue, Jan 22, 2019 at 3:44 PM Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote: > > On Mon, Jan 21, 2019 at 11:19 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Wed, 16 Jan 2019 21:18:36 +0200 > > Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote: > > > > > Store the user configured clock (if any) in the record context, > > > struct common_record_context. The clock is going to be used when > > > synchronizing timestamps with remote tracing machine. > > > --- > > > tracecmd/trace-record.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > > > index f7349eb..900d0d4 100644 > > > --- a/tracecmd/trace-record.c > > > +++ b/tracecmd/trace-record.c > > > @@ -227,6 +227,7 @@ struct common_record_context { > > > int topt; > > > int do_child; > > > int run_command; > > > + char *clock; > > > }; > > > > > > static void add_reset_file(const char *file, const char *val, int prio) > > > @@ -5183,6 +5184,7 @@ static void parse_record_options(int argc, > > > break; > > > case 'C': > > > ctx->instance->clock = optarg; > > > + ctx->clock = optarg; > > > > Hmm, this can cause an issue. The first thing I have to ask is, why > > can't we use ctx->instance->clock. I'm sure the answer is that we are > > adding more than one clock. But this begs the question, which clock > > should be use, if there are more than one clock to add? > > > > I think this patch is not needed. In the last patch, test if > > ctx->instance->clock exists, and if not, we search the other instances > > for a clock to use. Or something else that doesn't make which clock to > > use ambiguous. > > Ok, I'll remove it and will search for the first ctx->instance->clock instead. > Current algorithm assumes that only one clock is used, and it is the > same in both > host and guest contexts. We should think how to handle the case with > multiple instances, > each one with different clock. I do not know, how instances are > written in the trace.dat file, > is there a different time offset option in the file, per instance ? Each instance goes into a separate file (by default "trace-$(name).dat"). For guest instances, this command $ trace-cmd record -A guest0 -e irq -e sched -A guest1 -e irq -e sched creates two trace files: -rw-r--r-- 1 4.8M Jan 22 16:06 trace-guest0.dat -rw-r--r-- 1 7.4M Jan 22 16:06 trace-guest1.dat Cheers, - Slavi
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index f7349eb..900d0d4 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -227,6 +227,7 @@ struct common_record_context { int topt; int do_child; int run_command; + char *clock; }; static void add_reset_file(const char *file, const char *val, int prio) @@ -5183,6 +5184,7 @@ static void parse_record_options(int argc, break; case 'C': ctx->instance->clock = optarg; + ctx->clock = optarg; break; case 'v': neg_event = 1;