diff mbox series

[2/7] trace-cmd: Added new option in trace.dat file for negative time offsets

Message ID 20190116191838.32127-3-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

Commit Message

Tzvetomir Stoyanov Jan. 16, 2019, 7:18 p.m. UTC 
When synchronizing timestamps between different machines, there are cases
when the time offset is negative. A new option, TRACECMD_OPTION_OFFSET_N,
is added in trace.dat file for such use cases. The format is the same as
TRACECMD_OPTION_OFFSET, but the value is interpreted as negative integer.

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 include/trace-cmd/trace-cmd.h |   1 +
 lib/trace-cmd/trace-input.c   |   7 +-
 tracecmd/trace-record.c       | 123 +++++++++++++++++-----------------
 3 files changed, 70 insertions(+), 61 deletions(-)

Comments

Steven Rostedt Jan. 21, 2019, 9:08 a.m. UTC | #1 
On Wed, 16 Jan 2019 21:18:33 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index b628570..de15d4b 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -2151,6 +2151,7 @@ static int handle_options(struct tracecmd_input *handle)
>  			handle->ts_offset += offset;
>  			break;
>  		case TRACECMD_OPTION_OFFSET:
> +		case TRACECMD_OPTION_OFFSET_N:
>  			/*
>  			 * Similar to date option, but just adds an
>  			 * offset to the timestamp.
> @@ -2158,7 +2159,11 @@ static int handle_options(struct tracecmd_input *handle)
>  			if (handle->flags & TRACECMD_FL_IGNORE_DATE)
>  				break;
>  			offset = strtoll(buf, NULL, 0);
> -			handle->ts_offset += offset;
> +			if (option == TRACECMD_OPTION_OFFSET_N) {

Instead of adding a new option, couldn't we just make option a "long
long" and save a negative number in the trace.dat file? strtoll()
should be able to handle it.

-- Steve


> +				if (handle->ts_offset > offset)
> +					handle->ts_offset -= offset;
> +			} else
> +				handle->ts_offset += offset;
>  			break;
>  		case TRACECMD_OPTION_CPUSTAT:
>  			buf[size-1] = '\n';
Tzvetomir Stoyanov Jan. 22, 2019, 1:28 p.m. UTC | #2 
On Mon, Jan 21, 2019 at 11:08 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 16 Jan 2019 21:18:33 +0200
> Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:
>
> > diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> > index b628570..de15d4b 100644
> > --- a/lib/trace-cmd/trace-input.c
> > +++ b/lib/trace-cmd/trace-input.c
> > @@ -2151,6 +2151,7 @@ static int handle_options(struct tracecmd_input *handle)
> >                       handle->ts_offset += offset;
> >                       break;
> >               case TRACECMD_OPTION_OFFSET:
> > +             case TRACECMD_OPTION_OFFSET_N:
> >                       /*
> >                        * Similar to date option, but just adds an
> >                        * offset to the timestamp.
> > @@ -2158,7 +2159,11 @@ static int handle_options(struct tracecmd_input *handle)
> >                       if (handle->flags & TRACECMD_FL_IGNORE_DATE)
> >                               break;
> >                       offset = strtoll(buf, NULL, 0);
> > -                     handle->ts_offset += offset;
> > +                     if (option == TRACECMD_OPTION_OFFSET_N) {
>
> Instead of adding a new option, couldn't we just make option a "long
> long" and save a negative number in the trace.dat file? strtoll()
> should be able to handle it.

Yes,  we can make in long long, but this will break the backward compatibility.
The old versions of  trace-cmd will get confused, as they will cast it
to unsigned.
That's why I added a new option, in this way old trace-cmd versions
will not read the
negative offset at all.

>
> -- Steve
>
>
> > +                             if (handle->ts_offset > offset)
> > +                                     handle->ts_offset -= offset;
> > +                     } else
> > +                             handle->ts_offset += offset;
> >                       break;
> >               case TRACECMD_OPTION_CPUSTAT:
> >                       buf[size-1] = '\n';
diff mbox series

Patch

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index c79fc63..d0bddfc 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -82,6 +82,7 @@  enum {
 	TRACECMD_OPTION_HOOK,
 	TRACECMD_OPTION_OFFSET,
 	TRACECMD_OPTION_CPUCOUNT,
+	TRACECMD_OPTION_OFFSET_N,
 };
 
 enum {
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index b628570..de15d4b 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -2151,6 +2151,7 @@  static int handle_options(struct tracecmd_input *handle)
 			handle->ts_offset += offset;
 			break;
 		case TRACECMD_OPTION_OFFSET:
+		case TRACECMD_OPTION_OFFSET_N:
 			/*
 			 * Similar to date option, but just adds an
 			 * offset to the timestamp.
@@ -2158,7 +2159,11 @@  static int handle_options(struct tracecmd_input *handle)
 			if (handle->flags & TRACECMD_FL_IGNORE_DATE)
 				break;
 			offset = strtoll(buf, NULL, 0);
-			handle->ts_offset += offset;
+			if (option == TRACECMD_OPTION_OFFSET_N) {
+				if (handle->ts_offset > offset)
+					handle->ts_offset -= offset;
+			} else
+				handle->ts_offset += offset;
 			break;
 		case TRACECMD_OPTION_CPUSTAT:
 			buf[size-1] = '\n';
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index cd80462..f7349eb 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -201,6 +201,13 @@  enum trace_cmd {
 	CMD_record_agent,
 };
 
+enum {
+	DATA_FL_NONE		= 0,
+	DATA_FL_DATE		= 1,
+	DATA_FL_OFFSET		= 2,
+	DATA_FL_OFFSET_N	= 4,
+};
+
 struct common_record_context {
 	enum trace_cmd curr_cmd;
 	struct buffer_instance *instance;
@@ -3230,7 +3237,62 @@  again:
 	return msg_handle;
 }
 
-static void add_options(struct tracecmd_output *handle, struct common_record_context *ctx);
+static void add_option_hooks(struct tracecmd_output *handle)
+{
+	struct hook_list *hook;
+	int len;
+
+	for (hook = hooks; hook; hook = hook->next) {
+		len = strlen(hook->hook);
+		tracecmd_add_option(handle, TRACECMD_OPTION_HOOK,
+				    len + 1, hook->hook);
+	}
+}
+
+static void add_uname(struct tracecmd_output *handle)
+{
+	struct utsname buf;
+	char *str;
+	int len;
+	int ret;
+
+	ret = uname(&buf);
+	/* if this fails for some reason, just ignore it */
+	if (ret < 0)
+		return;
+
+	len = strlen(buf.sysname) + strlen(buf.nodename) +
+		strlen(buf.release) + strlen(buf.machine) + 4;
+	str = malloc(len);
+	if (!str)
+		return;
+	sprintf(str, "%s %s %s %s", buf.sysname, buf.nodename, buf.release, buf.machine);
+	tracecmd_add_option(handle, TRACECMD_OPTION_UNAME, len, str);
+	free(str);
+}
+
+static void add_options(struct tracecmd_output *handle,
+			struct common_record_context *ctx)
+{
+	int type = 0;
+
+	if (ctx->date2ts) {
+		if (ctx->data_flags & DATA_FL_DATE)
+			type = TRACECMD_OPTION_DATE;
+		else if (ctx->data_flags & DATA_FL_OFFSET)
+			type = TRACECMD_OPTION_OFFSET;
+		else if (ctx->data_flags & DATA_FL_OFFSET_N)
+			type = TRACECMD_OPTION_OFFSET_N;
+	}
+
+	if (type)
+		tracecmd_add_option(handle, type,
+				    strlen(ctx->date2ts)+1, ctx->date2ts);
+
+	tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
+	add_option_hooks(handle);
+	add_uname(handle);
+}
 
 static struct tracecmd_msg_handle *
 setup_connection(struct buffer_instance *instance, struct common_record_context *ctx)
@@ -3457,40 +3519,6 @@  add_buffer_stat(struct tracecmd_output *handle, struct buffer_instance *instance
 				    instance->s_save[i].buffer);
 }
 
-static void add_option_hooks(struct tracecmd_output *handle)
-{
-	struct hook_list *hook;
-	int len;
-
-	for (hook = hooks; hook; hook = hook->next) {
-		len = strlen(hook->hook);
-		tracecmd_add_option(handle, TRACECMD_OPTION_HOOK,
-				    len + 1, hook->hook);
-	}
-}
-
-static void add_uname(struct tracecmd_output *handle)
-{
-	struct utsname buf;
-	char *str;
-	int len;
-	int ret;
-
-	ret = uname(&buf);
-	/* if this fails for some reason, just ignore it */
-	if (ret < 0)
-		return;
-
-	len = strlen(buf.sysname) + strlen(buf.nodename) +
-		strlen(buf.release) + strlen(buf.machine) + 4;
-	str = malloc(len);
-	if (!str)
-		return;
-	sprintf(str, "%s %s %s %s", buf.sysname, buf.nodename, buf.release, buf.machine);
-	tracecmd_add_option(handle, TRACECMD_OPTION_UNAME, len, str);
-	free(str);
-}
-
 static void print_stat(struct buffer_instance *instance)
 {
 	int cpu;
@@ -3505,31 +3533,6 @@  static void print_stat(struct buffer_instance *instance)
 		trace_seq_do_printf(&instance->s_print[cpu]);
 }
 
-enum {
-	DATA_FL_NONE		= 0,
-	DATA_FL_DATE		= 1,
-	DATA_FL_OFFSET		= 2,
-};
-
-static void add_options(struct tracecmd_output *handle, struct common_record_context *ctx)
-{
-	int type = 0;
-
-	if (ctx->date2ts) {
-		if (ctx->data_flags & DATA_FL_DATE)
-			type = TRACECMD_OPTION_DATE;
-		else if (ctx->data_flags & DATA_FL_OFFSET)
-			type = TRACECMD_OPTION_OFFSET;
-	}
-
-	if (type)
-		tracecmd_add_option(handle, type, strlen(ctx->date2ts)+1, ctx->date2ts);
-
-	tracecmd_add_option(handle, TRACECMD_OPTION_TRACECLOCK, 0, NULL);
-	add_option_hooks(handle);
-	add_uname(handle);
-}
-
 static void write_guest_file(struct buffer_instance *instance)
 {
 	struct tracecmd_output *handle;