All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session
@ 2020-10-29 11:18 Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string Tzvetomir Stoyanov (VMware)
                   ` (15 more replies)
  0 siblings, 16 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Basic infrastructure for host - guest timestamp synchronization and a
PoC implementation of PTP-like and KVM algorithms. 

The KVM plugin is with higher weight than PTP, it is prefered in case both
are active. KVM is used only if x86-tsc clock is selected for the current
trace, PTP is used for all other cases.

[
 v25 changes:
  - Addressed Steven's comments.
  - Reorded the patches in the plugin - put the PTP and KVM PoC patches
    last and all other fixes and improvements before them.
  - Implemented a new libtrasefs API to get the current trace clock.
  - Replaced IDs of trace timestamps synchronization protocols with strings.
  - Implemented an "accuracy" parameter for timestamps synchronization
    protocols.
  - Added "scaling ratio" correction, calculated by the timestamp
    synchronization protocols.
  - Added flags, declared by the timestamps synchronization protocols,
    and stored in the trace.dat file, together with the calculated
    time offsets.
  - Implemented timestamp offset calculation per virtual CPU.
 v24 changes:
  - trace-cmd internal refactoring, needed for KVM timestamp synchronization
    plugin.
  - Added KVM timestamp synchronization plugin.

 v23 changes:
  - Added flags to PTP start message to control the behaviour of the
    algorithm. This is for development purposes mostly, to compare and
    evaluate how these changes affect the accuracy.
	PTP_FLAG_FASTEST_RESPONSE:  Consider only the probe with fastest
				    response time, otherwise make a histogram
				    from all probes.
	PTP_FLAG_USE_MARKER: Use trace marker to get the clock,
 			     otherwise use directly the system clock

 v22 changes, addressed Steven's comments:
  - Fixed error flow in read_qemu_guests_pids(), free allocated resources
    in case of an error.
  - Fixed initialisation of reallocated memmory in set_vcpu_pid_mapping().
  - Fixed bitmask logic in tracecmd_tsync_proto_select().
  - Fixed usage of wrong structure in make_trace_resp().

 v21 changes:
  - Rebased on top of latest master.
  - Remove these two patches from the set, as they are already merged:
       trace-cmd: Add new library API for local CPU count
       trace-cmd: Add support for negative time offsets in trace.dat file
  - Added more descriptive warning message when fails to extract Time Shift
    information from the trace.dat file.
  - Added a warning message when fails to obtain vcpu - pid mappings.
  - Handle the case with sparse VCPU numbers in VCPU - PID mapping
    array.
  - Fixed broken recording over FIFOs.
 v20 changes:
  - Rebased on top of latest master.
  - Removed the VCPUS_MAX hardcoded limit, reimplemented the cpu_pid[] array
    to be dynamically allocated.
  - Added a warning when reading of TRACECMD_OPTION_TIME_SHIFT option
    fails, due to unexpected option's size.
  - Improved loading of TRACECMD_OPTION_GUEST option data, as Steven
    suggested.
 v19 changes:
  - Rebased on top of latest master. The librtacefs is already merged,
    this allows to drop few patches from the set and use librtacefs APIs
    instead.
  - Reimplemented all new trace.dat options to be in binary format,
    instead of text. Leverage the new "trace-cmd dump" sub command to
    track what is written in the file.
  - Addressed Steven's comments.

 v18 changes: addressed Steven Rostdet comments:
  - Replaced semaphores with pthread mutexes.
  - Made bitmask with time sync protocols unlimited, so we can support
    more than 32 protocols. That required to redesign the trace request
    packet format.
  - A lot of small fixes.
	
 v17 changes:
  - Implemented new PTP logic for calculating the clocks offset, using
    histogram of all PTP samples. It gives better results than the logic
    with the fastest response time, so set the new one as default in the
    POC patch.

 v16 changes:
  - Fixed compilation in case no VSOCK is available (Thanks to Slavomir Kaslev)
  - Fixed a typo in trace-cmd-record.1.txt (Thanks to Slavomir Kaslev)
  - Added forgotten file in the patch "trace-cmd: Add new library APIs for
    ftrace instances." - trace-instance.c
  - Fixed few compilation warnings related to TSYNC_DEBUG code.
  - Removed a blank line at the end of "tsync_readme" file. 

 v15 changes:
  - Removed the patch for "--proc-map" from the series, as it should not
    be part of it.

 v14 changes:
  - Bring back the PTP-like algorithm and removed the ftrace event based logic.
  - Reimplemented the PTP-like algorithm to use raw ftrace markers, instead of clock_gettime() API.
  - Refactored the logic to be algorithm independent and plugin friendly.
  - Implemented continuous timestamps synchronization, while the trace is running.
  - Moved logic from trace-cmd application to libtracecmd, as new library APIs.
  - Implemented new trace id functionality.
  - Implemented new guest section in host trace.dat file.
	
 v13 changes:
  - Remove few patches from the set, as they were merged.
  - Rebased to the latest master, Slavomirs patchest "Add VM kernel tracing over
    vsockets and FIFOs" got merged!
 	
 v12 changes:
  - Rebased on top of Slavomir's v13 "Add VM kernel tracing over vsockets and FIFOs"

 v11 changes:
  - Rebased on top of Slavomir's v10 "Add VM kernel tracing over vsockets and FIFOs"
  - Addressed Slavomir's commnents from version 10 of the patch series.

 v10 changes:
  - Fixed broken compilation, call to timestamp_correction_calc() in timestamp_correct
    was smashed.
  - Replaced deprecated tep_data_event_from_type() API with tep_find_event().
  - Fixed a warning on assignment const to non const.

 v9 changes:
  - Fixed implementation of binary search algorithm in timestamp_correct()

 v8 changes:
  - Added rmdir() call in tracecmd_remove_instance(), to completely remove the instance. 
  However, there is an issue with deleting the instances using rmdir(), which is investigated.
  - Few changes in read_qemu_guests_pids(), timestamp_correct(), tsync_offset_load() 
 tracecmd_clock_context_new() and find_raw_events() suggested by Slavomir. 

 v7 changes:
  - Added warning messages in case time synchronization cannot be negotiated or fails.
  - Few optimizations and checks in read_qemu_guests_pids(), tsync_offset_load(),
    and find_raw_events(), suggested by Slavomir Kaslev.
  - Reworked timestamp_correct() to not use static variables.
  - Check TRACECMD_OPTION_TIME_SHIFT before reading time sync samples from the trace.dat file

 v6 changes:
  - Refactored tracecmd_msg_snd_time_sync() and tracecmd_msg_rcv_time_sync() functions:
    removed any time sync calculations logic as separate functions in trace-timesync.c file
  - Defined TSYNC_PROBE, TSYNC_REQ and TSYNC_RESP messages, in order to make the time sync
    protocol comprehensible.
  - Addressed Steven Rostedt comments.
  - Addressed Slavomir Kaslev commnets.

v5 changes:
  - Rebased to Slavomir's v8 "Add VM kernel tracing over vsockets and FIFOs"
    patch series.
  - Implemented an algorithm for time drift correction.
  - Addressed Slavomir's commnets.
  - Refactored the code: moved all time sync specific implementation in trace-timesync.c
  - Isolated all hardcoded event specific stuff in a structure, so it could be easily
    moved to external plugins.
  - Added a check for VSOCK support: do not perform vsock dependent time synchronisation
    in case there is no VSOCK support.

 v4 changes:
  - Removed the implementation of PTP-like algorithm. The current
    logic relies on matching time stamps of kvm_exit/virtio_transport_recv_pkt
    events on host to virtio_transport_alloc_pkt/vp_notify events on guest.
  - Rebased to Slavomir's v7 "Add VM kernel tracing over vsockets and FIFOs"
    patch series.
  - Decreased the time synch probes from 5000 to 300.
  - Addressed Steven Rostedt comments.
  - Code cleanup.

 v3 changes:
 - Removed any magic constants, used in the PTP-like algorithm,
   as Slavomir Kaslev suggested.
 - Implemented new algorithm, based on mapping kvm_exit events
   in host context to vsock_send events in guest context,
   suggested by Steven Rostedt.

 v2 changes:
  - Addressed Steven Rostedt comments.
  - Modified PTP-like timestamps sync algorithm to gain more accuracy, with the
    help of Yordan Karadzhov and Slavomir Kaslev.
]

Tzvetomir Stoyanov (VMware) (16):
  trace-cmd: Replace time sync protocol ID with string
  trace-cmd: Add trace-cmd library APIs for ftrace clock name
  trace-cmd: Move VM related logic in a separate file
  trace-cmd: Add new libtrasefs API to get the current trace clock
  trace-cmd: Add clock parameter to timestamp synchronization plugins
  trace-cmd: Add role parameter to timestamp synchronization plugins
  trace-cmd: Add host / guest role in timestamp synchronization context
  trace-cmd: Add guest CPU count PID in tracecmd_time_sync struct
  trace-cmd: Add scaling ratio for timestamp correction
  trace-cmd: Add time sync protocol flags
  trace-cmd: Add timestamp synchronization per vCPU
  trace-cmd: Define a macro for packed structures
  trace-cmd: Add dummy function to initialize timestamp sync logic
  trace-cmd: [POC] PTP-like algorithm for host - guest timestamp
    synchronization
  trace-cmd: Debug scripts for PTP-like algorithm for host - guest
    timestamp synchronization
  trace-cmd [POC]: Add KVM timestamp synchronization plugin

 include/trace-cmd/trace-cmd.h             |  67 +-
 include/tracefs/tracefs.h                 |   2 +
 lib/trace-cmd/Makefile                    |   2 +
 lib/trace-cmd/include/trace-cmd-local.h   |   2 +
 lib/trace-cmd/include/trace-tsync-local.h |  38 +-
 lib/trace-cmd/trace-input.c               | 205 +++++--
 lib/trace-cmd/trace-msg.c                 | 116 ++--
 lib/trace-cmd/trace-timesync-kvm.c        | 459 ++++++++++++++
 lib/trace-cmd/trace-timesync-ptp.c        | 714 ++++++++++++++++++++++
 lib/trace-cmd/trace-timesync.c            | 421 +++++++++----
 lib/trace-cmd/trace-util.c                |  52 ++
 lib/tracefs/tracefs-instance.c            |  35 ++
 scripts/debug/tsync_hist.py               |  57 ++
 scripts/debug/tsync_readme                |  12 +
 scripts/debug/tsync_res.py                |  46 ++
 tracecmd/Makefile                         |   1 +
 tracecmd/include/trace-local.h            |  25 +-
 tracecmd/trace-agent.c                    |  10 +-
 tracecmd/trace-dump.c                     |  52 +-
 tracecmd/trace-record.c                   | 283 ++-------
 tracecmd/trace-stat.c                     |  29 +-
 tracecmd/trace-tsync.c                    |  96 ++-
 tracecmd/trace-vm.c                       | 214 +++++++
 23 files changed, 2365 insertions(+), 573 deletions(-)
 create mode 100644 lib/trace-cmd/trace-timesync-kvm.c
 create mode 100644 lib/trace-cmd/trace-timesync-ptp.c
 create mode 100644 scripts/debug/tsync_hist.py
 create mode 100644 scripts/debug/tsync_readme
 create mode 100644 scripts/debug/tsync_res.py
 create mode 100644 tracecmd/trace-vm.c

-- 
2.26.2


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-11-05 14:52   ` Steven Rostedt
  2020-12-02 22:43   ` Steven Rostedt
  2020-10-29 11:18 ` [PATCH v25 02/16] trace-cmd: Add trace-cmd library APIs for ftrace clock name Tzvetomir Stoyanov (VMware)
                   ` (14 subsequent siblings)
  15 siblings, 2 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Different timestamp synchronization algorithms will be implemented as
trace-cmd plugins. Using IDs for identifying these algorithms is
a problem, as these IDs must be defined in a single place. In order
to be more flexible, protocol IDs are replaced by protocol names -
strings that are part of the plugin code and are registered with
plugin initialisation.
The plugin names are limited up to 15 symbols, in order to simplify
the structure of sync messages.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h             |  32 +++---
 lib/trace-cmd/include/trace-tsync-local.h |  12 +-
 lib/trace-cmd/trace-msg.c                 | 102 ++++++++++------
 lib/trace-cmd/trace-timesync.c            | 134 ++++++++++------------
 tracecmd/include/trace-local.h            |   5 +-
 tracecmd/trace-agent.c                    |   8 +-
 tracecmd/trace-record.c                   |  19 +--
 tracecmd/trace-tsync.c                    |  19 ++-
 8 files changed, 170 insertions(+), 161 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index f3c95f30..8d3bea73 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -385,50 +385,44 @@ void tracecmd_msg_set_done(struct tracecmd_msg_handle *msg_handle);
 int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
 				int argc, char **argv, bool use_fifos,
 				unsigned long long trace_id,
-				char *tsync_protos,
-				int tsync_protos_size);
+				char **tsync_protos);
 int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
 				int *argc, char ***argv, bool *use_fifos,
 				unsigned long long *trace_id,
-				char **tsync_protos,
-				unsigned int *tsync_protos_size);
+				char ***tsync_protos);
 
 int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int nr_cpus, int page_size,
 				 unsigned int *ports, bool use_fifos,
 				 unsigned long long trace_id,
-				 unsigned int tsync_proto,
-				 unsigned int tsync_port);
+				 char *tsync_proto, unsigned int tsync_port);
 int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int *nr_cpus, int *page_size,
 				 unsigned int **ports, bool *use_fifos,
 				 unsigned long long *trace_id,
-				 unsigned int *tsync_proto,
+				 char **tsync_proto,
 				 unsigned int *tsync_port);
 
 int tracecmd_msg_send_time_sync(struct tracecmd_msg_handle *msg_handle,
-				unsigned int sync_protocol,
-				unsigned int sync_msg_id,
+				char *sync_protocol, unsigned int sync_msg_id,
 				unsigned int payload_size, char *payload);
 int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle,
-				unsigned int *sync_protocol,
+				char *sync_protocol,
 				unsigned int *sync_msg_id,
 				unsigned int *payload_size, char **payload);
 
 /* --- Timestamp synchronization --- */
 
-enum{
-	TRACECMD_TIME_SYNC_PROTO_NONE	= 0,
-};
+#define TRACECMD_TSYNC_PNAME_LENGTH	16
+#define TRACECMD_TSYNC_PROTO_NONE	"none"
+
 enum{
 	TRACECMD_TIME_SYNC_CMD_PROBE	= 1,
 	TRACECMD_TIME_SYNC_CMD_STOP	= 2,
 };
 
-#define TRACECMD_TIME_SYNC_PROTO_PTP_WEIGHT	10
-
 struct tracecmd_time_sync {
-	unsigned int			sync_proto;
+	char				*proto_name;
 	int				loop_interval;
 	pthread_mutex_t			lock;
 	pthread_cond_t			cond;
@@ -438,9 +432,9 @@ struct tracecmd_time_sync {
 };
 
 void tracecmd_tsync_init(void);
-int tracecmd_tsync_proto_getall(char **proto_mask, int *words);
-unsigned int tracecmd_tsync_proto_select(char *proto_mask, int words);
-bool tsync_proto_is_supported(unsigned int proto_id);
+int tracecmd_tsync_proto_getall(char ***protos);
+char *tracecmd_tsync_proto_select(char **protos);
+bool tsync_proto_is_supported(char *proto_name);
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
 void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
 int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 1de9d5e5..1f3bc443 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -26,12 +26,12 @@ struct clock_sync_context {
 	unsigned int			remote_port;
 };
 
-int tracecmd_tsync_proto_register(unsigned int proto_id, int weight,
-				int (*init)(struct tracecmd_time_sync *),
-				int (*free)(struct tracecmd_time_sync *),
-				int (*calc)(struct tracecmd_time_sync *,
-					    long long *, long long *));
-int tracecmd_tsync_proto_unregister(unsigned int proto_id);
+int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
+				  int (*init)(struct tracecmd_time_sync *),
+				  int (*free)(struct tracecmd_time_sync *),
+				  int (*calc)(struct tracecmd_time_sync *,
+					      long long *, long long *));
+int tracecmd_tsync_proto_unregister(char *proto_name);
 
 int ptp_clock_sync_register(void);
 
diff --git a/lib/trace-cmd/trace-msg.c b/lib/trace-cmd/trace-msg.c
index 4a0bfa93..59ff33c8 100644
--- a/lib/trace-cmd/trace-msg.c
+++ b/lib/trace-cmd/trace-msg.c
@@ -26,6 +26,7 @@
 #include "trace-cmd-local.h"
 #include "trace-local.h"
 #include "trace-msg.h"
+#include "trace-cmd.h"
 
 typedef __u32 u32;
 typedef __be32 be32;
@@ -85,12 +86,12 @@ struct tracecmd_msg_trace_resp {
 	be32 cpus;
 	be32 page_size;
 	u64 trace_id;
-	be32 tsync_proto;
+	char tsync_proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
 	be32 tsync_port;
 } __attribute__((packed));
 
 struct tracecmd_msg_tsync {
-	be32 sync_protocol;
+	char sync_protocol_name[TRACECMD_TSYNC_PNAME_LENGTH];
 	be32 sync_msg_id;
 } __attribute__((packed));
 
@@ -847,13 +848,20 @@ int tracecmd_msg_wait_close_resp(struct tracecmd_msg_handle *msg_handle)
 	return tracecmd_msg_wait_for_cmd(msg_handle, MSG_CLOSE_RESP);
 }
 
-static int make_trace_req_protos(char **buf, int *size,
-				 int protos_size, char *tsync_protos)
+static int make_trace_req_protos(char **buf, int *size, char **tsync_protos)
 {
+	int protos_size = 1;
 	size_t buf_size;
+	char **protos;
 	char *nbuf;
 	char *p;
 
+	protos = tsync_protos;
+	while (*protos) {
+		protos_size += strlen(*protos) + 1;
+		protos++;
+	}
+
 	buf_size = TRACE_REQ_PARAM_SIZE + protos_size;
 	nbuf = realloc(*buf, *size + buf_size);
 	if (!nbuf)
@@ -867,7 +875,13 @@ static int make_trace_req_protos(char **buf, int *size,
 	*(unsigned int *)p = htonl(protos_size);
 	p += sizeof(int);
 
-	memcpy(p, tsync_protos, protos_size);
+	protos = tsync_protos;
+	while (*protos) {
+		memcpy(p, *protos, strlen(*protos) + 1);
+		p += strlen(*protos) + 1;
+		protos++;
+	}
+	p = NULL;
 
 	*size += buf_size;
 	*buf = nbuf;
@@ -911,7 +925,7 @@ static int make_trace_req_args(char **buf, int *size, int argc, char **argv)
 
 static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv,
 			  bool use_fifos, unsigned long long trace_id,
-			  char *tsync_protos, int tsync_protos_size)
+			  char **tsync_protos)
 {
 	int size = 0;
 	char *buf = NULL;
@@ -924,9 +938,8 @@ static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv,
 
 	if (argc && argv)
 		make_trace_req_args(&buf, &size, argc, argv);
-	if (tsync_protos_size && tsync_protos)
-		make_trace_req_protos(&buf, &size,
-				      tsync_protos_size, tsync_protos);
+	if (tsync_protos)
+		make_trace_req_protos(&buf, &size, tsync_protos);
 
 	msg->buf = buf;
 	msg->hdr.size = htonl(ntohl(msg->hdr.size) + size);
@@ -937,30 +950,45 @@ static int make_trace_req(struct tracecmd_msg *msg, int argc, char **argv,
 int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
 				int argc, char **argv, bool use_fifos,
 				unsigned long long trace_id,
-				char *tsync_protos,
-				int tsync_protos_size)
+				char **tsync_protos)
 {
 	struct tracecmd_msg msg;
 	int ret;
 
 	tracecmd_msg_init(MSG_TRACE_REQ, &msg);
-	ret = make_trace_req(&msg, argc, argv, use_fifos,
-			     trace_id, tsync_protos, tsync_protos_size);
+	ret = make_trace_req(&msg, argc, argv, use_fifos, trace_id, tsync_protos);
 	if (ret < 0)
 		return ret;
 
 	return tracecmd_msg_send(msg_handle->fd, &msg);
 }
 
-static int get_trace_req_protos(char *buf, int length,
-				char **tsync_protos,
-				unsigned int *tsync_protos_size)
+static int get_trace_req_protos(char *buf, int length, char ***tsync_protos)
 {
-	*tsync_protos = malloc(length);
-	if (!*tsync_protos)
+	int count = 0;
+	char *p;
+	int i, j;
+
+	i = length;
+	p = buf;
+	while (i > 0) {
+		i -= strlen(p) + 1;
+		count++;
+		p -= strlen(p) + 1;
+	}
+
+	*tsync_protos = calloc(count + 1, sizeof(char *));
+	if (!(*tsync_protos))
 		return -1;
-	memcpy(*tsync_protos, buf, length);
-	*tsync_protos_size = length;
+
+	i = length;
+	p = buf;
+	j = 0;
+	while (i > 0 && j < (count - 1)) {
+		i -= strlen(p) + 1;
+		(*tsync_protos)[j++] = strdup(p);
+		p -= strlen(p) + 1;
+	}
 
 	return 0;
 }
@@ -1026,8 +1054,7 @@ out:
 int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
 				int *argc, char ***argv, bool *use_fifos,
 				unsigned long long *trace_id,
-				char **tsync_protos,
-				unsigned int *tsync_protos_size)
+				char ***tsync_protos)
 {
 	struct tracecmd_msg msg;
 	unsigned int param_id;
@@ -1069,8 +1096,7 @@ int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
 			ret = get_trace_req_args(p, param_length, argc, argv);
 			break;
 		case TRACE_REQUEST_TSYNC_PROTOS:
-			ret = get_trace_req_protos(p, param_length,
-						   tsync_protos, tsync_protos_size);
+			ret = get_trace_req_protos(p, param_length, tsync_protos);
 			break;
 		default:
 			break;
@@ -1095,7 +1121,8 @@ out:
 /**
  * tracecmd_msg_send_time_sync - Send a time sync packet
  * @msg_handle: message handle, holding the communication context
- * @sync_protocol: id of the time synch protocol
+ * @sync_protocol: name of the time synch protocol, string up to
+ *		   TRACECMD_TSYNC_PNAME_LENGTH characters length.
  * @sync_msg_id: id if the time synch message, protocol dependent
  * @payload_size: size of the packet payload, 0 in case of no payload
  * @payload: pointer to the packet payload, or NULL in case of no payload
@@ -1103,14 +1130,13 @@ out:
  * Returns 0 if packet is sent successfully, or negative error otherwise.
  */
 int tracecmd_msg_send_time_sync(struct tracecmd_msg_handle *msg_handle,
-				unsigned int sync_protocol,
-				unsigned int sync_msg_id,
+				char *sync_protocol, unsigned int sync_msg_id,
 				unsigned int payload_size, char *payload)
 {
 	struct tracecmd_msg msg;
 
 	tracecmd_msg_init(MSG_TIME_SYNC, &msg);
-	msg.tsync.sync_protocol = htonl(sync_protocol);
+	strncpy(msg.tsync.sync_protocol_name, sync_protocol, TRACECMD_TSYNC_PNAME_LENGTH);
 	msg.tsync.sync_msg_id = htonl(sync_msg_id);
 	msg.hdr.size = htonl(ntohl(msg.hdr.size) + payload_size);
 
@@ -1121,7 +1147,9 @@ int tracecmd_msg_send_time_sync(struct tracecmd_msg_handle *msg_handle,
 /**
  * tracecmd_msg_recv_time_sync - Receive a time sync packet
  * @msg_handle: message handle, holding the communication context
- * @sync_protocol: return the id of the packet's time synch protocol
+ * @sync_protocol: return the name of the packet's time synch protocol.
+ *		   It must point to a prealocated buffer with size
+ *		   TRACECMD_TSYNC_PNAME_LENGTH
  * @sync_msg_id: return the id of the packet's time synch message
  * @payload_size: size of the packet's payload, can be:
  *		 NULL - the payload is not interested and should be ignored
@@ -1146,7 +1174,7 @@ int tracecmd_msg_send_time_sync(struct tracecmd_msg_handle *msg_handle,
  * Returns 0 if packet is received successfully, or negative error otherwise.
  */
 int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle,
-				unsigned int *sync_protocol,
+				char *sync_protocol,
 				unsigned int *sync_msg_id,
 				unsigned int *payload_size, char **payload)
 {
@@ -1165,7 +1193,8 @@ int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle,
 	}
 
 	if (sync_protocol)
-		*sync_protocol = ntohl(msg.tsync.sync_protocol);
+		strncpy(sync_protocol, msg.tsync.sync_protocol_name,
+				TRACECMD_TSYNC_PNAME_LENGTH);
 	if (sync_msg_id)
 		*sync_msg_id = ntohl(msg.tsync.sync_msg_id);
 
@@ -1202,7 +1231,7 @@ out:
 static int make_trace_resp(struct tracecmd_msg *msg, int page_size, int nr_cpus,
 			   unsigned int *ports, bool use_fifos,
 			   unsigned long long trace_id,
-			   unsigned int tsync_proto,
+			   char *tsync_proto,
 			   unsigned int tsync_port)
 {
 	int data_size;
@@ -1216,7 +1245,7 @@ static int make_trace_resp(struct tracecmd_msg *msg, int page_size, int nr_cpus,
 	msg->hdr.size = htonl(ntohl(msg->hdr.size) + data_size);
 	msg->trace_resp.flags = use_fifos ? MSG_TRACE_USE_FIFOS : 0;
 	msg->trace_resp.flags = htonl(msg->trace_resp.flags);
-	msg->trace_resp.tsync_proto = htonl(tsync_proto);
+	strncpy(msg->trace_resp.tsync_proto_name, tsync_proto, TRACECMD_TSYNC_PNAME_LENGTH);
 	msg->trace_resp.tsync_port = htonl(tsync_port);
 
 	msg->trace_resp.cpus = htonl(nr_cpus);
@@ -1230,8 +1259,7 @@ int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int nr_cpus, int page_size,
 				 unsigned int *ports, bool use_fifos,
 				 unsigned long long trace_id,
-				 unsigned int tsync_proto,
-				 unsigned int tsync_port)
+				 char *tsync_proto, unsigned int tsync_port)
 {
 	struct tracecmd_msg msg;
 	int ret;
@@ -1249,7 +1277,7 @@ int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
 				 int *nr_cpus, int *page_size,
 				 unsigned int **ports, bool *use_fifos,
 				 unsigned long long *trace_id,
-				 unsigned int *tsync_proto,
+				 char **tsync_proto,
 				 unsigned int *tsync_port)
 {
 	struct tracecmd_msg msg;
@@ -1276,7 +1304,7 @@ int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
 	*nr_cpus = ntohl(msg.trace_resp.cpus);
 	*page_size = ntohl(msg.trace_resp.page_size);
 	*trace_id = ntohll(msg.trace_resp.trace_id);
-	*tsync_proto = ntohl(msg.trace_resp.tsync_proto);
+	*tsync_proto = strdup(msg.trace_resp.tsync_proto_name);
 	*tsync_port = ntohl(msg.trace_resp.tsync_port);
 	*ports = calloc(*nr_cpus, sizeof(**ports));
 	if (!*ports) {
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 35a41394..58c92ef3 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -24,8 +24,8 @@
 
 struct tsync_proto {
 	struct tsync_proto *next;
-	unsigned int proto_id;
-	int	weight;
+	char proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
+	int accuracy;
 
 	int (*clock_sync_init)(struct tracecmd_time_sync *clock_context);
 	int (*clock_sync_free)(struct tracecmd_time_sync *clock_context);
@@ -35,32 +35,35 @@ struct tsync_proto {
 
 static struct tsync_proto *tsync_proto_list;
 
-static struct tsync_proto *tsync_proto_find(unsigned int proto_id)
+static struct tsync_proto *tsync_proto_find(char *proto_name)
 {
 	struct tsync_proto *proto;
 
-	for (proto = tsync_proto_list; proto; proto = proto->next)
-		if (proto->proto_id == proto_id)
+	if (!proto_name)
+		return NULL;
+	for (proto = tsync_proto_list; proto; proto = proto->next) {
+		if (strlen(proto->proto_name) == strlen(proto_name) &&
+		     !strncmp(proto->proto_name, proto_name, TRACECMD_TSYNC_PNAME_LENGTH))
 			return proto;
-
+	}
 	return NULL;
 }
 
-int tracecmd_tsync_proto_register(unsigned int proto_id, int weight,
-				int (*init)(struct tracecmd_time_sync *),
-				int (*free)(struct tracecmd_time_sync *),
-				int (*calc)(struct tracecmd_time_sync *,
-					    long long *, long long *))
+int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
+				  int (*init)(struct tracecmd_time_sync *),
+				  int (*free)(struct tracecmd_time_sync *),
+				  int (*calc)(struct tracecmd_time_sync *,
+					      long long *, long long *))
 {
-	struct tsync_proto *proto;
+	struct tsync_proto *proto = NULL;
 
-	if (tsync_proto_find(proto_id))
+	if (tsync_proto_find(proto_name))
 		return -1;
 	proto = calloc(1, sizeof(struct tsync_proto));
 	if (!proto)
 		return -1;
-	proto->proto_id = proto_id;
-	proto->weight = weight;
+	strncpy(proto->proto_name, proto_name, TRACECMD_TSYNC_PNAME_LENGTH);
+	proto->accuracy = accuracy;
 	proto->clock_sync_init = init;
 	proto->clock_sync_free = free;
 	proto->clock_sync_calc = calc;
@@ -70,12 +73,16 @@ int tracecmd_tsync_proto_register(unsigned int proto_id, int weight,
 	return 0;
 }
 
-int tracecmd_tsync_proto_unregister(unsigned int proto_id)
+int tracecmd_tsync_proto_unregister(char *proto_name)
 {
 	struct tsync_proto **last = &tsync_proto_list;
 
+	if (!proto_name)
+		return -1;
+
 	for (; *last; last = &(*last)->next) {
-		if ((*last)->proto_id == proto_id) {
+		if (strlen((*last)->proto_name) == strlen(proto_name) &&
+		    !strncmp((*last)->proto_name, proto_name, TRACECMD_TSYNC_PNAME_LENGTH)) {
 			struct tsync_proto *proto = *last;
 
 			*last = proto->next;
@@ -87,9 +94,9 @@ int tracecmd_tsync_proto_unregister(unsigned int proto_id)
 	return -1;
 }
 
-bool tsync_proto_is_supported(unsigned int proto_id)
+bool tsync_proto_is_supported(char *proto_name)
 {
-	if (tsync_proto_find(proto_id))
+	if (tsync_proto_find(proto_name))
 		return true;
 	return false;
 }
@@ -129,80 +136,64 @@ int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
  * tracecmd_tsync_proto_select - Select time sync protocol, to be used for
  *		timestamp synchronization with a peer
  *
- * @proto_mask: bitmask array of time sync protocols, supported by the peer
- * @length: size of the @protos array
+ * @protos: array of pointers to protocol names
  *
- * Retuns Id of a time sync protocol, that can be used with the peer, or 0
- *	  in case there is no match with supported protocols
+ * Retuns pointer to a protocol name, that can be used with the peer, or NULL
+ *	  in case there is no match with supported protocols.
+ *	  The returned string MUST NOT be freed by the caller
  */
-unsigned int tracecmd_tsync_proto_select(char *proto_mask, int length)
+char *tracecmd_tsync_proto_select(char **protos)
 {
 	struct tsync_proto *selected = NULL;
 	struct tsync_proto *proto;
-	int word;
-	int id;
+	char **pname;
 
-	for (word = 0; word < length; word++) {
+	pname = protos;
+	while (*pname) {
 		for (proto = tsync_proto_list; proto; proto = proto->next) {
-			if (proto->proto_id < word * PROTO_MASK_SIZE)
+			if (strncmp(proto->proto_name, *pname, TRACECMD_TSYNC_PNAME_LENGTH))
 				continue;
-
-			id = proto->proto_id - word * PROTO_MASK_SIZE;
-			if (id >= PROTO_MASK_BITS)
-				continue;
-
-			if ((1 << id) & proto_mask[word]) {
-				if (selected) {
-					if (selected->weight < proto->weight)
-						selected = proto;
-				} else
+			if (selected) {
+				if (selected->accuracy > proto->accuracy)
 					selected = proto;
-			}
+			} else
+				selected = proto;
 		}
+		pname++;
 	}
 
 	if (selected)
-		return selected->proto_id;
+		return selected->proto_name;
 
-	return 0;
+	return NULL;
 }
 
 /**
  * tracecmd_tsync_proto_getall - Returns bitmask of all supported
  *				 time sync protocols
- * @proto_mask: return, allocated bitmask array of time sync protocols,
+ * @protos: return, allocated array of time sync protocol names,
  *	       supported by the peer. Must be freed by free()
- * @words: return, allocated size of the @protobits array
  *
- * If completed successfully 0 is returned and allocated array in @proto_mask of
- * size @words. In case of an error, -1 is returned.
- * @proto_mask must be freed with free()
+ * If completed successfully 0 is returned and allocated array in @protos of
+ * strings. The last array entry is NULL.  In case of an error, -1 is returned.
+ * @protos must be freed with free()
  */
-int tracecmd_tsync_proto_getall(char **proto_mask, int *words)
+int tracecmd_tsync_proto_getall(char ***protos)
 {
 	struct tsync_proto *proto;
-	int proto_max = 0;
-	int count = 0;
-	char *protos;
+	int count = 1;
+	int i;
 
 	for (proto = tsync_proto_list; proto; proto = proto->next)
-		if (proto->proto_id > proto_max)
-			proto_max = proto->proto_id;
+		count++;
 
-	count = proto_max / PROTO_MASK_SIZE + 1;
-	protos = calloc(count, sizeof(char));
+	*protos = calloc(count, sizeof(char *));
 	if (!protos)
 		return -1;
 
-	for (proto = tsync_proto_list; proto; proto = proto->next) {
-		if ((proto->proto_id / PROTO_MASK_SIZE) >= count)
-			continue;
-		protos[proto->proto_id / PROTO_MASK_SIZE] |=
-				(1 << (proto->proto_id % PROTO_MASK_SIZE));
-	}
+	for (i = 0, proto = tsync_proto_list; proto && i < (count - 1); proto = proto->next)
+		(*protos)[i++] = proto->proto_name;
 
-	*proto_mask = protos;
-	*words = count;
 	return 0;
 }
 
@@ -268,7 +259,7 @@ static int clock_context_init(struct tracecmd_time_sync *tsync, bool server)
 	if (tsync->context)
 		return 0;
 
-	protocol = tsync_proto_find(tsync->sync_proto);
+	protocol = tsync_proto_find(tsync->proto_name);
 	if (!protocol)
 		return -1;
 
@@ -314,7 +305,7 @@ void tracecmd_tsync_free(struct tracecmd_time_sync *tsync)
 		return;
 	tsync_context = (struct clock_sync_context *)tsync->context;
 
-	proto = tsync_proto_find(tsync->sync_proto);
+	proto = tsync_proto_find(tsync->proto_name);
 	if (proto && proto->clock_sync_free)
 		proto->clock_sync_free(tsync);
 
@@ -356,12 +347,12 @@ int tracecmd_tsync_send(struct tracecmd_time_sync *tsync,
  */
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync)
 {
+	char protocol[TRACECMD_TSYNC_PNAME_LENGTH];
 	struct tsync_proto *proto;
-	unsigned int protocol;
 	unsigned int command;
 	int ret;
 
-	proto = tsync_proto_find(tsync->sync_proto);
+	proto = tsync_proto_find(tsync->proto_name);
 	if (!proto || !proto->clock_sync_calc)
 		return;
 
@@ -371,11 +362,10 @@ void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync)
 
 	while (true) {
 		ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
-						  &protocol, &command,
+						  protocol, &command,
 						  NULL, NULL);
 
-		if (ret ||
-		    protocol != TRACECMD_TIME_SYNC_PROTO_NONE ||
+		if (ret || strncmp(protocol, TRACECMD_TSYNC_PROTO_NONE, TRACECMD_TSYNC_PNAME_LENGTH) ||
 		    command != TRACECMD_TIME_SYNC_CMD_PROBE)
 			break;
 		ret = tracecmd_tsync_send(tsync, proto);
@@ -456,7 +446,7 @@ void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync)
 	bool end = false;
 	int ret;
 
-	proto = tsync_proto_find(tsync->sync_proto);
+	proto = tsync_proto_find(tsync->proto_name);
 	if (!proto || !proto->clock_sync_calc)
 		return;
 
@@ -471,7 +461,7 @@ void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync)
 	while (true) {
 		pthread_mutex_lock(&tsync->lock);
 		ret = tracecmd_msg_send_time_sync(tsync->msg_handle,
-						  TRACECMD_TIME_SYNC_PROTO_NONE,
+						  TRACECMD_TSYNC_PROTO_NONE,
 						  TRACECMD_TIME_SYNC_CMD_PROBE,
 						  0, NULL);
 		ret = tsync_get_sample(tsync, proto, ts_array_size);
@@ -493,7 +483,7 @@ void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync)
 	};
 
 	tracecmd_msg_send_time_sync(tsync->msg_handle,
-				    TRACECMD_TIME_SYNC_PROTO_NONE,
+				    TRACECMD_TSYNC_PROTO_NONE,
 				    TRACECMD_TIME_SYNC_CMD_STOP,
 				    0, NULL);
 }
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index d148aa16..13fd60a9 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -298,9 +298,8 @@ void tracecmd_stat_cpu(struct trace_seq *s, int cpu);
 int tracecmd_host_tsync(struct buffer_instance *instance,
 			 unsigned int tsync_port);
 void tracecmd_host_tsync_complete(struct buffer_instance *instance);
-unsigned int tracecmd_guest_tsync(char *tsync_protos,
-				  unsigned int tsync_protos_size, char *clock,
-				  unsigned int *tsync_port, pthread_t *thr_id);
+char *tracecmd_guest_tsync(char **tsync_protos, char *clock,
+			   unsigned int *tsync_port, pthread_t *thr_id);
 
 int trace_make_vsock(unsigned int port);
 int trace_get_vsock_port(int sd, unsigned int *port);
diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c
index b5816966..ca810607 100644
--- a/tracecmd/trace-agent.c
+++ b/tracecmd/trace-agent.c
@@ -143,11 +143,10 @@ static char *get_clock(int argc, char **argv)
 static void agent_handle(int sd, int nr_cpus, int page_size)
 {
 	struct tracecmd_msg_handle *msg_handle;
-	unsigned int tsync_protos_size = 0;
-	unsigned int tsync_proto = 0;
+	char *tsync_proto = NULL;
 	unsigned long long trace_id;
 	unsigned int tsync_port = 0;
-	char *tsync_protos = NULL;
+	char **tsync_protos = NULL;
 	unsigned int *ports;
 	pthread_t sync_thr;
 	char **argv = NULL;
@@ -167,7 +166,7 @@ static void agent_handle(int sd, int nr_cpus, int page_size)
 
 	ret = tracecmd_msg_recv_trace_req(msg_handle, &argc, &argv,
 					  &use_fifos, &trace_id,
-					  &tsync_protos, &tsync_protos_size);
+					  &tsync_protos);
 	if (ret < 0)
 		die("Failed to receive trace request");
 
@@ -178,7 +177,6 @@ static void agent_handle(int sd, int nr_cpus, int page_size)
 		make_vsocks(nr_cpus, fds, ports);
 	if (tsync_protos) {
 		tsync_proto = tracecmd_guest_tsync(tsync_protos,
-						   tsync_protos_size,
 						   get_clock(argc, argv),
 						   &tsync_port, &sync_thr);
 		if (!tsync_proto)
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 72a5c8c9..76dcaa2b 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3854,10 +3854,9 @@ static void connect_to_agent(struct buffer_instance *instance)
 {
 	struct tracecmd_msg_handle *msg_handle;
 	int sd, ret, nr_fifos, nr_cpus, page_size;
-	unsigned int tsync_protos_reply = 0;
+	char *tsync_protos_reply = NULL;
 	unsigned int tsync_port = 0;
-	char *protos = NULL;
-	int protos_count = 0;
+	char **protos = NULL;
 	unsigned int *ports;
 	int i, *fds = NULL;
 	bool use_fifos = false;
@@ -3879,12 +3878,11 @@ static void connect_to_agent(struct buffer_instance *instance)
 		die("Failed to allocate message handle");
 
 	if (instance->tsync.loop_interval >= 0)
-		tracecmd_tsync_proto_getall(&protos, &protos_count);
+		tracecmd_tsync_proto_getall(&protos);
 
 	ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc,
 					  instance->argv, use_fifos,
-					  top_instance.trace_id,
-					  protos, protos_count);
+					  top_instance.trace_id, protos);
 	if (ret < 0)
 		die("Failed to send trace request");
 
@@ -3896,14 +3894,17 @@ static void connect_to_agent(struct buffer_instance *instance)
 					   &tsync_protos_reply, &tsync_port);
 	if (ret < 0)
 		die("Failed to receive trace response %d", ret);
-
-	if (protos_count && tsync_protos_reply) {
+	if (tsync_protos_reply && tsync_protos_reply[0]) {
 		if (tsync_proto_is_supported(tsync_protos_reply)) {
-			instance->tsync.sync_proto = tsync_protos_reply;
+			instance->tsync.proto_name = strdup(tsync_protos_reply);
+			printf("Negotiated %s time sync protocol with guest %s\n",
+				instance->tsync.proto_name,
+				tracefs_instance_get_name(instance->tracefs));
 			tracecmd_host_tsync(instance, tsync_port);
 		} else
 			warning("Failed to negotiate timestamps synchronization with the guest");
 	}
+	free(tsync_protos_reply);
 
 	if (use_fifos) {
 		if (nr_cpus != nr_fifos) {
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index e639788d..bd96110d 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -82,7 +82,7 @@ int tracecmd_host_tsync(struct buffer_instance *instance,
 	int ret;
 	int fd;
 
-	if (!instance->tsync.sync_proto)
+	if (!instance->tsync.proto_name)
 		return -1;
 
 	fd = trace_open_vsock(instance->cid, tsync_port);
@@ -207,22 +207,21 @@ out:
 	pthread_exit(0);
 }
 
-unsigned int tracecmd_guest_tsync(char *tsync_protos,
-				  unsigned int tsync_protos_size, char *clock,
-				  unsigned int *tsync_port, pthread_t *thr_id)
+char *tracecmd_guest_tsync(char **tsync_protos, char *clock,
+			   unsigned int *tsync_port, pthread_t *thr_id)
 {
 	struct tracecmd_time_sync *tsync = NULL;
 	cpu_set_t *pin_mask = NULL;
 	pthread_attr_t attrib;
 	size_t mask_size = 0;
-	unsigned int proto;
+	char *proto;
 	int ret;
 	int fd;
 
 	fd = -1;
-	proto = tracecmd_tsync_proto_select(tsync_protos, tsync_protos_size);
+	proto = tracecmd_tsync_proto_select(tsync_protos);
 	if (!proto)
-		return 0;
+		return NULL;
 #ifdef VSOCK
 	fd = trace_make_vsock(VMADDR_PORT_ANY);
 	if (fd < 0)
@@ -232,7 +231,7 @@ unsigned int tracecmd_guest_tsync(char *tsync_protos,
 	if (ret < 0)
 		goto error;
 #else
-	return 0;
+	return NULL;
 #endif
 
 	tsync = calloc(1, sizeof(struct tracecmd_time_sync));
@@ -241,7 +240,7 @@ unsigned int tracecmd_guest_tsync(char *tsync_protos,
 		tsync->clock_str = strdup(clock);
 
 	pthread_attr_init(&attrib);
-	tsync->sync_proto = proto;
+	tsync->proto_name = proto;
 	pthread_attr_setdetachstate(&attrib, PTHREAD_CREATE_JOINABLE);
 	if (!get_first_cpu(&pin_mask, &mask_size))
 		pthread_attr_setaffinity_np(&attrib, mask_size, pin_mask);
@@ -266,5 +265,5 @@ error:
 	}
 	if (fd > 0)
 		close(fd);
-	return 0;
+	return NULL;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 02/16] trace-cmd: Add trace-cmd library APIs for ftrace clock name
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 03/16] trace-cmd: Move VM related logic in a separate file Tzvetomir Stoyanov (VMware)
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Added enum with ftrace clock IDs and APIs to convert ftrace name to ID
and vice versa, as part of libtracecmd. The clock items in the enum are
organized as a bitmask, as it will be used by the timestamp
synchronization protocol to declare supported ftrace clocks.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h | 16 +++++++++++
 lib/trace-cmd/trace-util.c    | 52 +++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 8d3bea73..b48381cd 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -411,6 +411,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
+};
+
+enum tracecmd_clocks tracecmd_clock_str2id(const char *clock);
+const char *tracecmd_clock_id2str(enum tracecmd_clocks clock);
+
 /* --- Timestamp synchronization --- */
 
 #define TRACECMD_TSYNC_PNAME_LENGTH	16
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 0ead96ea..524e3309 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -33,6 +33,58 @@ static bool debug;
 
 static FILE *logfp;
 
+const static struct {
+	const char *clock_str;
+	enum tracecmd_clocks clock_id;
+} trace_clocks[] = {
+	{"local", TRACECMD_CLOCK_LOCAL},
+	{"global", TRACECMD_CLOCK_GLOBAL},
+	{"counter", TRACECMD_CLOCK_COUNTER},
+	{"uptime", TRACECMD_CLOCK_UPTIME},
+	{"perf", TRACECMD_CLOCK_PERF},
+	{"mono", TRACECMD_CLOCK_MONO},
+	{"mono_raw", TRACECMD_CLOCK_MONO_RAW},
+	{"boot", TRACECMD_CLOCK_BOOT},
+	{"x86-tsc", TRACECMD_CLOCK_X86_TSC},
+	{NULL, -1}
+};
+
+/**
+ * tracecmd_clock_str2id - Convert ftrace clock name to clock ID
+ * @clock: Ftrace clock name
+ * Returns ID of the ftrace clock
+ */
+enum tracecmd_clocks tracecmd_clock_str2id(const char *clock)
+{
+	int i;
+
+	if (!clock)
+		return TRACECMD_CLOCK_UNKNOWN;
+
+	for (i = 0; trace_clocks[i].clock_str; i++) {
+		if (!strncmp(clock, trace_clocks[i].clock_str,
+		    strlen(trace_clocks[i].clock_str)))
+			return trace_clocks[i].clock_id;
+	}
+	return TRACECMD_CLOCK_UNKNOWN;
+}
+
+/**
+ * tracecmd_clock_id2str - Convert clock ID to ftare clock name
+ * @clock: Clock ID
+ * Returns name of a ftrace clock
+ */
+const char *tracecmd_clock_id2str(enum tracecmd_clocks clock)
+{
+	int i;
+
+	for (i = 0; trace_clocks[i].clock_str; i++) {
+		if (trace_clocks[i].clock_id == clock)
+			return trace_clocks[i].clock_str;
+	}
+	return NULL;
+}
+
 /**
  * tracecmd_set_debug - Set debug mode of the tracecmd library
  * @set_debug: The new "debug" mode. If true, the tracecmd library is
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 03/16] trace-cmd: Move VM related logic in a separate file
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 02/16] trace-cmd: Add trace-cmd library APIs for ftrace clock name Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 04/16] trace-cmd: Add new libtrasefs API to get the current trace clock Tzvetomir Stoyanov (VMware)
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

All trace-cmd internal functions related to VM / guest resolving and
mappings are moved from trace-record.c to trace-vm.c. Internal APIs are
added to access the guest database, so this functionality can be used by
other logic, internally in the trace-cmd context.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/Makefile              |   1 +
 tracecmd/include/trace-local.h |  20 +++
 tracecmd/trace-record.c        | 232 +++------------------------------
 tracecmd/trace-vm.c            | 214 ++++++++++++++++++++++++++++++
 4 files changed, 250 insertions(+), 217 deletions(-)
 create mode 100644 tracecmd/trace-vm.c

diff --git a/tracecmd/Makefile b/tracecmd/Makefile
index 5e59adf8..09f13cf9 100644
--- a/tracecmd/Makefile
+++ b/tracecmd/Makefile
@@ -31,6 +31,7 @@ TRACE_CMD_OBJS += trace-show.o
 TRACE_CMD_OBJS += trace-list.o
 TRACE_CMD_OBJS += trace-usage.o
 TRACE_CMD_OBJS += trace-dump.o
+TRACE_CMD_OBJS += trace-vm.o
 ifeq ($(VSOCK_DEFINED), 1)
 TRACE_CMD_OBJS += trace-tsync.o
 endif
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 13fd60a9..aab2abe7 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -8,6 +8,7 @@
 
 #include <sys/types.h>
 #include <dirent.h>	/* for DIR */
+#include <ctype.h>	/* for isdigit() */
 
 #include "trace-cmd.h"
 #include "event-utils.h"
@@ -283,6 +284,17 @@ void update_first_instance(struct buffer_instance *instance, int topt);
 
 void show_instance_file(struct buffer_instance *instance, const char *name);
 
+struct trace_guest {
+	char *name;
+	int cid;
+	int pid;
+	int cpu_max;
+	int *cpu_pid;
+};
+struct trace_guest *get_guest_by_cid(unsigned int guest_cid);
+struct trace_guest *get_guest_by_name(char *name);
+void read_qemu_guests(void);
+int get_guest_pid(unsigned int guest_cid);
 int get_guest_vcpu_pid(unsigned int guest_cid, unsigned int guest_vcpu);
 
 /* moved from trace-cmd.h */
@@ -313,4 +325,12 @@ void *malloc_or_die(unsigned int size); /* Can be overridden */
 void __noreturn __die(const char *fmt, ...);
 void __noreturn _vdie(const char *fmt, va_list ap);
 
+static inline bool is_digits(const char *s)
+{
+	for (; *s; s++)
+		if (!isdigit(*s))
+			return false;
+	return true;
+}
+
 #endif /* __TRACE_LOCAL_H */
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 76dcaa2b..1149aaaa 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3190,239 +3190,37 @@ static int do_accept(int sd)
 	return -1;
 }
 
-static bool is_digits(const char *s)
+static char *parse_guest_name(char *gname, int *cid, int *port)
 {
-	for (; *s; s++)
-		if (!isdigit(*s))
-			return false;
-	return true;
-}
-
-struct guest {
-	char *name;
-	int cid;
-	int pid;
-	int cpu_max;
-	int *cpu_pid;
-};
-
-static struct guest *guests;
-static size_t guests_len;
-
-static int set_vcpu_pid_mapping(struct guest *guest, int cpu, int pid)
-{
-	int *cpu_pid;
-	int i;
-
-	if (cpu >= guest->cpu_max) {
-		cpu_pid = realloc(guest->cpu_pid, (cpu + 1) * sizeof(int));
-		if (!cpu_pid)
-			return -1;
-		/* Handle sparse CPU numbers */
-		for (i = guest->cpu_max; i < cpu; i++)
-			cpu_pid[i] = -1;
-		guest->cpu_max = cpu + 1;
-		guest->cpu_pid = cpu_pid;
-	}
-	guest->cpu_pid[cpu] = pid;
-	return 0;
-}
-
-static struct guest *get_guest_info(unsigned int guest_cid)
-{
-	int i;
-
-	if (!guests)
-		return NULL;
-
-	for (i = 0; i < guests_len; i++)
-		if (guest_cid == guests[i].cid)
-			return guests + i;
-	return NULL;
-}
-
-static char *get_qemu_guest_name(char *arg)
-{
-	char *tok, *end = arg;
-
-	while ((tok = strsep(&end, ","))) {
-		if (strncmp(tok, "guest=", 6) == 0)
-			return tok + 6;
-	}
-
-	return arg;
-}
-
-static int read_qemu_guests_pids(char *guest_task, struct guest *guest)
-{
-	struct dirent *entry;
-	char path[PATH_MAX];
-	char *buf = NULL;
-	size_t n = 0;
-	int ret = 0;
-	long vcpu;
-	long pid;
-	DIR *dir;
-	FILE *f;
-
-	snprintf(path, sizeof(path), "/proc/%s/task", guest_task);
-	dir = opendir(path);
-	if (!dir)
-		return -1;
-
-	while (!ret && (entry = readdir(dir))) {
-		if (!(entry->d_type == DT_DIR && is_digits(entry->d_name)))
-			continue;
-
-		snprintf(path, sizeof(path), "/proc/%s/task/%s/comm",
-			 guest_task, entry->d_name);
-		f = fopen(path, "r");
-		if (!f)
-			continue;
-
-		if (getline(&buf, &n, f) >= 0 &&
-		    strncmp(buf, "CPU ", 4) == 0) {
-			vcpu = strtol(buf + 4, NULL, 10);
-			pid = strtol(entry->d_name, NULL, 10);
-			if (vcpu < INT_MAX && pid < INT_MAX &&
-			    vcpu >= 0 && pid >= 0) {
-				if (set_vcpu_pid_mapping(guest, vcpu, pid))
-					ret = -1;
-			}
-		}
-
-		fclose(f);
-	}
-	free(buf);
-	return ret;
-}
-
-static void read_qemu_guests(void)
-{
-	static bool initialized;
-	struct dirent *entry;
-	char path[PATH_MAX];
-	DIR *dir;
-
-	if (initialized)
-		return;
-
-	initialized = true;
-	dir = opendir("/proc");
-	if (!dir)
-		die("Can not open /proc");
-
-	while ((entry = readdir(dir))) {
-		bool is_qemu = false, last_was_name = false;
-		struct guest guest = {};
-		char *p, *arg = NULL;
-		size_t arg_size = 0;
-		FILE *f;
-
-		if (!(entry->d_type == DT_DIR && is_digits(entry->d_name)))
-			continue;
-
-		guest.pid = atoi(entry->d_name);
-		snprintf(path, sizeof(path), "/proc/%s/cmdline", entry->d_name);
-		f = fopen(path, "r");
-		if (!f)
-			continue;
-
-		while (getdelim(&arg, &arg_size, 0, f) != -1) {
-			if (!is_qemu && strstr(arg, "qemu-system-")) {
-				is_qemu = true;
-				continue;
-			}
-
-			if (!is_qemu)
-				continue;
-
-			if (strcmp(arg, "-name") == 0) {
-				last_was_name = true;
-				continue;
-			}
-
-			if (last_was_name) {
-				guest.name = strdup(get_qemu_guest_name(arg));
-				if (!guest.name)
-					die("allocating guest name");
-				last_was_name = false;
-				continue;
-			}
-
-			p = strstr(arg, "guest-cid=");
-			if (p) {
-				guest.cid = atoi(p + 10);
-				continue;
-			}
-		}
-
-		if (!is_qemu)
-			goto next;
-
-		if (read_qemu_guests_pids(entry->d_name, &guest))
-			warning("Failed to retrieve VPCU - PID mapping for guest %s",
-					guest.name ? guest.name : "Unknown");
-
-		guests = realloc(guests, (guests_len + 1) * sizeof(*guests));
-		if (!guests)
-			die("Can not allocate guest buffer");
-		guests[guests_len++] = guest;
-
-next:
-		free(arg);
-		fclose(f);
-	}
-
-	closedir(dir);
-}
-
-static char *parse_guest_name(char *guest, int *cid, int *port)
-{
-	size_t i;
+	struct trace_guest *guest;
 	char *p;
 
 	*port = -1;
-	p = strrchr(guest, ':');
+	p = strrchr(gname, ':');
 	if (p) {
 		*p = '\0';
 		*port = atoi(p + 1);
 	}
 
 	*cid = -1;
-	p = strrchr(guest, '@');
+	p = strrchr(gname, '@');
 	if (p) {
 		*p = '\0';
 		*cid = atoi(p + 1);
-	} else if (is_digits(guest))
-		*cid = atoi(guest);
+	} else if (is_digits(gname))
+		*cid = atoi(gname);
 
 	read_qemu_guests();
-	for (i = 0; i < guests_len; i++) {
-		if ((*cid > 0 && *cid == guests[i].cid) ||
-		    strcmp(guest, guests[i].name) == 0) {
-			*cid = guests[i].cid;
-			return guests[i].name;
-		}
+	if (*cid > 0)
+		guest = get_guest_by_cid(*cid);
+	else
+		guest = get_guest_by_name(gname);
+	if (guest) {
+		*cid = guest->cid;
+		return guest->name;
 	}
 
-	return guest;
-}
-
-int get_guest_vcpu_pid(unsigned int guest_cid, unsigned int guest_vcpu)
-{
-	int i;
-
-	if (!guests)
-		return -1;
-
-	for (i = 0; i < guests_len; i++) {
-		if (guests[i].cpu_pid < 0 || guest_vcpu >= guests[i].cpu_max)
-			continue;
-		if (guest_cid == guests[i].cid)
-			return guests[i].cpu_pid[guest_vcpu];
-	}
-	return -1;
+	return gname;
 }
 
 static void set_prio(int prio)
@@ -4089,7 +3887,7 @@ static void append_buffer(struct tracecmd_output *handle,
 static void
 add_guest_info(struct tracecmd_output *handle, struct buffer_instance *instance)
 {
-	struct guest *guest = get_guest_info(instance->cid);
+	struct trace_guest *guest = get_guest_by_cid(instance->cid);
 	char *buf, *p;
 	int size;
 	int i;
diff --git a/tracecmd/trace-vm.c b/tracecmd/trace-vm.c
new file mode 100644
index 00000000..c8924ece
--- /dev/null
+++ b/tracecmd/trace-vm.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com>
+ * Copyright (C) 2020, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <limits.h>
+
+#include "trace-local.h"
+#include "trace-msg.h"
+
+static struct trace_guest *guests;
+static size_t guests_len;
+
+static int set_vcpu_pid_mapping(struct trace_guest *guest, int cpu, int pid)
+{
+	int *cpu_pid;
+	int i;
+
+	if (cpu >= guest->cpu_max) {
+		cpu_pid = realloc(guest->cpu_pid, (cpu + 1) * sizeof(int));
+		if (!cpu_pid)
+			return -1;
+		/* Handle sparse CPU numbers */
+		for (i = guest->cpu_max; i < cpu; i++)
+			cpu_pid[i] = -1;
+		guest->cpu_max = cpu + 1;
+		guest->cpu_pid = cpu_pid;
+	}
+	guest->cpu_pid[cpu] = pid;
+	return 0;
+}
+
+struct trace_guest *get_guest_by_cid(unsigned int guest_cid)
+{
+	int i;
+
+	if (!guests)
+		return NULL;
+
+	for (i = 0; i < guests_len; i++)
+		if (guest_cid == guests[i].cid)
+			return guests + i;
+	return NULL;
+}
+
+struct trace_guest *get_guest_by_name(char *name)
+{
+	int i;
+
+	if (!guests)
+		return NULL;
+
+	for (i = 0; i < guests_len; i++)
+		if (strcmp(name, guests[i].name) == 0)
+			return guests + i;
+	return NULL;
+}
+
+static char *get_qemu_guest_name(char *arg)
+{
+	char *tok, *end = arg;
+
+	while ((tok = strsep(&end, ","))) {
+		if (strncmp(tok, "guest=", 6) == 0)
+			return tok + 6;
+	}
+
+	return arg;
+}
+
+static int read_qemu_guests_pids(char *guest_task, struct trace_guest *guest)
+{
+	struct dirent *entry;
+	char path[PATH_MAX];
+	char *buf = NULL;
+	size_t n = 0;
+	int ret = 0;
+	long vcpu;
+	long pid;
+	DIR *dir;
+	FILE *f;
+
+	snprintf(path, sizeof(path), "/proc/%s/task", guest_task);
+	dir = opendir(path);
+	if (!dir)
+		return -1;
+
+	while (!ret && (entry = readdir(dir))) {
+		if (!(entry->d_type == DT_DIR && is_digits(entry->d_name)))
+			continue;
+
+		snprintf(path, sizeof(path), "/proc/%s/task/%s/comm",
+			 guest_task, entry->d_name);
+		f = fopen(path, "r");
+		if (!f)
+			continue;
+
+		if (getline(&buf, &n, f) >= 0 &&
+		    strncmp(buf, "CPU ", 4) == 0) {
+			vcpu = strtol(buf + 4, NULL, 10);
+			pid = strtol(entry->d_name, NULL, 10);
+			if (vcpu < INT_MAX && pid < INT_MAX &&
+			    vcpu >= 0 && pid >= 0) {
+				if (set_vcpu_pid_mapping(guest, vcpu, pid))
+					ret = -1;
+			}
+		}
+
+		fclose(f);
+	}
+	free(buf);
+	return ret;
+}
+
+void read_qemu_guests(void)
+{
+	static bool initialized;
+	struct dirent *entry;
+	char path[PATH_MAX];
+	DIR *dir;
+
+	if (initialized)
+		return;
+
+	initialized = true;
+	dir = opendir("/proc");
+	if (!dir)
+		die("Can not open /proc");
+
+	while ((entry = readdir(dir))) {
+		bool is_qemu = false, last_was_name = false;
+		struct trace_guest guest = {};
+		char *p, *arg = NULL;
+		size_t arg_size = 0;
+		FILE *f;
+
+		if (!(entry->d_type == DT_DIR && is_digits(entry->d_name)))
+			continue;
+
+		guest.pid = atoi(entry->d_name);
+		snprintf(path, sizeof(path), "/proc/%s/cmdline", entry->d_name);
+		f = fopen(path, "r");
+		if (!f)
+			continue;
+
+		while (getdelim(&arg, &arg_size, 0, f) != -1) {
+			if (!is_qemu && strstr(arg, "qemu-system-")) {
+				is_qemu = true;
+				continue;
+			}
+
+			if (!is_qemu)
+				continue;
+
+			if (strcmp(arg, "-name") == 0) {
+				last_was_name = true;
+				continue;
+			}
+
+			if (last_was_name) {
+				guest.name = strdup(get_qemu_guest_name(arg));
+				if (!guest.name)
+					die("allocating guest name");
+				last_was_name = false;
+				continue;
+			}
+
+			p = strstr(arg, "guest-cid=");
+			if (p) {
+				guest.cid = atoi(p + 10);
+				continue;
+			}
+		}
+
+		if (!is_qemu)
+			goto next;
+
+		if (read_qemu_guests_pids(entry->d_name, &guest))
+			warning("Failed to retrieve VPCU - PID mapping for guest %s",
+					guest.name ? guest.name : "Unknown");
+
+		guests = realloc(guests, (guests_len + 1) * sizeof(*guests));
+		if (!guests)
+			die("Can not allocate guest buffer");
+		guests[guests_len++] = guest;
+
+next:
+		free(arg);
+		fclose(f);
+	}
+
+	closedir(dir);
+}
+
+int get_guest_vcpu_pid(unsigned int guest_cid, unsigned int guest_vcpu)
+{
+	int i;
+
+	if (!guests)
+		return -1;
+
+	for (i = 0; i < guests_len; i++) {
+		if (guests[i].cpu_pid < 0 || guest_vcpu >= guests[i].cpu_max)
+			continue;
+		if (guest_cid == guests[i].cid)
+			return guests[i].cpu_pid[guest_vcpu];
+	}
+	return -1;
+}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 04/16] trace-cmd: Add new libtrasefs API to get the current trace clock
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (2 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 03/16] trace-cmd: Move VM related logic in a separate file Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins Tzvetomir Stoyanov (VMware)
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

A new API is added to tracefs library:
	tracefs_get_clock()
It reads the trace_clock file from the ftrace file system, parses it
and returns a string with the current trace clock. The returned string
must be freed by free().

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/tracefs/tracefs.h      |  2 ++
 lib/tracefs/tracefs-instance.c | 35 ++++++++++++++++++++++++++++++++++
 tracecmd/trace-stat.c          | 29 +++++++---------------------
 3 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h
index 8ee7ba6e..fb33e6e8 100644
--- a/include/tracefs/tracefs.h
+++ b/include/tracefs/tracefs.h
@@ -55,4 +55,6 @@ struct tep_handle *tracefs_local_events_system(const char *tracing_dir,
 int tracefs_fill_local_events(const char *tracing_dir,
 			       struct tep_handle *tep, int *parsing_failures);
 
+char *tracefs_get_clock(struct tracefs_instance *instance);
+
 #endif /* _TRACE_FS_H */
diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c
index 50e88534..89d0b484 100644
--- a/lib/tracefs/tracefs-instance.c
+++ b/lib/tracefs/tracefs-instance.c
@@ -291,3 +291,38 @@ bool tracefs_dir_exists(struct tracefs_instance *instance, char *name)
 {
 	return check_file_exists(instance, name, true);
 }
+
+/**
+ * tracefs_get_clock - Get the current trace clock
+ * @instance: ftrace instance, can be NULL for the top instance
+ *
+ * Returns the current trace clock of the given instance, or NULL in
+ * case of an error.
+ * The return string must be freed by free()
+ */
+char *tracefs_get_clock(struct tracefs_instance *instance)
+{
+	char *all_clocks = NULL;
+	char *ret = NULL;
+	int bytes = 0;
+	char *clock;
+	char *cont;
+
+	all_clocks  = tracefs_instance_file_read(instance, "trace_clock", &bytes);
+	if (!all_clocks || !bytes)
+		goto out;
+
+	clock = strstr(all_clocks, "[");
+	if (!clock)
+		goto out;
+	clock++;
+	cont = strstr(clock, "]");
+	if (!cont)
+		goto out;
+	*cont = '\0';
+
+	ret = strdup(clock);
+out:
+	free(all_clocks);
+	return ret;
+}
diff --git a/tracecmd/trace-stat.c b/tracecmd/trace-stat.c
index 260d0c37..a29e0c34 100644
--- a/tracecmd/trace-stat.c
+++ b/tracecmd/trace-stat.c
@@ -736,33 +736,18 @@ static void report_buffers(struct buffer_instance *instance)
 
 static void report_clock(struct buffer_instance *instance)
 {
-	char *str;
-	char *cont;
 	char *clock;
 
-	str = get_instance_file_content(instance, "trace_clock");
-	if (!str)
-		return;
-
-	clock = strstr(str, "[");
-	if (!clock)
-		goto out;
-	clock++;
-
-	cont = strstr(clock, "]");
-	if (!cont) /* should never happen */
-		goto out;
-
-	*cont = '\0';
+	if (instance)
+		clock = tracefs_get_clock(instance->tracefs);
+	else
+		clock = tracefs_get_clock(NULL);
 
 	/* Default clock is "local", only show others */
-	if (strcmp(clock, "local") == 0)
-		goto out;
+	if (clock && !strcmp(clock, "local") == 0)
+		printf("\nClock: %s\n", clock);
 
-	printf("\nClock: %s\n", clock);
-
- out:
-	free(str);
+	free(clock);
 }
 
 static void report_cpumask(struct buffer_instance *instance)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (3 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 04/16] trace-cmd: Add new libtrasefs API to get the current trace clock Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-12-02 22:48   ` Steven Rostedt
  2020-10-29 11:18 ` [PATCH v25 06/16] trace-cmd: Add role " Tzvetomir Stoyanov (VMware)
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Some timestamp synchronization plugins may not support all ftrace
clocks. Added logic to timestamp synchronization plugins to declare what
ftace clocks they support. Added logic to select plugin depending on the
ftrace clock used in the current trace session and supported clocks.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h             |  4 ++--
 lib/trace-cmd/include/trace-tsync-local.h |  1 +
 lib/trace-cmd/trace-timesync.c            | 29 +++++++++++++++++++----
 tracecmd/trace-record.c                   | 10 +++++++-
 tracecmd/trace-tsync.c                    |  2 +-
 5 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index b48381cd..21028940 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -448,8 +448,8 @@ struct tracecmd_time_sync {
 };
 
 void tracecmd_tsync_init(void);
-int tracecmd_tsync_proto_getall(char ***protos);
-char *tracecmd_tsync_proto_select(char **protos);
+int tracecmd_tsync_proto_getall(char ***protos, const char *clock);
+char *tracecmd_tsync_proto_select(char **protos, char *clock);
 bool tsync_proto_is_supported(char *proto_name);
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
 void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 1f3bc443..25d5b4e8 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -27,6 +27,7 @@ struct clock_sync_context {
 };
 
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
+				  int supported_clocks,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 58c92ef3..8ca47bf6 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -26,6 +26,7 @@ struct tsync_proto {
 	struct tsync_proto *next;
 	char proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
 	int accuracy;
+	int supported_clocks;
 
 	int (*clock_sync_init)(struct tracecmd_time_sync *clock_context);
 	int (*clock_sync_free)(struct tracecmd_time_sync *clock_context);
@@ -50,6 +51,7 @@ static struct tsync_proto *tsync_proto_find(char *proto_name)
 }
 
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
+				  int supported_clocks,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
@@ -64,6 +66,7 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
 		return -1;
 	strncpy(proto->proto_name, proto_name, TRACECMD_TSYNC_PNAME_LENGTH);
 	proto->accuracy = accuracy;
+	proto->supported_clocks = supported_clocks;
 	proto->clock_sync_init = init;
 	proto->clock_sync_free = free;
 	proto->clock_sync_calc = calc;
@@ -137,20 +140,26 @@ int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
  *		timestamp synchronization with a peer
  *
  * @protos: array of pointers to protocol names
+ * @clock : trace clock
  *
  * Retuns pointer to a protocol name, that can be used with the peer, or NULL
  *	  in case there is no match with supported protocols.
  *	  The returned string MUST NOT be freed by the caller
  */
-char *tracecmd_tsync_proto_select(char **protos)
+char *tracecmd_tsync_proto_select(char **protos, char *clock)
 {
 	struct tsync_proto *selected = NULL;
 	struct tsync_proto *proto;
 	char **pname;
+	int clock_id = 0;
 
+	clock_id = tracecmd_clock_str2id(clock);
 	pname = protos;
 	while (*pname) {
 		for (proto = tsync_proto_list; proto; proto = proto->next) {
+			if (proto->supported_clocks && clock_id &&
+			    !(proto->supported_clocks & clock_id))
+				continue;
 			if (strncmp(proto->proto_name, *pname, TRACECMD_TSYNC_PNAME_LENGTH))
 				continue;
 			if (selected) {
@@ -173,26 +182,38 @@ char *tracecmd_tsync_proto_select(char **protos)
  *				 time sync protocols
  * @protos: return, allocated array of time sync protocol names,
  *	       supported by the peer. Must be freed by free()
+ * @clock: selected trace clock
  *
  * If completed successfully 0 is returned and allocated array in @protos of
  * strings. The last array entry is NULL.  In case of an error, -1 is returned.
  * @protos must be freed with free()
  */
-int tracecmd_tsync_proto_getall(char ***protos)
+int tracecmd_tsync_proto_getall(char ***protos, const char *clock)
 {
 	struct tsync_proto *proto;
+	int clock_id = 0;
 	int count = 1;
 	int i;
 
-	for (proto = tsync_proto_list; proto; proto = proto->next)
+	if (clock)
+		clock_id =  tracecmd_clock_str2id(clock);
+	for (proto = tsync_proto_list; proto; proto = proto->next) {
+		if (proto->supported_clocks && clock_id &&
+		    !(proto->supported_clocks & clock_id))
+			continue;
 		count++;
+	}
 
 	*protos = calloc(count, sizeof(char *));
 	if (!protos)
 		return -1;
 
-	for (i = 0, proto = tsync_proto_list; proto && i < (count - 1); proto = proto->next)
+	for (i = 0, proto = tsync_proto_list; proto && i < (count - 1); proto = proto->next) {
+		if (proto->supported_clocks && clock_id &&
+		    !(proto->supported_clocks & clock_id))
+			continue;
 		(*protos)[i++] = proto->proto_name;
+	}
 
 	return 0;
 }
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 1149aaaa..41e0dd3a 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3675,8 +3675,11 @@ static void connect_to_agent(struct buffer_instance *instance)
 	if (!msg_handle)
 		die("Failed to allocate message handle");
 
+	if (!instance->clock)
+		instance->clock = tracefs_get_clock(NULL);
+
 	if (instance->tsync.loop_interval >= 0)
-		tracecmd_tsync_proto_getall(&protos);
+		tracecmd_tsync_proto_getall(&protos, instance->clock);
 
 	ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc,
 					  instance->argv, use_fifos,
@@ -6112,6 +6115,11 @@ static void parse_record_options(int argc,
 						 (char *)top_instance.clock,
 						 true);
 					add_argv(instance, "-C", true);
+					if (!instance->clock) {
+						instance->clock = strdup((char *)top_instance.clock);
+						if (!instance->clock)
+							die("Could not allocate instance clock");
+					}
 				}
 			}
 			instance->tsync.loop_interval = top_instance.tsync.loop_interval;
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index bd96110d..2c8afe30 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -219,7 +219,7 @@ char *tracecmd_guest_tsync(char **tsync_protos, char *clock,
 	int fd;
 
 	fd = -1;
-	proto = tracecmd_tsync_proto_select(tsync_protos);
+	proto = tracecmd_tsync_proto_select(tsync_protos, clock);
 	if (!proto)
 		return NULL;
 #ifdef VSOCK
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 06/16] trace-cmd: Add role parameter to timestamp synchronization plugins
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (4 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-12-02 23:04   ` Steven Rostedt
  2020-10-29 11:18 ` [PATCH v25 07/16] trace-cmd: Add host / guest role in timestamp synchronization context Tzvetomir Stoyanov (VMware)
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Defined HOST and GUEST roles in timestamp synchronization context.
Some timestamp synchronization plugins may not support running in both
host and guest roles. This could happen in nested virtualization use
case, where the same plugin can be used as a host and as a guest.
Added logic to timestamp synchronization plugins to declare what roles
they support. Added logic to select plugin depending on supported roles
and currently requested role.

clocks. Added logic to timestamp synchronization plugins to declare what
ftace clocks they support. Added logic to select plugin depending on the
ftrace clock used in the current trace session and supported clocks.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h             | 10 ++++++++--
 lib/trace-cmd/include/trace-tsync-local.h |  2 +-
 lib/trace-cmd/trace-timesync.c            | 17 ++++++++++++++---
 tracecmd/trace-record.c                   |  3 ++-
 tracecmd/trace-tsync.c                    |  3 ++-
 5 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 21028940..1ca2da10 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -437,6 +437,11 @@ enum{
 	TRACECMD_TIME_SYNC_CMD_STOP	= 2,
 };
 
+enum tracecmd_time_sync_role {
+	TRACECMD_TIME_SYNC_ROLE_HOST	= 0x01,
+	TRACECMD_TIME_SYNC_ROLE_GUEST	= 0x02
+};
+
 struct tracecmd_time_sync {
 	char				*proto_name;
 	int				loop_interval;
@@ -448,8 +453,9 @@ struct tracecmd_time_sync {
 };
 
 void tracecmd_tsync_init(void);
-int tracecmd_tsync_proto_getall(char ***protos, const char *clock);
-char *tracecmd_tsync_proto_select(char **protos, char *clock);
+int tracecmd_tsync_proto_getall(char ***protos, const char *clock, int role);
+char *tracecmd_tsync_proto_select(char **protos, char *clock,
+				  enum tracecmd_time_sync_role role);
 bool tsync_proto_is_supported(char *proto_name);
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
 void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 25d5b4e8..33add609 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -26,7 +26,7 @@ struct clock_sync_context {
 	unsigned int			remote_port;
 };
 
-int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
+int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int supported_clocks,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 8ca47bf6..d461392b 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -25,6 +25,7 @@
 struct tsync_proto {
 	struct tsync_proto *next;
 	char proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
+	enum tracecmd_time_sync_role roles;
 	int accuracy;
 	int supported_clocks;
 
@@ -50,7 +51,7 @@ static struct tsync_proto *tsync_proto_find(char *proto_name)
 	return NULL;
 }
 
-int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
+int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int supported_clocks,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
@@ -66,6 +67,7 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
 		return -1;
 	strncpy(proto->proto_name, proto_name, TRACECMD_TSYNC_PNAME_LENGTH);
 	proto->accuracy = accuracy;
+	proto->roles = roles;
 	proto->supported_clocks = supported_clocks;
 	proto->clock_sync_init = init;
 	proto->clock_sync_free = free;
@@ -141,12 +143,14 @@ int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
  *
  * @protos: array of pointers to protocol names
  * @clock : trace clock
+ * @role : local time sync role
  *
  * Retuns pointer to a protocol name, that can be used with the peer, or NULL
  *	  in case there is no match with supported protocols.
  *	  The returned string MUST NOT be freed by the caller
  */
-char *tracecmd_tsync_proto_select(char **protos, char *clock)
+char *tracecmd_tsync_proto_select(char **protos, char *clock,
+				  enum tracecmd_time_sync_role role)
 {
 	struct tsync_proto *selected = NULL;
 	struct tsync_proto *proto;
@@ -157,6 +161,8 @@ char *tracecmd_tsync_proto_select(char **protos, char *clock)
 	pname = protos;
 	while (*pname) {
 		for (proto = tsync_proto_list; proto; proto = proto->next) {
+			if (!(proto->roles & role))
+				continue;
 			if (proto->supported_clocks && clock_id &&
 			    !(proto->supported_clocks & clock_id))
 				continue;
@@ -183,12 +189,13 @@ char *tracecmd_tsync_proto_select(char **protos, char *clock)
  * @protos: return, allocated array of time sync protocol names,
  *	       supported by the peer. Must be freed by free()
  * @clock: selected trace clock
+ * @role: supported protocol role
  *
  * If completed successfully 0 is returned and allocated array in @protos of
  * strings. The last array entry is NULL.  In case of an error, -1 is returned.
  * @protos must be freed with free()
  */
-int tracecmd_tsync_proto_getall(char ***protos, const char *clock)
+int tracecmd_tsync_proto_getall(char ***protos, const char *clock, int role)
 {
 	struct tsync_proto *proto;
 	int clock_id = 0;
@@ -198,6 +205,8 @@ int tracecmd_tsync_proto_getall(char ***protos, const char *clock)
 	if (clock)
 		clock_id =  tracecmd_clock_str2id(clock);
 	for (proto = tsync_proto_list; proto; proto = proto->next) {
+		if (!(proto->roles & role))
+			continue;
 		if (proto->supported_clocks && clock_id &&
 		    !(proto->supported_clocks & clock_id))
 			continue;
@@ -209,6 +218,8 @@ int tracecmd_tsync_proto_getall(char ***protos, const char *clock)
 		return -1;
 
 	for (i = 0, proto = tsync_proto_list; proto && i < (count - 1); proto = proto->next) {
+		if (!(proto->roles & role))
+			continue;
 		if (proto->supported_clocks && clock_id &&
 		    !(proto->supported_clocks & clock_id))
 			continue;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 41e0dd3a..b4e68234 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3679,7 +3679,8 @@ static void connect_to_agent(struct buffer_instance *instance)
 		instance->clock = tracefs_get_clock(NULL);
 
 	if (instance->tsync.loop_interval >= 0)
-		tracecmd_tsync_proto_getall(&protos, instance->clock);
+		tracecmd_tsync_proto_getall(&protos, instance->clock,
+					    TRACECMD_TIME_SYNC_ROLE_HOST);
 
 	ret = tracecmd_msg_send_trace_req(msg_handle, instance->argc,
 					  instance->argv, use_fifos,
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index 2c8afe30..aede3754 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -219,7 +219,8 @@ char *tracecmd_guest_tsync(char **tsync_protos, char *clock,
 	int fd;
 
 	fd = -1;
-	proto = tracecmd_tsync_proto_select(tsync_protos, clock);
+	proto = tracecmd_tsync_proto_select(tsync_protos, clock,
+					    TRACECMD_TIME_SYNC_ROLE_GUEST);
 	if (!proto)
 		return NULL;
 #ifdef VSOCK
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 07/16] trace-cmd: Add host / guest role in timestamp synchronization context
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (5 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 06/16] trace-cmd: Add role " Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-12-02 23:07   ` Steven Rostedt
  2020-10-29 11:18 ` [PATCH v25 08/16] trace-cmd: Add guest CPU count PID in tracecmd_time_sync struct Tzvetomir Stoyanov (VMware)
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Added new parameter in timestamp synchronization context, holding the
current role in the timestamp synchronization process - host or guest.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/trace-tsync-local.h | 1 +
 lib/trace-cmd/trace-timesync.c            | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 33add609..793737e8 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -11,6 +11,7 @@
 struct clock_sync_context {
 	void				*proto_data;	/* time sync protocol specific data */
 	bool				is_server;	/* server side time sync role */
+	bool				is_guest;	/* guest or host time sync role */
 	struct tracefs_instance		*instance;	/* ftrace buffer, used for time sync events */
 
 	/* Arrays with calculated time offsets at given time */
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index d461392b..12e00686 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -283,7 +283,7 @@ clock_synch_delete_instance(struct tracefs_instance *inst)
 	tracefs_instance_free(inst);
 }
 
-static int clock_context_init(struct tracecmd_time_sync *tsync, bool server)
+static int clock_context_init(struct tracecmd_time_sync *tsync, bool guest)
 {
 	struct clock_sync_context *clock = NULL;
 	struct tsync_proto *protocol;
@@ -298,8 +298,11 @@ static int clock_context_init(struct tracecmd_time_sync *tsync, bool server)
 	clock = calloc(1, sizeof(struct clock_sync_context));
 	if (!clock)
 		return -1;
-
-	clock->is_server = server;
+	clock->is_guest = guest;
+	if (clock->is_guest)
+		clock->is_server = true;
+	else
+		clock->is_server = false;
 	if (get_vsocket_params(tsync->msg_handle->fd, &clock->local_cid,
 			       &clock->local_port, &clock->remote_cid,
 			       &clock->remote_port))
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 08/16] trace-cmd: Add guest CPU count PID in tracecmd_time_sync struct
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (6 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 07/16] trace-cmd: Add host / guest role in timestamp synchronization context Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction Tzvetomir Stoyanov (VMware)
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The tracecmd_time_sync struct holds the timestamp synchronization
context, used by the timestamp synchronization plugins. Guest CPU count and
PID of the host task, running the guest, is important information which
may be needed by the plugins.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h | 4 ++++
 tracecmd/trace-tsync.c        | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 1ca2da10..9629d074 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -450,6 +450,10 @@ struct tracecmd_time_sync {
 	char				*clock_str;
 	struct tracecmd_msg_handle	*msg_handle;
 	void				*context;
+
+	int				guest_pid;
+	int				vcpu_count;
+
 };
 
 void tracecmd_tsync_init(void);
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index aede3754..bc5c744e 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -77,6 +77,7 @@ int tracecmd_host_tsync(struct buffer_instance *instance,
 {
 	struct tracecmd_msg_handle *msg_handle = NULL;
 	cpu_set_t *pin_mask = NULL;
+	struct trace_guest *guest;
 	pthread_attr_t attrib;
 	size_t mask_size = 0;
 	int ret;
@@ -84,7 +85,11 @@ int tracecmd_host_tsync(struct buffer_instance *instance,
 
 	if (!instance->tsync.proto_name)
 		return -1;
-
+	guest = get_guest_by_cid(instance->cid);
+	if (guest == NULL)
+		return -1;
+	instance->tsync.guest_pid = guest->pid;
+	instance->tsync.vcpu_count = guest->cpu_max;
 	fd = trace_open_vsock(instance->cid, tsync_port);
 	if (fd < 0) {
 		ret = -1;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (7 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 08/16] trace-cmd: Add guest CPU count PID in tracecmd_time_sync struct Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-12-03  1:47   ` Steven Rostedt
  2020-10-29 11:18 ` [PATCH v25 10/16] trace-cmd: Add time sync protocol flags Tzvetomir Stoyanov (VMware)
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Some hypervisors support guest time scaling by given ratio, additional
to the time offset. The guest time is calculated using the formula:
 guest time = host time * scaling ratio + time offset
Scaling is useful in case of guest VM migration to a different host
machine.
The scaling parameter is added to timestamp synchronization algorithms.
It is saved in trace.dat file metadata and is used in timestamp
calculations when reading the file.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h             |  4 +--
 lib/trace-cmd/include/trace-tsync-local.h | 11 +++++---
 lib/trace-cmd/trace-input.c               |  8 +++++-
 lib/trace-cmd/trace-timesync.c            | 32 +++++++++++++++++------
 tracecmd/trace-dump.c                     | 11 ++++++--
 tracecmd/trace-tsync.c                    | 16 +++++++-----
 6 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 9629d074..6a6e4061 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -464,8 +464,8 @@ bool tsync_proto_is_supported(char *proto_name);
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
 void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
 int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
-				int *count,
-				long long **ts, long long **offsets);
+				int *count, long long **ts,
+				long long **offsets, long long **scalings);
 void tracecmd_tsync_free(struct tracecmd_time_sync *tsync);
 
 /* --- Plugin handling --- */
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 793737e8..492a0a90 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -15,10 +15,15 @@ struct clock_sync_context {
 	struct tracefs_instance		*instance;	/* ftrace buffer, used for time sync events */
 
 	/* Arrays with calculated time offsets at given time */
-	int				sync_size;	/* Allocated size of sync_ts and sync_offsets */
-	int				sync_count;	/* Number of elements in sync_ts and sync_offsets */
+	int				sync_size;	/* Allocated size of sync_ts,
+							 * sync_offsets and sync_scalings
+							 */
+	int				sync_count;	/* Number of elements in sync_ts,
+							 * sync_offsets and sync_scalings
+							 */
 	long long			*sync_ts;
 	long long			*sync_offsets;
+	long long			*sync_scalings;
 
 	/* Identifiers of local and remote time sync peers: cid and port */
 	unsigned int			local_cid;
@@ -32,7 +37,7 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
-					      long long *, long long *));
+					      long long *, long long *, long long *));
 int tracecmd_tsync_proto_unregister(char *proto_name);
 
 int ptp_clock_sync_register(void);
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index af97883e..1ac50f48 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -80,6 +80,7 @@ struct input_buffer_instance {
 struct ts_offset_sample {
 	long long	time;
 	long long	offset;
+	long long	scaling;
 };
 
 struct guest_trace_info {
@@ -1123,12 +1124,14 @@ static inline unsigned long long
 timestamp_correction_calc(unsigned long long ts, struct ts_offset_sample *min,
 			  struct ts_offset_sample *max)
 {
+	long long scaling = (min->scaling + max->scaling) / 2;
 	long long offset = ((long long)ts - min->time) *
 			   (max->offset - min->offset);
 	long long delta = max->time - min->time;
 	long long tscor = min->offset +
 			(offset + delta / 2) / delta;
 
+	ts *= scaling;
 	if (tscor < 0)
 		return ts - llabs(tscor);
 
@@ -2181,7 +2184,9 @@ static void tsync_offset_load(struct tracecmd_input *handle, char *buf)
 		host->ts_samples[i].time = tep_read_number(handle->pevent,
 							   buf8 + i, 8);
 		host->ts_samples[i].offset = tep_read_number(handle->pevent,
-						buf8 + host->ts_samples_count+i, 8);
+						buf8 + host->ts_samples_count + i, 8);
+		host->ts_samples[i].scaling = tep_read_number(handle->pevent,
+						buf8 + (2 * host->ts_samples_count) + i, 8);
 	}
 	qsort(host->ts_samples, host->ts_samples_count,
 	      sizeof(struct ts_offset_sample), tsync_offset_cmp);
@@ -2534,6 +2539,7 @@ static int handle_options(struct tracecmd_input *handle)
 			 * long long array of size [count] of times,
 			 *      when the offsets were calculated.
 			 * long long array of size [count] of timestamp offsets.
+			 * long long array of size [count] of timestamp scaling ratios.*
 			 */
 			if (size < 12 || handle->flags & TRACECMD_FL_IGNORE_DATE)
 				break;
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 12e00686..1036917e 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -32,7 +32,8 @@ struct tsync_proto {
 	int (*clock_sync_init)(struct tracecmd_time_sync *clock_context);
 	int (*clock_sync_free)(struct tracecmd_time_sync *clock_context);
 	int (*clock_sync_calc)(struct tracecmd_time_sync *clock_context,
-			       long long *offset, long long *timestamp);
+			       long long *offset, long long *scaling,
+			       long long *timestamp);
 };
 
 static struct tsync_proto *tsync_proto_list;
@@ -56,7 +57,7 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
-					      long long *, long long *))
+					      long long *, long long *, long long *))
 {
 	struct tsync_proto *proto = NULL;
 
@@ -113,12 +114,13 @@ bool tsync_proto_is_supported(char *proto_name)
  * @count: Returns the number of calculated time offsets
  * @ts: Array of size @count containing timestamps of callculated offsets
  * @offsets: array of size @count, containing offsets for each timestamp
+ * @scalings: array of size @count, containing scaling ratios for each timestamp
  *
  * Retuns -1 in case of an error, or 0 otherwise
  */
 int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
-				int *count,
-				long long **ts, long long **offsets)
+				int *count, long long **ts,
+				long long **offsets, long long **scalings)
 {
 	struct clock_sync_context *tsync_context;
 
@@ -131,6 +133,9 @@ int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
 		*ts = tsync_context->sync_ts;
 	if (offsets)
 		*offsets = tsync_context->sync_offsets;
+	if (scalings)
+		*scalings = tsync_context->sync_scalings;
+
 	return 0;
 }
 
@@ -349,8 +354,10 @@ void tracecmd_tsync_free(struct tracecmd_time_sync *tsync)
 
 	free(tsync_context->sync_ts);
 	free(tsync_context->sync_offsets);
+	free(tsync_context->sync_scalings);
 	tsync_context->sync_ts = NULL;
 	tsync_context->sync_offsets = NULL;
+	tsync_context->sync_scalings = NULL;
 	tsync_context->sync_count = 0;
 	tsync_context->sync_size = 0;
 	pthread_mutex_destroy(&tsync->lock);
@@ -362,10 +369,11 @@ int tracecmd_tsync_send(struct tracecmd_time_sync *tsync,
 				  struct tsync_proto *proto)
 {
 	long long timestamp = 0;
+	long long scaling = 0;
 	long long offset = 0;
 	int ret;
 
-	ret = proto->clock_sync_calc(tsync, &offset, &timestamp);
+	ret = proto->clock_sync_calc(tsync, &offset, &scaling, &timestamp);
 
 	return ret;
 }
@@ -413,18 +421,20 @@ static int tsync_get_sample(struct tracecmd_time_sync *tsync,
 			    struct tsync_proto *proto, int array_step)
 {
 	struct clock_sync_context *clock;
+	long long *sync_scalings = NULL;
 	long long *sync_offsets = NULL;
 	long long *sync_ts = NULL;
 	long long timestamp = 0;
+	long long scaling = 0;
 	long long offset = 0;
 	int ret;
 
-	ret = proto->clock_sync_calc(tsync, &offset, &timestamp);
+	ret = proto->clock_sync_calc(tsync, &offset, &scaling, &timestamp);
 	if (ret) {
 		warning("Failed to synchronize timestamps with guest");
 		return -1;
 	}
-	if (!offset || !timestamp)
+	if (!offset || !timestamp || !scaling)
 		return 0;
 	clock = tsync->context;
 	if (clock->sync_count >= clock->sync_size) {
@@ -432,18 +442,24 @@ static int tsync_get_sample(struct tracecmd_time_sync *tsync,
 				  (clock->sync_size + array_step) * sizeof(long long));
 		sync_offsets = realloc(clock->sync_offsets,
 				       (clock->sync_size + array_step) * sizeof(long long));
-		if (!sync_ts || !sync_offsets) {
+		sync_scalings = realloc(clock->sync_scalings,
+				       (clock->sync_size + array_step) * sizeof(long long));
+
+		if (!sync_ts || !sync_offsets || !sync_scalings) {
 			free(sync_ts);
 			free(sync_offsets);
+			free(sync_scalings);
 			return -1;
 		}
 		clock->sync_size += array_step;
 		clock->sync_ts = sync_ts;
 		clock->sync_offsets = sync_offsets;
+		clock->sync_scalings = sync_scalings;
 	}
 
 	clock->sync_ts[clock->sync_count] = timestamp;
 	clock->sync_offsets[clock->sync_count] = offset;
+	clock->sync_scalings[clock->sync_count] = scaling;
 	clock->sync_count++;
 
 	return 0;
diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
index 5642f12a..0117f979 100644
--- a/tracecmd/trace-dump.c
+++ b/tracecmd/trace-dump.c
@@ -369,6 +369,7 @@ static void dump_option_xlong(int fd, int size, char *desc)
 
 static void dump_option_timeshift(int fd, int size)
 {
+	long long *scalings = NULL;
 	long long *offsets = NULL;
 	long long *times = NULL;
 	long long trace_id;
@@ -397,19 +398,25 @@ static void dump_option_timeshift(int fd, int size)
 	offsets = calloc(count, sizeof(long long));
 	if (!offsets)
 		goto out;
+	scalings = calloc(count, sizeof(long long));
+	if (!scalings)
+		goto out;
 
 	for (i = 0; i < count; i++)
 		read_file_number(fd, times + i, 8);
 	for (i = 0; i < count; i++)
 		read_file_number(fd, offsets + i, 8);
+	for (i = 0; i < count; i++)
+		read_file_number(fd, scalings + i, 8);
 
 	for (i = 0; i < count; i++)
-		do_print(OPTIONS, "\t%lld %lld [offset @ time]\n",
-			 offsets[i], times[i]);
+		do_print(OPTIONS, "\t%lld * %lld %lld [offset * scaling @ time]\n",
+			 offsets[i], scalings[1], times[i]);
 
 out:
 	free(times);
 	free(offsets);
+	free(scalings);
 }
 
 void dump_option_guest(int fd, int size)
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index bc5c744e..6d0a9e85 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -132,16 +132,18 @@ out:
 static void write_guest_time_shift(struct buffer_instance *instance)
 {
 	struct tracecmd_output *handle;
-	struct iovec vector[4];
-	long long *offsets;
-	long long *ts;
+	struct iovec vector[5];
+	long long *scalings = NULL;
+	long long *offsets = NULL;
+	long long *ts = NULL;
 	const char *file;
 	int count;
 	int ret;
 	int fd;
 
-	ret = tracecmd_tsync_get_offsets(&instance->tsync, &count, &ts, &offsets);
-	if (ret < 0 || !count || !ts || !offsets)
+	ret = tracecmd_tsync_get_offsets(&instance->tsync, &count,
+					 &ts, &offsets, &scalings);
+	if (ret < 0 || !count || !ts || !offsets || !scalings)
 		return;
 
 	file = instance->output_file;
@@ -157,7 +159,9 @@ static void write_guest_time_shift(struct buffer_instance *instance)
 	vector[2].iov_base = ts;
 	vector[3].iov_len = 8 * count;
 	vector[3].iov_base = offsets;
-	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 4);
+	vector[4].iov_len = 8 * count;
+	vector[4].iov_base = scalings;
+	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 5);
 	tracecmd_append_options(handle);
 	tracecmd_output_close(handle);
 #ifdef TSYNC_DEBUG
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 10/16] trace-cmd: Add time sync protocol flags
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (8 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-12-03  2:09   ` Steven Rostedt
  2020-10-29 11:18 ` [PATCH v25 11/16] trace-cmd: Add timestamp synchronization per vCPU Tzvetomir Stoyanov (VMware)
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Added 32bit flags for the time synchronization protocols. The first added
flag is TRACECMD_TSYNC_FLAG_INTERPOLATE, used to specify how the
timestamps must be corrected.
 - If the flag is set, an interpolation is performed:
   Find the (min, max) interval from the offsets array and calculate
offset specific to the given timestamp using interpolation in that
interval.
 - If the flag is not set, do not interpolate:
   Find the (min, max) interval from the offsets array and use the
   min offset for all timespamps within the interval.

These flags are set by the timestamp synchronization protocols at the
protocol initialization time.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h             |  5 +++
 lib/trace-cmd/include/trace-tsync-local.h |  2 +-
 lib/trace-cmd/trace-input.c               | 48 +++++++++++++++--------
 lib/trace-cmd/trace-timesync.c            | 28 ++++++++++++-
 tracecmd/trace-dump.c                     |  3 ++
 tracecmd/trace-tsync.c                    | 20 ++++++----
 6 files changed, 81 insertions(+), 25 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 6a6e4061..0a6c3ad9 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -456,6 +456,9 @@ struct tracecmd_time_sync {
 
 };
 
+/* Timestamp synchronization flags */
+#define TRACECMD_TSYNC_FLAG_INTERPOLATE	0x1
+
 void tracecmd_tsync_init(void);
 int tracecmd_tsync_proto_getall(char ***protos, const char *clock, int role);
 char *tracecmd_tsync_proto_select(char **protos, char *clock,
@@ -466,6 +469,8 @@ void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
 int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
 				int *count, long long **ts,
 				long long **offsets, long long **scalings);
+int tracecmd_tsync_get_proto_flags(struct tracecmd_time_sync *tsync,
+				   unsigned int *flags);
 void tracecmd_tsync_free(struct tracecmd_time_sync *tsync);
 
 /* --- Plugin handling --- */
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 492a0a90..54a85ee7 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -33,7 +33,7 @@ struct clock_sync_context {
 };
 
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
-				  int supported_clocks,
+				  int supported_clocks, unsigned int flags,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 1ac50f48..c38b634b 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -93,6 +93,7 @@ struct guest_trace_info {
 
 struct host_trace_info {
 	unsigned long long	peer_trace_id;
+	unsigned int		flags;
 	bool			sync_enable;
 	struct tracecmd_input	*peer_data;
 	int			ts_samples_count;
@@ -1121,15 +1122,25 @@ static void free_next(struct tracecmd_input *handle, int cpu)
 }
 
 static inline unsigned long long
-timestamp_correction_calc(unsigned long long ts, struct ts_offset_sample *min,
+timestamp_correction_calc(unsigned long long ts, unsigned int flags,
+			  struct ts_offset_sample *min,
 			  struct ts_offset_sample *max)
 {
-	long long scaling = (min->scaling + max->scaling) / 2;
-	long long offset = ((long long)ts - min->time) *
-			   (max->offset - min->offset);
-	long long delta = max->time - min->time;
-	long long tscor = min->offset +
-			(offset + delta / 2) / delta;
+	long long scaling;
+	long long tscor;
+
+	if (flags & TRACECMD_TSYNC_FLAG_INTERPOLATE) {
+		long long delta = max->time - min->time;
+		long long offset = ((long long)ts - min->time) *
+				   (max->offset - min->offset);
+
+		scaling = (min->scaling + max->scaling) / 2;
+		tscor = min->offset + (offset + delta / 2) / delta;
+
+	} else {
+		scaling = min->scaling;
+		tscor = min->offset;
+	}
 
 	ts *= scaling;
 	if (tscor < 0)
@@ -1156,16 +1167,17 @@ static unsigned long long timestamp_correct(unsigned long long ts,
 
 	/* We have two samples, nothing to search here */
 	if (host->ts_samples_count == 2)
-		return timestamp_correction_calc(ts, &host->ts_samples[0],
+		return timestamp_correction_calc(ts, host->flags,
+						 &host->ts_samples[0],
 						 &host->ts_samples[1]);
 
 	/* We have more than two samples */
 	if (ts <= host->ts_samples[0].time)
-		return timestamp_correction_calc(ts,
+		return timestamp_correction_calc(ts, host->flags,
 						 &host->ts_samples[0],
 						 &host->ts_samples[1]);
 	else if (ts >= host->ts_samples[host->ts_samples_count-1].time)
-		return timestamp_correction_calc(ts,
+		return timestamp_correction_calc(ts, host->flags,
 						 &host->ts_samples[host->ts_samples_count-2],
 						 &host->ts_samples[host->ts_samples_count-1]);
 	min = 0;
@@ -1181,7 +1193,8 @@ static unsigned long long timestamp_correct(unsigned long long ts,
 		mid = (min + max)/2;
 	}
 
-	return timestamp_correction_calc(ts, &host->ts_samples[mid],
+	return timestamp_correction_calc(ts, host->flags,
+					 &host->ts_samples[mid],
 					 &host->ts_samples[mid+1]);
 }
 
@@ -2535,28 +2548,31 @@ static int handle_options(struct tracecmd_input *handle)
 		case TRACECMD_OPTION_TIME_SHIFT:
 			/*
 			 * long long int (8 bytes) trace session ID
+			 * int (4 bytes) protocol flags.
 			 * int (4 bytes) count of timestamp offsets.
 			 * long long array of size [count] of times,
 			 *      when the offsets were calculated.
 			 * long long array of size [count] of timestamp offsets.
 			 * long long array of size [count] of timestamp scaling ratios.*
 			 */
-			if (size < 12 || handle->flags & TRACECMD_FL_IGNORE_DATE)
+			if (size < 16 || handle->flags & TRACECMD_FL_IGNORE_DATE)
 				break;
 			handle->host.peer_trace_id = tep_read_number(handle->pevent,
 								     buf, 8);
+			handle->host.flags = tep_read_number(handle->pevent,
+							     buf + 8, 4);
 			handle->host.ts_samples_count = tep_read_number(handle->pevent,
-									buf + 8, 4);
+									buf + 12, 4);
 			samples_size = (8 * handle->host.ts_samples_count);
-			if (size != (12 + (2 * samples_size))) {
+			if (size != (16 + (2 * samples_size))) {
 				warning("Failed to extract Time Shift information from the file: found size %d, expected is %d",
-					size, 12 + (2 * samples_size));
+					size, 16 + (2 * samples_size));
 				break;
 			}
 			handle->host.ts_samples = malloc(2 * samples_size);
 			if (!handle->host.ts_samples)
 				return -ENOMEM;
-			tsync_offset_load(handle, buf + 12);
+			tsync_offset_load(handle, buf + 16);
 			break;
 		case TRACECMD_OPTION_CPUSTAT:
 			buf[size-1] = '\n';
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 1036917e..0f3246c6 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -28,6 +28,7 @@ struct tsync_proto {
 	enum tracecmd_time_sync_role roles;
 	int accuracy;
 	int supported_clocks;
+	unsigned int flags;
 
 	int (*clock_sync_init)(struct tracecmd_time_sync *clock_context);
 	int (*clock_sync_free)(struct tracecmd_time_sync *clock_context);
@@ -53,7 +54,7 @@ static struct tsync_proto *tsync_proto_find(char *proto_name)
 }
 
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
-				  int supported_clocks,
+				  int supported_clocks, unsigned int flags,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
@@ -139,6 +140,31 @@ int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
 	return 0;
 }
 
+/**
+ * tracecmd_tsync_get_proto_flags - Get protocol flags
+ *
+ * @tsync: Pointer to time sync context
+ * @flags: Returns the protocol flags, a combination of TRACECMD_TSYNC_FLAG_...
+ *
+ * Retuns -1 in case of an error, or 0 otherwise
+ */
+int tracecmd_tsync_get_proto_flags(struct tracecmd_time_sync *tsync,
+				   unsigned int *flags)
+{
+	struct tsync_proto *protocol;
+
+	if (!tsync)
+		return -1;
+	protocol = tsync_proto_find(tsync->proto_name);
+	if (!protocol)
+		return -1;
+
+	if (flags)
+		*flags = protocol->flags;
+
+	return 0;
+}
+
 
 #define PROTO_MASK_SIZE (sizeof(char))
 #define PROTO_MASK_BITS (PROTO_MASK_SIZE * 8)
diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
index 0117f979..6172231e 100644
--- a/tracecmd/trace-dump.c
+++ b/tracecmd/trace-dump.c
@@ -374,6 +374,7 @@ static void dump_option_timeshift(int fd, int size)
 	long long *times = NULL;
 	long long trace_id;
 	unsigned int count;
+	unsigned int flags;
 	int i;
 
 	/*
@@ -390,6 +391,8 @@ static void dump_option_timeshift(int fd, int size)
 	do_print(OPTIONS, "\t\t[Option TimeShift, %d bytes]\n", size);
 	read_file_number(fd, &trace_id, 8);
 	do_print(OPTIONS, "0x%llX [peer's trace id]\n", trace_id);
+	read_file_number(fd, &flags, 4);
+	do_print(OPTIONS, "0x%llX [peer's protocol flags]\n", flags);
 	read_file_number(fd, &count, 4);
 	do_print(OPTIONS, "%lld [samples count]\n", count);
 	times = calloc(count, sizeof(long long));
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index 6d0a9e85..943a274a 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -132,7 +132,8 @@ out:
 static void write_guest_time_shift(struct buffer_instance *instance)
 {
 	struct tracecmd_output *handle;
-	struct iovec vector[5];
+	struct iovec vector[6];
+	unsigned int flags;
 	long long *scalings = NULL;
 	long long *offsets = NULL;
 	long long *ts = NULL;
@@ -145,6 +146,9 @@ static void write_guest_time_shift(struct buffer_instance *instance)
 					 &ts, &offsets, &scalings);
 	if (ret < 0 || !count || !ts || !offsets || !scalings)
 		return;
+	ret = tracecmd_tsync_get_proto_flags(&instance->tsync, &flags);
+	if (ret < 0)
+		return;
 
 	file = instance->output_file;
 	fd = open(file, O_RDWR);
@@ -154,14 +158,16 @@ static void write_guest_time_shift(struct buffer_instance *instance)
 	vector[0].iov_len = 8;
 	vector[0].iov_base = &top_instance.trace_id;
 	vector[1].iov_len = 4;
-	vector[1].iov_base = &count;
-	vector[2].iov_len = 8 * count;
-	vector[2].iov_base = ts;
+	vector[1].iov_base = &flags;
+	vector[2].iov_len = 4;
+	vector[2].iov_base = &count;
 	vector[3].iov_len = 8 * count;
-	vector[3].iov_base = offsets;
+	vector[3].iov_base = ts;
 	vector[4].iov_len = 8 * count;
-	vector[4].iov_base = scalings;
-	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 5);
+	vector[4].iov_base = offsets;
+	vector[5].iov_len = 8 * count;
+	vector[5].iov_base = scalings;
+	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 6);
 	tracecmd_append_options(handle);
 	tracecmd_output_close(handle);
 #ifdef TSYNC_DEBUG
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 11/16] trace-cmd: Add timestamp synchronization per vCPU
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (9 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 10/16] trace-cmd: Add time sync protocol flags Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 12/16] trace-cmd: Define a macro for packed structures Tzvetomir Stoyanov (VMware)
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Timestamp synchronization logic is changed to work per virtual CPU. Some
hypervisors maintain time offset and scaling per vCPU. The host-guest
communication protocol is changed to request time stamp offset calculation
for particular vCPU. The guest thread, responsible for running that logic
is pinned to the requested CPU. The time sync medata data, saved in the
trace.dat file is changed to an array of vCPUs. When an event time stamp
is corrected, the CPU on that the event happened is used to get the
correct offset.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/trace-cmd/trace-cmd.h             |   2 +-
 lib/trace-cmd/include/trace-tsync-local.h |  22 ++-
 lib/trace-cmd/trace-input.c               | 179 +++++++++++-------
 lib/trace-cmd/trace-timesync.c            | 215 ++++++++++++++++------
 tracecmd/trace-dump.c                     |  52 +++---
 tracecmd/trace-tsync.c                    |  67 ++++---
 6 files changed, 365 insertions(+), 172 deletions(-)

diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
index 0a6c3ad9..338aebc6 100644
--- a/include/trace-cmd/trace-cmd.h
+++ b/include/trace-cmd/trace-cmd.h
@@ -466,7 +466,7 @@ char *tracecmd_tsync_proto_select(char **protos, char *clock,
 bool tsync_proto_is_supported(char *proto_name);
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
 void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
-int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
+int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync, int cpu,
 				int *count, long long **ts,
 				long long **offsets, long long **scalings);
 int tracecmd_tsync_get_proto_flags(struct tracecmd_time_sync *tsync,
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index 54a85ee7..a25eae27 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -8,12 +8,7 @@
 
 #include <stdbool.h>
 
-struct clock_sync_context {
-	void				*proto_data;	/* time sync protocol specific data */
-	bool				is_server;	/* server side time sync role */
-	bool				is_guest;	/* guest or host time sync role */
-	struct tracefs_instance		*instance;	/* ftrace buffer, used for time sync events */
-
+struct clock_sync_offsets {
 	/* Arrays with calculated time offsets at given time */
 	int				sync_size;	/* Allocated size of sync_ts,
 							 * sync_offsets and sync_scalings
@@ -24,6 +19,18 @@ struct clock_sync_context {
 	long long			*sync_ts;
 	long long			*sync_offsets;
 	long long			*sync_scalings;
+};
+
+struct clock_sync_context {
+	void				*proto_data;	/* time sync protocol specific data */
+	bool				is_server;	/* server side time sync role */
+	bool				is_guest;	/* guest or host time sync role */
+	struct tracefs_instance		*instance;	/* ftrace buffer, used for time sync events */
+
+	int				cpu_count;
+	struct clock_sync_offsets	*offsets;	/* Array of size cpu_count
+							 * calculated offsets per CPU
+							 */
 
 	/* Identifiers of local and remote time sync peers: cid and port */
 	unsigned int			local_cid;
@@ -37,7 +44,8 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
-					      long long *, long long *, long long *));
+					      long long *, long long *,
+					      long long *, unsigned int));
 int tracecmd_tsync_proto_unregister(char *proto_name);
 
 int ptp_clock_sync_register(void);
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index c38b634b..18ee7e8b 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -91,13 +91,19 @@ struct guest_trace_info {
 	int			*cpu_pid;
 };
 
+struct timesync_offsets {
+	int	ts_samples_count;
+	struct ts_offset_sample	*ts_samples;
+
+};
+
 struct host_trace_info {
 	unsigned long long	peer_trace_id;
 	unsigned int		flags;
 	bool			sync_enable;
 	struct tracecmd_input	*peer_data;
-	int			ts_samples_count;
-	struct ts_offset_sample	*ts_samples;
+	int			cpu_count;
+	struct timesync_offsets	*ts_offsets;
 };
 
 struct tracecmd_input {
@@ -1149,53 +1155,56 @@ timestamp_correction_calc(unsigned long long ts, unsigned int flags,
 	return ts + tscor;
 }
 
-static unsigned long long timestamp_correct(unsigned long long ts,
+static unsigned long long timestamp_correct(unsigned long long ts, int cpu,
 					    struct tracecmd_input *handle)
 {
-	struct host_trace_info	*host = &handle->host;
+	struct timesync_offsets *tsync;
 	int min, mid, max;
 
 	if (handle->ts_offset)
 		return ts + handle->ts_offset;
 
-	if (!host->sync_enable)
+	if (!handle->host.sync_enable)
 		return ts;
+	if (cpu >= handle->host.cpu_count)
+		return ts;
+	tsync = &handle->host.ts_offsets[cpu];
 
 	/* We have one sample, nothing to calc here */
-	if (host->ts_samples_count == 1)
-		return ts + host->ts_samples[0].offset;
+	if (tsync->ts_samples_count == 1)
+		return ts + tsync->ts_samples[0].offset;
 
 	/* We have two samples, nothing to search here */
-	if (host->ts_samples_count == 2)
-		return timestamp_correction_calc(ts, host->flags,
-						 &host->ts_samples[0],
-						 &host->ts_samples[1]);
+	if (tsync->ts_samples_count == 2)
+		return timestamp_correction_calc(ts, handle->host.flags,
+						 &tsync->ts_samples[0],
+						 &tsync->ts_samples[1]);
 
 	/* We have more than two samples */
-	if (ts <= host->ts_samples[0].time)
-		return timestamp_correction_calc(ts, host->flags,
-						 &host->ts_samples[0],
-						 &host->ts_samples[1]);
-	else if (ts >= host->ts_samples[host->ts_samples_count-1].time)
-		return timestamp_correction_calc(ts, host->flags,
-						 &host->ts_samples[host->ts_samples_count-2],
-						 &host->ts_samples[host->ts_samples_count-1]);
+	if (ts <= tsync->ts_samples[0].time)
+		return timestamp_correction_calc(ts, handle->host.flags,
+						 &tsync->ts_samples[0],
+						 &tsync->ts_samples[1]);
+	else if (ts >= tsync->ts_samples[tsync->ts_samples_count-1].time)
+		return timestamp_correction_calc(ts, handle->host.flags,
+						 &tsync->ts_samples[tsync->ts_samples_count-2],
+						 &tsync->ts_samples[tsync->ts_samples_count-1]);
 	min = 0;
-	max = host->ts_samples_count-1;
+	max = tsync->ts_samples_count-1;
 	mid = (min + max)/2;
 	while (min <= max) {
-		if (ts < host->ts_samples[mid].time)
+		if (ts < tsync->ts_samples[mid].time)
 			max = mid - 1;
-		else if (ts > host->ts_samples[mid].time)
+		else if (ts > tsync->ts_samples[mid].time)
 			min = mid + 1;
 		else
 			break;
 		mid = (min + max)/2;
 	}
 
-	return timestamp_correction_calc(ts, host->flags,
-					 &host->ts_samples[mid],
-					 &host->ts_samples[mid+1]);
+	return timestamp_correction_calc(ts, handle->host.flags,
+					 &tsync->ts_samples[mid],
+					 &tsync->ts_samples[mid+1]);
 }
 
 /*
@@ -1219,7 +1228,8 @@ static int update_page_info(struct tracecmd_input *handle, int cpu)
 		    kbuffer_subbuffer_size(kbuf));
 		return -1;
 	}
-	handle->cpu_data[cpu].timestamp = timestamp_correct(kbuffer_timestamp(kbuf), handle);
+	handle->cpu_data[cpu].timestamp = timestamp_correct(kbuffer_timestamp(kbuf),
+							    cpu, handle);
 
 	if (handle->ts2secs)
 		handle->cpu_data[cpu].timestamp *= handle->ts2secs;
@@ -1852,7 +1862,7 @@ read_again:
 		goto read_again;
 	}
 
-	handle->cpu_data[cpu].timestamp = timestamp_correct(ts, handle);
+	handle->cpu_data[cpu].timestamp = timestamp_correct(ts, cpu, handle);
 
 	if (handle->ts2secs) {
 		handle->cpu_data[cpu].timestamp *= handle->ts2secs;
@@ -2187,42 +2197,80 @@ static int tsync_offset_cmp(const void *a, const void *b)
 	return 0;
 }
 
-static void tsync_offset_load(struct tracecmd_input *handle, char *buf)
+#define safe_read(R, C) \
+	{ if ((C) > size) return -EFAULT; \
+	 (R) = tep_read_number(tep, buf, (C)); \
+	 buf += (C); \
+	 size -= (C); }
+static int tsync_offset_load(struct tep_handle	*tep,
+			     struct timesync_offsets *ts_offsets, char *buf, int size)
 {
-	struct host_trace_info *host = &handle->host;
-	long long *buf8 = (long long *)buf;
+	int start_size = size;
 	int i, j;
 
-	for (i = 0; i < host->ts_samples_count; i++) {
-		host->ts_samples[i].time = tep_read_number(handle->pevent,
-							   buf8 + i, 8);
-		host->ts_samples[i].offset = tep_read_number(handle->pevent,
-						buf8 + host->ts_samples_count + i, 8);
-		host->ts_samples[i].scaling = tep_read_number(handle->pevent,
-						buf8 + (2 * host->ts_samples_count) + i, 8);
-	}
-	qsort(host->ts_samples, host->ts_samples_count,
+	for (i = 0; i < ts_offsets->ts_samples_count; i++)
+		safe_read(ts_offsets->ts_samples[i].time, 8);
+	for (i = 0; i < ts_offsets->ts_samples_count; i++)
+		safe_read(ts_offsets->ts_samples[i].offset, 8);
+	for (i = 0; i < ts_offsets->ts_samples_count; i++)
+		safe_read(ts_offsets->ts_samples[i].scaling, 8);
+	qsort(ts_offsets->ts_samples, ts_offsets->ts_samples_count,
 	      sizeof(struct ts_offset_sample), tsync_offset_cmp);
 	/* Filter possible samples with equal time */
-	for (i = 0, j = 0; i < host->ts_samples_count; i++) {
-		if (i == 0 || host->ts_samples[i].time != host->ts_samples[i-1].time)
-			host->ts_samples[j++] = host->ts_samples[i];
+	for (i = 0, j = 0; i < ts_offsets->ts_samples_count; i++) {
+		if (i == 0 || ts_offsets->ts_samples[i].time != ts_offsets->ts_samples[i-1].time)
+			ts_offsets->ts_samples[j++] = ts_offsets->ts_samples[i];
+	}
+	ts_offsets->ts_samples_count = j;
+
+	return start_size - size;
+}
+
+static int tsync_cpu_offsets_load(struct tracecmd_input *handle, char *buf, int size)
+{
+	struct tep_handle *tep = handle->pevent;
+	int ret;
+	int i;
+
+	safe_read(handle->host.cpu_count, 4);
+	handle->host.ts_offsets = calloc(handle->host.cpu_count,
+					 sizeof(struct timesync_offsets));
+	if (!handle->host.ts_offsets)
+		return -ENOMEM;
+	for (i = 0; i < handle->host.cpu_count; i++) {
+		safe_read(handle->host.ts_offsets[i].ts_samples_count, 4);
+		handle->host.ts_offsets[i].ts_samples = calloc(handle->host.ts_offsets[i].ts_samples_count,
+							       sizeof(struct ts_offset_sample));
+		if (!handle->host.ts_offsets[i].ts_samples)
+			return -ENOMEM;
+		ret = tsync_offset_load(tep, &handle->host.ts_offsets[i], buf, size);
+		if (ret <= 0)
+			return -EFAULT;
+		size -= ret;
+		buf += ret;
 	}
-	host->ts_samples_count = j;
+	return 0;
 }
 
 static void tsync_check_enable(struct tracecmd_input *handle)
 {
 	struct host_trace_info	*host = &handle->host;
 	struct guest_trace_info *guest;
+	int i;
 
 	host->sync_enable = false;
 
-	if (!host->peer_data || !host->peer_data->guest ||
-	    !host->ts_samples_count || !host->ts_samples)
+	if (!host->peer_data || !host->peer_data->guest)
 		return;
 	if (host->peer_trace_id != host->peer_data->trace_id)
 		return;
+	if (!host->cpu_count || !host->ts_offsets)
+		return;
+	for (i = 0; i < host->cpu_count; i++) {
+		if (!host->ts_offsets[i].ts_samples_count ||
+		    !host->ts_offsets[i].ts_samples)
+			return;
+	}
 	guest = host->peer_data->guest;
 	while (guest) {
 		if (guest->trace_id == handle->trace_id)
@@ -2237,8 +2285,14 @@ static void tsync_check_enable(struct tracecmd_input *handle)
 
 static void trace_tsync_offset_free(struct host_trace_info *host)
 {
-	free(host->ts_samples);
-	host->ts_samples = NULL;
+	int i;
+
+	if (host->ts_offsets) {
+		for (i = 0; i < host->cpu_count; i++)
+			free(host->ts_offsets[i].ts_samples);
+		free(host->ts_offsets);
+		host->ts_offsets = NULL;
+	}
 	if (host->peer_data) {
 		tracecmd_close(host->peer_data);
 		host->peer_data = NULL;
@@ -2497,8 +2551,8 @@ static int handle_options(struct tracecmd_input *handle)
 	struct input_buffer_instance *buffer;
 	struct hook_list *hook;
 	char *buf;
-	int samples_size;
 	int cpus;
+	int ret;
 
 	/* By default, use usecs, unless told otherwise */
 	handle->flags |= TRACECMD_FL_IN_USECS;
@@ -2549,11 +2603,15 @@ static int handle_options(struct tracecmd_input *handle)
 			/*
 			 * long long int (8 bytes) trace session ID
 			 * int (4 bytes) protocol flags.
-			 * int (4 bytes) count of timestamp offsets.
-			 * long long array of size [count] of times,
+			 * int (4 bytes) CPU count.
+			 * array of size [CPU count]:
+			 * [
+			 *  int (4 bytes) count of timestamp offsets.
+			 *  long long array of size [count] of times,
 			 *      when the offsets were calculated.
-			 * long long array of size [count] of timestamp offsets.
-			 * long long array of size [count] of timestamp scaling ratios.*
+			 *  long long array of size [count] of timestamp offsets.
+			 *  long long array of size [count] of timestamp scaling ratios.*
+			 * ]
 			 */
 			if (size < 16 || handle->flags & TRACECMD_FL_IGNORE_DATE)
 				break;
@@ -2561,18 +2619,9 @@ static int handle_options(struct tracecmd_input *handle)
 								     buf, 8);
 			handle->host.flags = tep_read_number(handle->pevent,
 							     buf + 8, 4);
-			handle->host.ts_samples_count = tep_read_number(handle->pevent,
-									buf + 12, 4);
-			samples_size = (8 * handle->host.ts_samples_count);
-			if (size != (16 + (2 * samples_size))) {
-				warning("Failed to extract Time Shift information from the file: found size %d, expected is %d",
-					size, 16 + (2 * samples_size));
-				break;
-			}
-			handle->host.ts_samples = malloc(2 * samples_size);
-			if (!handle->host.ts_samples)
-				return -ENOMEM;
-			tsync_offset_load(handle, buf + 16);
+			ret = tsync_cpu_offsets_load(handle, buf + 12, size - 12);
+			if (ret < 0)
+				return ret;
 			break;
 		case TRACECMD_OPTION_CPUSTAT:
 			buf[size-1] = '\n';
@@ -3911,7 +3960,7 @@ unsigned long long tracecmd_get_tsync_peer(struct tracecmd_input *handle)
 int tracecmd_enable_tsync(struct tracecmd_input *handle, bool enable)
 {
 	if (enable &&
-	    (!handle->host.ts_samples || !handle->host.ts_samples_count))
+	    (!handle->host.ts_offsets || !handle->host.cpu_count))
 		return -1;
 
 	handle->host.sync_enable = enable;
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 0f3246c6..676a9482 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -22,6 +22,8 @@
 #include "event-utils.h"
 #include "trace-tsync-local.h"
 
+typedef __be16 be16;
+
 struct tsync_proto {
 	struct tsync_proto *next;
 	char proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
@@ -34,9 +36,13 @@ struct tsync_proto {
 	int (*clock_sync_free)(struct tracecmd_time_sync *clock_context);
 	int (*clock_sync_calc)(struct tracecmd_time_sync *clock_context,
 			       long long *offset, long long *scaling,
-			       long long *timestamp);
+			       long long *timestamp, unsigned int cpu);
 };
 
+struct tsync_probe_request_msg {
+	be16	cpu;
+} __attribute__((packed));
+
 static struct tsync_proto *tsync_proto_list;
 
 static struct tsync_proto *tsync_proto_find(char *proto_name)
@@ -58,7 +64,8 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int (*init)(struct tracecmd_time_sync *),
 				  int (*free)(struct tracecmd_time_sync *),
 				  int (*calc)(struct tracecmd_time_sync *,
-					      long long *, long long *, long long *))
+					      long long *, long long *,
+					      long long *, unsigned int))
 {
 	struct tsync_proto *proto = NULL;
 
@@ -70,6 +77,7 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 	strncpy(proto->proto_name, proto_name, TRACECMD_TSYNC_PNAME_LENGTH);
 	proto->accuracy = accuracy;
 	proto->roles = roles;
+	proto->flags = flags;
 	proto->supported_clocks = supported_clocks;
 	proto->clock_sync_init = init;
 	proto->clock_sync_free = free;
@@ -112,6 +120,7 @@ bool tsync_proto_is_supported(char *proto_name)
  * tracecmd_tsync_get_offsets - Return the calculated time offsets
  *
  * @tsync: Pointer to time sync context
+ * @cpu: CPU for which to get the calculated offsets
  * @count: Returns the number of calculated time offsets
  * @ts: Array of size @count containing timestamps of callculated offsets
  * @offsets: array of size @count, containing offsets for each timestamp
@@ -119,7 +128,7 @@ bool tsync_proto_is_supported(char *proto_name)
  *
  * Retuns -1 in case of an error, or 0 otherwise
  */
-int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
+int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync, int cpu,
 				int *count, long long **ts,
 				long long **offsets, long long **scalings)
 {
@@ -128,14 +137,16 @@ int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
 	if (!tsync || !tsync->context)
 		return -1;
 	tsync_context = (struct clock_sync_context *)tsync->context;
+	if (cpu >= tsync_context->cpu_count || !tsync_context->offsets)
+		return -1;
 	if (count)
-		*count = tsync_context->sync_count;
+		*count = tsync_context->offsets[cpu].sync_count;
 	if (ts)
-		*ts = tsync_context->sync_ts;
+		*ts = tsync_context->offsets[cpu].sync_ts;
 	if (offsets)
-		*offsets = tsync_context->sync_offsets;
+		*offsets = tsync_context->offsets[cpu].sync_offsets;
 	if (scalings)
-		*scalings = tsync_context->sync_scalings;
+		*scalings = tsync_context->offsets[cpu].sync_scalings;
 
 	return 0;
 }
@@ -344,6 +355,13 @@ static int clock_context_init(struct tracecmd_time_sync *tsync, bool guest)
 	if (!clock->instance)
 		goto error;
 
+	clock->cpu_count = tsync->vcpu_count;
+	if (clock->cpu_count) {
+		clock->offsets = calloc(clock->cpu_count, sizeof(struct clock_sync_offsets));
+		if (!clock->offsets)
+			goto error;
+	}
+
 	tsync->context = clock;
 	if (protocol->clock_sync_init && protocol->clock_sync_init(tsync) < 0)
 		goto error;
@@ -351,6 +369,9 @@ static int clock_context_init(struct tracecmd_time_sync *tsync, bool guest)
 	return 0;
 error:
 	tsync->context = NULL;
+	if (clock->instance)
+		clock_synch_delete_instance(clock->instance);
+	free(clock->offsets);
 	free(clock);
 	return -1;
 }
@@ -366,6 +387,7 @@ void tracecmd_tsync_free(struct tracecmd_time_sync *tsync)
 {
 	struct clock_sync_context *tsync_context;
 	struct tsync_proto *proto;
+	int i;
 
 	if (!tsync->context)
 		return;
@@ -378,28 +400,88 @@ void tracecmd_tsync_free(struct tracecmd_time_sync *tsync)
 	clock_synch_delete_instance(tsync_context->instance);
 	tsync_context->instance = NULL;
 
-	free(tsync_context->sync_ts);
-	free(tsync_context->sync_offsets);
-	free(tsync_context->sync_scalings);
-	tsync_context->sync_ts = NULL;
-	tsync_context->sync_offsets = NULL;
-	tsync_context->sync_scalings = NULL;
-	tsync_context->sync_count = 0;
-	tsync_context->sync_size = 0;
+	if (tsync_context->cpu_count && tsync_context->offsets) {
+		for (i = 0; i < tsync_context->cpu_count; i++) {
+			free(tsync_context->offsets[i].sync_ts);
+			free(tsync_context->offsets[i].sync_offsets);
+			free(tsync_context->offsets[i].sync_scalings);
+			tsync_context->offsets[i].sync_ts = NULL;
+			tsync_context->offsets[i].sync_offsets = NULL;
+			tsync_context->offsets[i].sync_scalings = NULL;
+			tsync_context->offsets[i].sync_count = 0;
+			tsync_context->offsets[i].sync_size = 0;
+		}
+		free(tsync_context->offsets);
+		tsync_context->offsets = NULL;
+	}
 	pthread_mutex_destroy(&tsync->lock);
 	pthread_cond_destroy(&tsync->cond);
 	free(tsync->clock_str);
 }
 
+static cpu_set_t *pin_to_cpu(int cpu)
+{
+	static size_t size;
+	static int cpus;
+	cpu_set_t *mask = NULL;
+	cpu_set_t *old = NULL;
+
+	if (!cpus) {
+		cpus = tracecmd_count_cpus();
+		size = CPU_ALLOC_SIZE(cpus);
+	}
+	if (cpu >= cpus)
+		goto error;
+
+	mask = CPU_ALLOC(cpus);
+	if (!mask)
+		goto error;
+	old = CPU_ALLOC(cpus);
+	if (!old)
+		goto error;
+
+	CPU_ZERO_S(size, mask);
+	CPU_SET_S(cpu, size, mask);
+	if (pthread_getaffinity_np(pthread_self(), size, old))
+		goto error;
+	if (pthread_setaffinity_np(pthread_self(), size, mask))
+		goto error;
+
+	CPU_FREE(mask);
+	return old;
+
+error:
+	if (mask)
+		CPU_FREE(mask);
+	if (old)
+		CPU_FREE(old);
+	return NULL;
+}
+
+static void restore_pin_to_cpu(cpu_set_t *mask)
+{
+	static size_t size;
+
+	if (!size)
+		size = CPU_ALLOC_SIZE(tracecmd_count_cpus());
+
+	pthread_setaffinity_np(pthread_self(), size, mask);
+	CPU_FREE(mask);
+}
+
 int tracecmd_tsync_send(struct tracecmd_time_sync *tsync,
-				  struct tsync_proto *proto)
+			struct tsync_proto *proto, unsigned int cpu)
 {
+	cpu_set_t *old_set = NULL;
 	long long timestamp = 0;
 	long long scaling = 0;
 	long long offset = 0;
 	int ret;
 
-	ret = proto->clock_sync_calc(tsync, &offset, &scaling, &timestamp);
+	old_set = pin_to_cpu(cpu);
+	ret = proto->clock_sync_calc(tsync, &offset, &scaling, &timestamp, cpu);
+	if (old_set)
+		restore_pin_to_cpu(old_set);
 
 	return ret;
 }
@@ -417,8 +499,11 @@ int tracecmd_tsync_send(struct tracecmd_time_sync *tsync,
 void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync)
 {
 	char protocol[TRACECMD_TSYNC_PNAME_LENGTH];
+	struct tsync_probe_request_msg probe;
 	struct tsync_proto *proto;
 	unsigned int command;
+	unsigned int size;
+	char *msg;
 	int ret;
 
 	proto = tsync_proto_find(tsync->proto_name);
@@ -429,47 +514,37 @@ void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync)
 	if (!tsync->context)
 		return;
 
+	msg = (char *)&probe;
+	size = sizeof(probe);
 	while (true) {
+		memset(&probe, 0, size);
 		ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
 						  protocol, &command,
-						  NULL, NULL);
+						  &size, &msg);
 
 		if (ret || strncmp(protocol, TRACECMD_TSYNC_PROTO_NONE, TRACECMD_TSYNC_PNAME_LENGTH) ||
 		    command != TRACECMD_TIME_SYNC_CMD_PROBE)
 			break;
-		ret = tracecmd_tsync_send(tsync, proto);
+		ret = tracecmd_tsync_send(tsync, proto, probe.cpu);
 		if (ret)
 			break;
 	}
 }
 
-static int tsync_get_sample(struct tracecmd_time_sync *tsync,
-			    struct tsync_proto *proto, int array_step)
+static int record_sync_sample(struct clock_sync_offsets *offsets, int array_step,
+			      long long offset, long long scaling, long long ts)
 {
-	struct clock_sync_context *clock;
 	long long *sync_scalings = NULL;
 	long long *sync_offsets = NULL;
 	long long *sync_ts = NULL;
-	long long timestamp = 0;
-	long long scaling = 0;
-	long long offset = 0;
-	int ret;
 
-	ret = proto->clock_sync_calc(tsync, &offset, &scaling, &timestamp);
-	if (ret) {
-		warning("Failed to synchronize timestamps with guest");
-		return -1;
-	}
-	if (!offset || !timestamp || !scaling)
-		return 0;
-	clock = tsync->context;
-	if (clock->sync_count >= clock->sync_size) {
-		sync_ts = realloc(clock->sync_ts,
-				  (clock->sync_size + array_step) * sizeof(long long));
-		sync_offsets = realloc(clock->sync_offsets,
-				       (clock->sync_size + array_step) * sizeof(long long));
-		sync_scalings = realloc(clock->sync_scalings,
-				       (clock->sync_size + array_step) * sizeof(long long));
+	if (offsets->sync_count >= offsets->sync_size) {
+		sync_ts = realloc(offsets->sync_ts,
+				  (offsets->sync_size + array_step) * sizeof(long long));
+		sync_offsets = realloc(offsets->sync_offsets,
+				       (offsets->sync_size + array_step) * sizeof(long long));
+		sync_scalings = realloc(offsets->sync_scalings,
+				       (offsets->sync_size + array_step) * sizeof(long long));
 
 		if (!sync_ts || !sync_offsets || !sync_scalings) {
 			free(sync_ts);
@@ -477,20 +552,43 @@ static int tsync_get_sample(struct tracecmd_time_sync *tsync,
 			free(sync_scalings);
 			return -1;
 		}
-		clock->sync_size += array_step;
-		clock->sync_ts = sync_ts;
-		clock->sync_offsets = sync_offsets;
-		clock->sync_scalings = sync_scalings;
+		offsets->sync_size += array_step;
+		offsets->sync_ts = sync_ts;
+		offsets->sync_offsets = sync_offsets;
+		offsets->sync_scalings = sync_scalings;
 	}
 
-	clock->sync_ts[clock->sync_count] = timestamp;
-	clock->sync_offsets[clock->sync_count] = offset;
-	clock->sync_scalings[clock->sync_count] = scaling;
-	clock->sync_count++;
+	offsets->sync_ts[offsets->sync_count] = ts;
+	offsets->sync_offsets[offsets->sync_count] = offset;
+	offsets->sync_scalings[offsets->sync_count] = scaling;
+	offsets->sync_count++;
 
 	return 0;
 }
 
+static int tsync_get_sample(struct tracecmd_time_sync *tsync, unsigned int cpu,
+			    struct tsync_proto *proto, int array_step)
+{
+	struct clock_sync_context *clock;
+	long long timestamp = 0;
+	long long scaling = 0;
+	long long offset = 0;
+	int ret;
+
+	ret = proto->clock_sync_calc(tsync, &offset, &scaling, &timestamp, cpu);
+	if (ret) {
+		warning("Failed to synchronize timestamps with guest");
+		return -1;
+	}
+	if (!offset || !timestamp || !scaling)
+		return 0;
+	clock = tsync->context;
+	if (!clock || cpu >= clock->cpu_count || !clock->offsets)
+		return -1;
+	return record_sync_sample(&clock->offsets[cpu], array_step,
+				  offset, scaling, timestamp);
+}
+
 #define TIMER_SEC_NANO 1000000000LL
 static inline void get_ts_loop_delay(struct timespec *timeout, int delay_ms)
 {
@@ -517,11 +615,13 @@ static inline void get_ts_loop_delay(struct timespec *timeout, int delay_ms)
  */
 void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync)
 {
+	struct tsync_probe_request_msg probe;
 	int ts_array_size = CLOCK_TS_ARRAY;
 	struct tsync_proto *proto;
 	struct timespec timeout;
 	bool end = false;
 	int ret;
+	int i;
 
 	proto = tsync_proto_find(tsync->proto_name);
 	if (!proto || !proto->clock_sync_calc)
@@ -537,12 +637,17 @@ void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync)
 
 	while (true) {
 		pthread_mutex_lock(&tsync->lock);
-		ret = tracecmd_msg_send_time_sync(tsync->msg_handle,
-						  TRACECMD_TSYNC_PROTO_NONE,
-						  TRACECMD_TIME_SYNC_CMD_PROBE,
-						  0, NULL);
-		ret = tsync_get_sample(tsync, proto, ts_array_size);
-		if (ret || end)
+		for (i = 0; i < tsync->vcpu_count; i++) {
+			probe.cpu = i;
+			ret = tracecmd_msg_send_time_sync(tsync->msg_handle,
+							  TRACECMD_TSYNC_PROTO_NONE,
+							  TRACECMD_TIME_SYNC_CMD_PROBE,
+							  sizeof(probe), (char *)&probe);
+			ret = tsync_get_sample(tsync, i, proto, ts_array_size);
+			if (ret)
+				break;
+		}
+		if (end || i < tsync->vcpu_count)
 			break;
 		if (tsync->loop_interval > 0) {
 			get_ts_loop_delay(&timeout, tsync->loop_interval);
diff --git a/tracecmd/trace-dump.c b/tracecmd/trace-dump.c
index 6172231e..c1143eba 100644
--- a/tracecmd/trace-dump.c
+++ b/tracecmd/trace-dump.c
@@ -375,7 +375,8 @@ static void dump_option_timeshift(int fd, int size)
 	long long trace_id;
 	unsigned int count;
 	unsigned int flags;
-	int i;
+	unsigned int cpus;
+	int i, j;
 
 	/*
 	 * long long int (8 bytes) trace session ID
@@ -393,29 +394,34 @@ static void dump_option_timeshift(int fd, int size)
 	do_print(OPTIONS, "0x%llX [peer's trace id]\n", trace_id);
 	read_file_number(fd, &flags, 4);
 	do_print(OPTIONS, "0x%llX [peer's protocol flags]\n", flags);
-	read_file_number(fd, &count, 4);
-	do_print(OPTIONS, "%lld [samples count]\n", count);
-	times = calloc(count, sizeof(long long));
-	if (!times)
-		goto out;
-	offsets = calloc(count, sizeof(long long));
-	if (!offsets)
-		goto out;
-	scalings = calloc(count, sizeof(long long));
-	if (!scalings)
-		goto out;
-
-	for (i = 0; i < count; i++)
-		read_file_number(fd, times + i, 8);
-	for (i = 0; i < count; i++)
-		read_file_number(fd, offsets + i, 8);
-	for (i = 0; i < count; i++)
-		read_file_number(fd, scalings + i, 8);
-
-	for (i = 0; i < count; i++)
-		do_print(OPTIONS, "\t%lld * %lld %lld [offset * scaling @ time]\n",
-			 offsets[i], scalings[1], times[i]);
+	read_file_number(fd, &cpus, 4);
+	do_print(OPTIONS, "0x%llX [peer's CPU count]\n", cpus);
+	for (j = 0; j < cpus; j++) {
+		read_file_number(fd, &count, 4);
+		do_print(OPTIONS, "%lld [samples count for CPU %d]\n", count, j);
+		times = calloc(count, sizeof(long long));
+		offsets = calloc(count, sizeof(long long));
+		scalings = calloc(count, sizeof(long long));
+		if (!times || !offsets || !scalings)
+			goto out;
+		for (i = 0; i < count; i++)
+			read_file_number(fd, times + i, 8);
+		for (i = 0; i < count; i++)
+			read_file_number(fd, offsets + i, 8);
+		for (i = 0; i < count; i++)
+			read_file_number(fd, scalings + i, 8);
+
+		for (i = 0; i < count; i++)
+			do_print(OPTIONS, "\t%lld %lld %lld [offset * scaling @ time]\n",
+				 offsets[i], scalings[1], times[i]);
+		free(times);
+		free(offsets);
+		free(scalings);
+		times = NULL;
+		offsets = NULL;
+		scalings = NULL;
 
+	}
 out:
 	free(times);
 	free(offsets);
diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c
index 943a274a..5db23ec3 100644
--- a/tracecmd/trace-tsync.c
+++ b/tracecmd/trace-tsync.c
@@ -131,51 +131,75 @@ out:
 
 static void write_guest_time_shift(struct buffer_instance *instance)
 {
-	struct tracecmd_output *handle;
-	struct iovec vector[6];
+	struct tracecmd_output *handle = NULL;
+	struct iovec *vector = NULL;
 	unsigned int flags;
 	long long *scalings = NULL;
 	long long *offsets = NULL;
 	long long *ts = NULL;
 	const char *file;
+	int fd = -1;
+	int vcount;
 	int count;
+	int i, j;
 	int ret;
-	int fd;
 
-	ret = tracecmd_tsync_get_offsets(&instance->tsync, &count,
-					 &ts, &offsets, &scalings);
-	if (ret < 0 || !count || !ts || !offsets || !scalings)
+	if (!instance->tsync.vcpu_count)
+		return;
+	vcount = 3 + (4 * instance->tsync.vcpu_count);
+	vector = calloc(vcount, sizeof(struct iovec));
+	if (!vector)
 		return;
 	ret = tracecmd_tsync_get_proto_flags(&instance->tsync, &flags);
 	if (ret < 0)
-		return;
+		goto out;
 
 	file = instance->output_file;
 	fd = open(file, O_RDWR);
 	if (fd < 0)
 		die("error opening %s", file);
 	handle = tracecmd_get_output_handle_fd(fd);
-	vector[0].iov_len = 8;
-	vector[0].iov_base = &top_instance.trace_id;
-	vector[1].iov_len = 4;
-	vector[1].iov_base = &flags;
-	vector[2].iov_len = 4;
-	vector[2].iov_base = &count;
-	vector[3].iov_len = 8 * count;
-	vector[3].iov_base = ts;
-	vector[4].iov_len = 8 * count;
-	vector[4].iov_base = offsets;
-	vector[5].iov_len = 8 * count;
-	vector[5].iov_base = scalings;
-	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 6);
+	if (!handle)
+		goto out;
+	j = 0;
+	vector[j].iov_len = 8;
+	vector[j++].iov_base = &top_instance.trace_id;
+	vector[j].iov_len = 4;
+	vector[j++].iov_base = &flags;
+	vector[j].iov_len = 4;
+	vector[j++].iov_base = &instance->tsync.vcpu_count;
+	for (i = 0; i < instance->tsync.vcpu_count; i++) {
+		if (j >= vcount)
+			break;
+		ret = tracecmd_tsync_get_offsets(&instance->tsync, i, &count,
+						 &ts, &offsets, &scalings);
+		if (ret < 0 || !count || !ts || !offsets || !scalings)
+			break;
+		vector[j].iov_len = 4;
+		vector[j++].iov_base = &count;
+		vector[j].iov_len = 8 * count;
+		vector[j++].iov_base = ts;
+		vector[j].iov_len = 8 * count;
+		vector[j++].iov_base = offsets;
+		vector[j].iov_len = 8 * count;
+		vector[j++].iov_base = scalings;
+	}
+	if (i < instance->tsync.vcpu_count)
+		goto out;
+	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, vcount);
 	tracecmd_append_options(handle);
-	tracecmd_output_close(handle);
 #ifdef TSYNC_DEBUG
 	if (count > 1)
 		printf("Got %d timestamp synch samples for guest %s in %lld ns trace\n\r",
 			count, tracefs_instance_get_name(instance->tracefs),
 			ts[count - 1] - ts[0]);
 #endif
+out:
+	if (handle)
+		tracecmd_output_close(handle);
+	else if (fd >= 0)
+		close(fd);
+	free(vector);
 }
 
 void tracecmd_host_tsync_complete(struct buffer_instance *instance)
@@ -257,6 +281,7 @@ char *tracecmd_guest_tsync(char **tsync_protos, char *clock,
 
 	pthread_attr_init(&attrib);
 	tsync->proto_name = proto;
+	tsync->vcpu_count = tracecmd_count_cpus();
 	pthread_attr_setdetachstate(&attrib, PTHREAD_CREATE_JOINABLE);
 	if (!get_first_cpu(&pin_mask, &mask_size))
 		pthread_attr_setaffinity_np(&attrib, mask_size, pin_mask);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 12/16] trace-cmd: Define a macro for packed structures
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (10 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 11/16] trace-cmd: Add timestamp synchronization per vCPU Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 13/16] trace-cmd: Add dummy function to initialize timestamp sync logic Tzvetomir Stoyanov (VMware)
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Defined a new macro
 #define __packed __attribute__((packed))
used for packing the structures, to make the code look cleaner.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/include/trace-cmd-local.h |  2 ++
 lib/trace-cmd/trace-msg.c               | 14 +++++++-------
 lib/trace-cmd/trace-timesync.c          |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h
index 95dce66c..19f7add8 100644
--- a/lib/trace-cmd/include/trace-cmd-local.h
+++ b/lib/trace-cmd/include/trace-cmd-local.h
@@ -9,6 +9,8 @@
 /* Can be overridden */
 void warning(const char *fmt, ...);
 
+#define __packed __attribute__((packed))
+
 /* trace.dat file format version */
 #define FILE_VERSION 6
 
diff --git a/lib/trace-cmd/trace-msg.c b/lib/trace-cmd/trace-msg.c
index 59ff33c8..905ae281 100644
--- a/lib/trace-cmd/trace-msg.c
+++ b/lib/trace-cmd/trace-msg.c
@@ -57,11 +57,11 @@ struct tracecmd_msg_tinit {
 	be32 cpus;
 	be32 page_size;
 	be32 opt_num;
-} __attribute__((packed));
+} __packed;
 
 struct tracecmd_msg_rinit {
 	be32 cpus;
-} __attribute__((packed));
+} __packed;
 
 #define TRACE_REQ_PARAM_SIZE  (2 * sizeof(int))
 enum trace_req_params {
@@ -79,7 +79,7 @@ struct tracecmd_msg_trace_req {
 	be32 flags;
 	be32 argc;
 	u64 trace_id;
-} __attribute__((packed));
+} __packed;
 
 struct tracecmd_msg_trace_resp {
 	be32 flags;
@@ -88,18 +88,18 @@ struct tracecmd_msg_trace_resp {
 	u64 trace_id;
 	char tsync_proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
 	be32 tsync_port;
-} __attribute__((packed));
+} __packed;
 
 struct tracecmd_msg_tsync {
 	char sync_protocol_name[TRACECMD_TSYNC_PNAME_LENGTH];
 	be32 sync_msg_id;
-} __attribute__((packed));
+} __packed;
 
 struct tracecmd_msg_header {
 	be32	size;
 	be32	cmd;
 	be32	cmd_size;
-} __attribute__((packed));
+} __packed;
 
 #define MSG_MAP								\
 	C(CLOSE,	0,	0),					\
@@ -148,7 +148,7 @@ struct tracecmd_msg {
 		struct tracecmd_msg_tsync	tsync;
 	};
 	char					*buf;
-} __attribute__((packed));
+} __packed;
 
 static inline int msg_buf_len(struct tracecmd_msg *msg)
 {
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 676a9482..40d50fd5 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -41,7 +41,7 @@ struct tsync_proto {
 
 struct tsync_probe_request_msg {
 	be16	cpu;
-} __attribute__((packed));
+} __packed;
 
 static struct tsync_proto *tsync_proto_list;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 13/16] trace-cmd: Add dummy function to initialize timestamp sync logic
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (11 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 12/16] trace-cmd: Define a macro for packed structures Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 14/16] trace-cmd: [POC] PTP-like algorithm for host - guest timestamp synchronization Tzvetomir Stoyanov (VMware)
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

A dummy empty function is added, will be used to initialize timestamp
synchronization logic:
  tracecmd_tsync_init()
When this code is implemented as real plugins, this function will be
removed. Then the initializion will be triggered at plugin load time.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-timesync.c |  8 ++++++++
 tracecmd/trace-agent.c         |  2 ++
 tracecmd/trace-record.c        | 23 +++++++++++++++--------
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 40d50fd5..c8c1f287 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -59,6 +59,14 @@ static struct tsync_proto *tsync_proto_find(char *proto_name)
 	return NULL;
 }
 
+/**
+ * tracecmd_tsync_init - Initialize the global, per task, time sync data.
+ */
+void tracecmd_tsync_init(void)
+{
+
+}
+
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 				  int supported_clocks, unsigned int flags,
 				  int (*init)(struct tracecmd_time_sync *),
diff --git a/tracecmd/trace-agent.c b/tracecmd/trace-agent.c
index ca810607..cb1e608d 100644
--- a/tracecmd/trace-agent.c
+++ b/tracecmd/trace-agent.c
@@ -245,6 +245,8 @@ static void agent_serve(unsigned int port)
 	if (sd < 0)
 		die("Failed to open vsocket");
 
+	tracecmd_tsync_init();
+
 	if (!get_local_cid(&cid))
 		printf("listening on @%u:%u\n", cid, port);
 
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index b4e68234..28bb9904 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -6233,10 +6233,6 @@ static bool has_local_instances(void)
 	return false;
 }
 
-/*
- * This function contains common code for the following commands:
- * record, start, stream, profile.
- */
 static void record_trace(int argc, char **argv,
 			 struct common_record_context *ctx)
 {
@@ -6394,12 +6390,23 @@ static void record_trace(int argc, char **argv,
 	finalize_record_trace(ctx);
 }
 
+/*
+ * This function contains common code for the following commands:
+ * record, start, stream, profile.
+ */
+static void record_trace_command(int argc, char **argv,
+				 struct common_record_context *ctx)
+{
+	tracecmd_tsync_init();
+	record_trace(argc, argv, ctx);
+}
+
 void trace_start(int argc, char **argv)
 {
 	struct common_record_context ctx;
 
 	parse_record_options(argc, argv, CMD_start, &ctx);
-	record_trace(argc, argv, &ctx);
+	record_trace_command(argc, argv, &ctx);
 	exit(0);
 }
 
@@ -6491,7 +6498,7 @@ void trace_stream(int argc, char **argv)
 	struct common_record_context ctx;
 
 	parse_record_options(argc, argv, CMD_stream, &ctx);
-	record_trace(argc, argv, &ctx);
+	record_trace_command(argc, argv, &ctx);
 	exit(0);
 }
 
@@ -6510,7 +6517,7 @@ void trace_profile(int argc, char **argv)
 	if (!buffer_instances)
 		top_instance.flags |= BUFFER_FL_PROFILE;
 
-	record_trace(argc, argv, &ctx);
+	record_trace_command(argc, argv, &ctx);
 	do_trace_profile();
 	exit(0);
 }
@@ -6529,7 +6536,7 @@ void trace_record(int argc, char **argv)
 	struct common_record_context ctx;
 
 	parse_record_options(argc, argv, CMD_record, &ctx);
-	record_trace(argc, argv, &ctx);
+	record_trace_command(argc, argv, &ctx);
 	exit(0);
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 14/16] trace-cmd: [POC] PTP-like algorithm for host - guest timestamp synchronization
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (12 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 13/16] trace-cmd: Add dummy function to initialize timestamp sync logic Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 15/16] trace-cmd: Debug scripts for " Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 16/16] trace-cmd [POC]: Add KVM timestamp synchronization plugin Tzvetomir Stoyanov (VMware)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

PTP protocol is designed for synchronizing clocks of machines in a local network.
The same approach can be used for host - guest timestamp synchronization.
This implementation uses ftrace raw markers to track trace timestamps of PTP events.
The patch is a POC, two different algorithms for PTP calculations are proposed:
  - Choosing the sample with the fastest response time for calculating the clocks offset.
  - Calculating the clocks offset using the average of all PTP samples.

The implementation can be tuned using those parameters:
 - #define FASTEST_RESPONSE - is defined, the sample with the fastest response time
    is used for calculating the clocks offset. Otherwise the histogram of all samples is used.
 - #define PTP_SYNC_LOOP 340 - defines the number of samples, used for one calculation.
 - --tsync-interval - a trace-cmd argument, choose the intervals between offset calculations,
	performed continuously during the trace.
 - #define TSYNC_DEBUG - if defined, a debug information is collected and stored in files,
   in the guest machine:
     s-cid*.txt - For each offset calculation: host and guest clocks and calculated offset.
     res-cid*.txt - For each tracing session: all calculated clock offsets.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/Makefile             |   1 +
 lib/trace-cmd/trace-timesync-ptp.c | 714 +++++++++++++++++++++++++++++
 lib/trace-cmd/trace-timesync.c     |   2 +-
 3 files changed, 716 insertions(+), 1 deletion(-)
 create mode 100644 lib/trace-cmd/trace-timesync-ptp.c

diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile
index 666a1ebf..7cab7514 100644
--- a/lib/trace-cmd/Makefile
+++ b/lib/trace-cmd/Makefile
@@ -18,6 +18,7 @@ OBJS += trace-msg.o
 OBJS += trace-plugin.o
 ifeq ($(VSOCK_DEFINED), 1)
 OBJS += trace-timesync.o
+OBJS += trace-timesync-ptp.o
 endif
 
 # Additional util objects
diff --git a/lib/trace-cmd/trace-timesync-ptp.c b/lib/trace-cmd/trace-timesync-ptp.c
new file mode 100644
index 00000000..d043053f
--- /dev/null
+++ b/lib/trace-cmd/trace-timesync-ptp.c
@@ -0,0 +1,714 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2019, VMware, Tzvetomir Stoyanov tz.stoyanov@gmail.com>
+ *
+ */
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <linux/vm_sockets.h>
+#include <sys/types.h>
+#include <linux/types.h>
+#include <time.h>
+#include <sched.h>
+#include <limits.h>
+
+#include "trace-cmd.h"
+#include "tracefs.h"
+#include "trace-tsync-local.h"
+#include "trace-msg.h"
+#include "trace-cmd-local.h"
+
+typedef __be32 be32;
+typedef __u64 u64;
+typedef __s64 s64;
+
+#define PTP_SYNC_LOOP	339
+
+#define PTP_SYNC_PKT_START	1
+#define PTP_SYNC_PKT_PROBE	2
+#define PTP_SYNC_PKT_PROBES	3
+#define PTP_SYNC_PKT_OFFSET	4
+#define PTP_SYNC_PKT_END	5
+
+/* print time sync debug messages */
+#define TSYNC_DEBUG
+
+struct ptp_clock_sync {
+	struct tep_handle	*tep;
+	struct tep_format_field	*id;
+	int			raw_id;
+	int			marker_fd;
+	int			series_id;
+	int			flags;
+	int			debug_fd;
+};
+
+enum {
+/*
+ * Consider only the probe with fastest response time,
+ * otherwise make a histogram from all probes.
+ */
+	PTP_FLAG_FASTEST_RESPONSE		= (1 << 0),
+/*
+ * Use trace marker to get the clock,
+ * otherwise use the system clock directly.
+ */
+	PTP_FLAG_USE_MARKER			= (1 << 1),
+};
+static int ptp_flags = PTP_FLAG_FASTEST_RESPONSE | PTP_FLAG_USE_MARKER;
+
+/*
+ * Calculated using formula [CPU rate]*[calculated offset deviation]
+ * tested on 3GHz CPU, with x86-tsc trace clock and compare the calculated
+ * offset with /sys/kernel/debug/kvm/<VM ID>/vcpu0/tsc-offset
+ * measured 2000ns deviation
+ * using PTP flags PTP_FLAG_FASTEST_RESPONSE | PTP_FLAG_USE_MARKER
+*/
+#define PTP_ACCURACY	6000
+#define PTP_NAME	"ptp"
+
+struct ptp_clock_start_msg {
+	be32	series_id;
+	be32	flags;
+} __packed;
+
+struct ptp_clock_sample {
+	s64		ts;
+	be32		id;
+} __packed;
+
+struct ptp_clock_result_msg {
+	be32			series_id;
+	be32			count;
+	struct ptp_clock_sample	samples[2*PTP_SYNC_LOOP];
+} __packed;
+
+struct ptp_clock_offset_msg {
+	s64	ts;
+	s64	offset;
+};
+
+struct ptp_markers_context {
+	struct clock_sync_context	*clock;
+	struct ptp_clock_sync		*ptp;
+	struct ptp_clock_result_msg	msg;
+	int				size;
+};
+
+struct ptp_marker_buf {
+	int local_cid;
+	int remote_cid;
+	int count;
+	int packet_id;
+} __packed;
+
+struct ptp_marker {
+	int series_id;
+	struct ptp_marker_buf data;
+} __packed;
+
+static int ptp_clock_sync_init(struct tracecmd_time_sync *tsync)
+{
+	const char *systems[] = {"ftrace", NULL};
+	struct clock_sync_context *clock_context;
+	struct ptp_clock_sync *ptp;
+	struct tep_event *raw;
+	char *path;
+
+	if (!tsync || !tsync->context)
+		return -1;
+	clock_context = (struct clock_sync_context *)tsync->context;
+	if (clock_context->proto_data)
+		return 0;
+
+	ptp = calloc(1, sizeof(struct ptp_clock_sync));
+	if (!ptp)
+		return -1;
+
+	ptp->marker_fd = -1;
+	ptp->debug_fd = -1;
+
+	path = tracefs_instance_get_dir(clock_context->instance);
+	if (!path)
+		goto error;
+	ptp->tep = tracefs_local_events_system(path, systems);
+	tracefs_put_tracing_file(path);
+	if (!ptp->tep)
+		goto error;
+	raw = tep_find_event_by_name(ptp->tep, "ftrace", "raw_data");
+	if (!raw)
+		goto error;
+	ptp->id = tep_find_field(raw, "id");
+	if (!ptp->id)
+		goto error;
+	ptp->raw_id = raw->id;
+
+	tep_set_file_bigendian(ptp->tep, tracecmd_host_bigendian());
+	tep_set_local_bigendian(ptp->tep, tracecmd_host_bigendian());
+
+	path = tracefs_instance_get_file(clock_context->instance, "trace_marker_raw");
+	if (!path)
+		goto error;
+	ptp->marker_fd = open(path, O_WRONLY);
+	tracefs_put_tracing_file(path);
+
+	clock_context->proto_data = ptp;
+
+#ifdef TSYNC_DEBUG
+	if (clock_context->is_server) {
+		char buff[256];
+		int res_fd;
+
+		sprintf(buff, "res-cid%d.txt", clock_context->remote_cid);
+
+		res_fd = open(buff, O_CREAT|O_WRONLY|O_TRUNC, 0644);
+		if (res_fd > 0)
+			close(res_fd);
+	}
+#endif
+
+	return 0;
+
+error:
+	if (ptp) {
+		tep_free(ptp->tep);
+		if (ptp->marker_fd >= 0)
+			close(ptp->marker_fd);
+	}
+	free(ptp);
+	return -1;
+}
+
+static int ptp_clock_sync_free(struct tracecmd_time_sync *tsync)
+{
+	struct clock_sync_context *clock_context;
+	struct ptp_clock_sync *ptp;
+
+	if (!tsync || !tsync->context)
+		return -1;
+	clock_context = (struct clock_sync_context *)tsync->context;
+
+	if (clock_context && clock_context->proto_data) {
+		ptp = (struct ptp_clock_sync *)clock_context->proto_data;
+		tep_free(ptp->tep);
+		if (ptp->marker_fd >= 0)
+			close(ptp->marker_fd);
+		if (ptp->debug_fd >= 0)
+			close(ptp->debug_fd);
+		free(clock_context->proto_data);
+		clock_context->proto_data = NULL;
+	}
+	return 0;
+}
+
+/* Save the timestamps of sent ('s') and returned ('r') probes in the
+ * ctx->msg.samples[] array. Depending of the context (server or client), there
+ * may be only returned probes, or both sent and returned probes. The returned
+ * probes are saved first in the array, after them are the sent probes.
+ * Depending of the context, the array can be with size:
+ *  [0 .. max data.count] - holds only returned probes
+ *  [0 .. 2 * max data.count] - holds both returned and sent probes
+ *  */
+static void ptp_probe_store(struct ptp_markers_context *ctx,
+			    struct ptp_marker *marker,
+			    unsigned long long ts)
+{
+	int index = -1;
+
+	if (marker->data.packet_id == 'r' &&
+	    marker->data.count <= ctx->size) {
+		index = marker->data.count - 1;
+	} else if (marker->data.packet_id == 's' &&
+		  marker->data.count * 2 <= ctx->size){
+		index = ctx->size / 2 + marker->data.count - 1;
+	}
+
+	if (index >= 0) {
+		ctx->msg.samples[index].id = marker->data.count;
+		ctx->msg.samples[index].ts = ts;
+		ctx->msg.count++;
+	}
+}
+
+static int ptp_marker_find(struct tep_event *event, struct tep_record *record,
+			   int cpu, void *context)
+{
+	struct ptp_markers_context *ctx;
+	struct ptp_marker *marker;
+
+	ctx = (struct ptp_markers_context *)context;
+
+	/* Make sure this is our event */
+	if (event->id != ctx->ptp->raw_id || !ctx->ptp->id)
+		return 0;
+	if (record->size >= (ctx->ptp->id->offset + sizeof(struct ptp_marker))) {
+		marker = (struct ptp_marker *)(record->data + ctx->ptp->id->offset);
+		if (marker->data.local_cid == ctx->clock->local_cid &&
+		    marker->data.remote_cid == ctx->clock->remote_cid &&
+		    marker->series_id == ctx->ptp->series_id &&
+		    marker->data.count)
+			ptp_probe_store(ctx, marker, record->ts);
+	}
+
+	return 0;
+}
+
+static inline bool good_probe(struct ptp_clock_sample *server_sample,
+			      struct ptp_clock_sample *send_sample,
+			      struct ptp_clock_sample *client_sample,
+			      int *bad_probes)
+{
+	if (server_sample->ts && send_sample->ts && client_sample->ts &&
+	    server_sample->id == send_sample->id &&
+	    server_sample->id == client_sample->id)
+		return true;
+	(*bad_probes)++;
+	return false;
+}
+
+static int ptp_calc_offset_fastest(struct clock_sync_context *clock,
+			   struct ptp_clock_result_msg *server,
+			   struct ptp_clock_result_msg *client,
+			   long long *offset_ret, long long *ts_ret,
+			   int *bad_probes)
+{
+	struct ptp_clock_sample *sample_send;
+	long long delta_min = LLONG_MAX;
+	long long offset = 0;
+	long long delta = 0;
+	long long ts = 0;
+	int max_i;
+	int i;
+
+	*bad_probes = 0;
+	sample_send = server->samples + (server->count / 2);
+	max_i = server->count / 2 < client->count ?
+		server->count / 2 : client->count;
+	for (i = 0; i < max_i; i++) {
+		if (!good_probe(&server->samples[i], &sample_send[i],
+		    &client->samples[i], bad_probes))
+			continue;
+		ts = (sample_send[i].ts + server->samples[i].ts) / 2;
+		offset = client->samples[i].ts - ts;
+
+		delta = server->samples[i].ts - sample_send[i].ts;
+		if (delta_min > delta) {
+			delta_min = delta;
+			*offset_ret = offset;
+			*ts_ret = ts;
+		}
+#ifdef TSYNC_DEBUG
+		{
+			struct ptp_clock_sync *ptp;
+
+			ptp = (struct ptp_clock_sync *)clock->proto_data;
+			if (ptp && ptp->debug_fd > 0) {
+				char buff[256];
+
+				sprintf(buff, "%lld %lld %lld\n",
+					ts, client->samples[i].ts, offset);
+				write(ptp->debug_fd, buff, strlen(buff));
+			}
+		}
+#endif
+	}
+
+	return 0;
+}
+
+static int ptp_calc_offset_hist(struct clock_sync_context *clock,
+			   struct ptp_clock_result_msg *server,
+			   struct ptp_clock_result_msg *client,
+			   long long *offset_ret, long long *ts_ret,
+			   int *bad_probes)
+{
+	struct ptp_clock_sample *sample_send;
+	long long timestamps[PTP_SYNC_LOOP];
+	long long offsets[PTP_SYNC_LOOP];
+	long long offset_min = LLONG_MAX;
+	long long offset_max = 0;
+	int hist[PTP_SYNC_LOOP];
+	int ind, max = 0;
+	long long bin;
+	int i, k = 0;
+
+	*bad_probes = 0;
+	memset(hist, 0, sizeof(int) * PTP_SYNC_LOOP);
+	sample_send = server->samples + (server->count / 2);
+	for (i = 0; i * 2 < server->count && i < client->count; i++) {
+		if (!good_probe(&server->samples[i], &sample_send[i],
+		    &client->samples[i], bad_probes))
+			continue;
+		timestamps[k] = (sample_send[i].ts + server->samples[i].ts) / 2;
+		offsets[k] = client->samples[i].ts - timestamps[k];
+		if (offset_max < llabs(offsets[k]))
+			offset_max = llabs(offsets[k]);
+		if (offset_min > llabs(offsets[k]))
+			offset_min = llabs(offsets[k]);
+#ifdef TSYNC_DEBUG
+		{
+			struct ptp_clock_sync *ptp;
+
+			ptp = (struct ptp_clock_sync *)clock->proto_data;
+
+			if (ptp && ptp->debug_fd > 0) {
+				char buff[256];
+
+				sprintf(buff, "%lld %lld %lld\n",
+					timestamps[k],
+					client->samples[i].ts, offsets[k]);
+				write(ptp->debug_fd, buff, strlen(buff));
+			}
+		}
+#endif
+		k++;
+	}
+
+	bin = (offset_max - offset_min) / PTP_SYNC_LOOP;
+	for (i = 0; i < k; i++) {
+		ind = (llabs(offsets[i]) - offset_min) / bin;
+		if (ind < PTP_SYNC_LOOP) {
+			hist[ind]++;
+			if (max < hist[ind]) {
+				max = hist[ind];
+				*offset_ret = offsets[i];
+				*ts_ret = timestamps[i];
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void ntoh_ptp_results(struct ptp_clock_result_msg *msg)
+{
+	int i;
+
+	msg->count = ntohl(msg->count);
+	for (i = 0; i < msg->count; i++) {
+		msg->samples[i].id = ntohl(msg->samples[i].id);
+		msg->samples[i].ts = ntohll(msg->samples[i].ts);
+	}
+	msg->series_id = ntohl(msg->series_id);
+}
+
+
+static void hton_ptp_results(struct ptp_clock_result_msg *msg)
+{
+	int i;
+
+	for (i = 0; i < msg->count; i++) {
+		msg->samples[i].id = htonl(msg->samples[i].id);
+		msg->samples[i].ts = htonll(msg->samples[i].ts);
+	}
+	msg->series_id = htonl(msg->series_id);
+	msg->count = htonl(msg->count);
+}
+
+static inline void ptp_track_clock(struct ptp_markers_context *ctx,
+				   struct ptp_marker *marker)
+{
+	if (ctx->ptp->flags & PTP_FLAG_USE_MARKER) {
+		write(ctx->ptp->marker_fd, marker, sizeof(struct ptp_marker));
+	} else {
+		struct timespec clock;
+		unsigned long long ts;
+
+		clock_gettime(CLOCK_MONOTONIC_RAW, &clock);
+		ts = clock.tv_sec * 1000000000LL;
+		ts += clock.tv_nsec;
+		ptp_probe_store(ctx, marker, ts);
+	}
+}
+
+static int ptp_clock_client(struct tracecmd_time_sync *tsync,
+			    long long *offset, long long *timestamp)
+{
+	char sync_proto[TRACECMD_TSYNC_PNAME_LENGTH];
+	struct clock_sync_context *clock_context;
+	struct ptp_clock_offset_msg res_offset;
+	struct ptp_clock_start_msg start;
+	struct ptp_markers_context ctx;
+	struct ptp_clock_sync *ptp;
+	struct ptp_marker marker;
+	unsigned int sync_msg;
+	unsigned int size;
+	char *msg;
+	int count;
+	int ret;
+
+	if (!tsync || !tsync->context || !tsync->msg_handle)
+		return -1;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+	if (clock_context->proto_data == NULL)
+		return -1;
+
+	ptp = (struct ptp_clock_sync *)clock_context->proto_data;
+	size = sizeof(start);
+	msg = (char *)&start;
+	ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+					  sync_proto, &sync_msg,
+					  &size, &msg);
+	if (ret || strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != PTP_SYNC_PKT_START)
+		return -1;
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+					  PTP_SYNC_PKT_START, sizeof(start),
+					  (char *)&start);
+	marker.data.local_cid = clock_context->local_cid;
+	marker.data.remote_cid = clock_context->remote_cid;
+	marker.series_id = ntohl(start.series_id);
+	marker.data.packet_id = 'r';
+	ptp->series_id = marker.series_id;
+	ptp->flags = ntohl(start.flags);
+	msg = (char *)&count;
+	size = sizeof(count);
+	ctx.msg.count = 0;
+	ctx.size = PTP_SYNC_LOOP;
+	ctx.ptp = ptp;
+	ctx.clock = clock_context;
+	ctx.msg.series_id = ptp->series_id;
+	while (true) {
+		count = 0;
+		ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+						  sync_proto, &sync_msg,
+						  &size, &msg);
+		if (ret || strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+		    sync_msg != PTP_SYNC_PKT_PROBE || !ntohl(count))
+			break;
+		marker.data.count = ntohl(count);
+		ptp_track_clock(&ctx, &marker);
+		ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+						  PTP_SYNC_PKT_PROBE,
+						  sizeof(count), (char *)&count);
+		if (ret)
+			break;
+	}
+
+	if (strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != PTP_SYNC_PKT_END)
+		return -1;
+
+	if (ptp->flags & PTP_FLAG_USE_MARKER)
+		tracefs_iterate_raw_events(ptp->tep, clock_context->instance,
+					   ptp_marker_find, &ctx);
+
+	hton_ptp_results(&ctx.msg);
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+					  PTP_SYNC_PKT_PROBES,
+					  sizeof(ctx.msg), (char *)&ctx.msg);
+
+	msg = (char *)&res_offset;
+	size = sizeof(res_offset);
+	ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+					  sync_proto, &sync_msg,
+					  &size, (char **)&msg);
+	if (ret || strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != PTP_SYNC_PKT_OFFSET)
+		return -1;
+
+	*offset = ntohll(res_offset.offset);
+	*timestamp = ntohll(res_offset.ts);
+
+	return 0;
+}
+
+
+static int ptp_clock_server(struct tracecmd_time_sync *tsync,
+			    long long *offset, long long *timestamp)
+{
+	char sync_proto[TRACECMD_TSYNC_PNAME_LENGTH];
+	struct ptp_clock_result_msg *results = NULL;
+	struct clock_sync_context *clock_context;
+	struct ptp_clock_offset_msg res_offset;
+	struct ptp_clock_start_msg start;
+	struct ptp_markers_context ctx;
+	int sync_loop = PTP_SYNC_LOOP;
+	struct ptp_clock_sync *ptp;
+	struct ptp_marker marker;
+	unsigned int sync_msg;
+	unsigned int size;
+	int bad_probes;
+	int count = 1;
+	int msg_count;
+	int msg_ret;
+	char *msg;
+	int ret;
+
+	if (!tsync || !tsync->context || !tsync->msg_handle)
+		return -1;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+	if (clock_context->proto_data == NULL)
+		return -1;
+
+	ptp = (struct ptp_clock_sync *)clock_context->proto_data;
+	ptp->flags = ptp_flags;
+	memset(&start, 0, sizeof(start));
+	start.series_id = htonl(ptp->series_id + 1);
+	start.flags = htonl(ptp->flags);
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+					 PTP_SYNC_PKT_START, sizeof(start),
+					 (char *)&start);
+	if (!ret)
+		ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+						  sync_proto, &sync_msg,
+						  NULL, NULL);
+	if (ret || strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != PTP_SYNC_PKT_START)
+		return -1;
+
+	tracefs_instance_file_write(clock_context->instance, "trace", "\0");
+
+	ptp->series_id++;
+	marker.data.local_cid = clock_context->local_cid;
+	marker.data.remote_cid = clock_context->remote_cid;
+	marker.series_id = ptp->series_id;
+	msg = (char *)&msg_ret;
+	size = sizeof(msg_ret);
+	ctx.size = 2*PTP_SYNC_LOOP;
+	ctx.ptp = ptp;
+	ctx.clock = clock_context;
+	ctx.msg.count = 0;
+	ctx.msg.series_id = ptp->series_id;
+	do {
+		marker.data.count = count++;
+		marker.data.packet_id = 's';
+		msg_count = htonl(marker.data.count);
+		ptp_track_clock(&ctx, &marker);
+		ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+						 PTP_SYNC_PKT_PROBE,
+						 sizeof(msg_count),
+						 (char *)&msg_count);
+		if (!ret)
+			ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+							  sync_proto, &sync_msg,
+							  &size, &msg);
+
+		marker.data.packet_id = 'r';
+		ptp_track_clock(&ctx, &marker);
+		if (ret || strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+		    sync_msg != PTP_SYNC_PKT_PROBE ||
+		    ntohl(msg_ret) != marker.data.count)
+			break;
+	} while (--sync_loop);
+
+	if (sync_loop)
+		return -1;
+
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+					  PTP_SYNC_PKT_END, 0, NULL);
+
+	size = 0;
+	ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+					  sync_proto, &sync_msg,
+					  &size, (char **)&results);
+	if (ret || strncmp(sync_proto, PTP_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != PTP_SYNC_PKT_PROBES || size == 0 || results == NULL)
+		return -1;
+
+	ntoh_ptp_results(results);
+	if (ptp->flags & PTP_FLAG_USE_MARKER)
+		tracefs_iterate_raw_events(ptp->tep, clock_context->instance,
+					   ptp_marker_find, &ctx);
+	if (ptp->flags & PTP_FLAG_FASTEST_RESPONSE)
+		ptp_calc_offset_fastest(clock_context, &ctx.msg, results, offset,
+					timestamp, &bad_probes);
+	else
+		ptp_calc_offset_hist(clock_context, &ctx.msg, results, offset,
+				     timestamp, &bad_probes);
+#ifdef TSYNC_DEBUG
+	{
+		char buff[256];
+		int res_fd;
+
+		sprintf(buff, "res-cid%d.txt", clock_context->remote_cid);
+
+		res_fd = open(buff, O_WRONLY|O_APPEND, 0644);
+		if (res_fd > 0) {
+			if (*offset && *timestamp) {
+				sprintf(buff, "%d %lld %lld\n",
+					ptp->series_id, *offset, *timestamp);
+				write(res_fd, buff, strlen(buff));
+			}
+			close(res_fd);
+		}
+
+		printf("\n calculated offset %d: %lld, %d probes, filtered out %d, PTP flags 0x%X\n\r",
+			ptp->series_id, *offset, results->count, bad_probes, ptp->flags);
+		if (ptp && ptp->debug_fd > 0) {
+			sprintf(buff, "%lld %lld 0\n", *offset, *timestamp);
+			write(ptp->debug_fd, buff, strlen(buff));
+			close(ptp->debug_fd);
+			ptp->debug_fd = -1;
+		}
+
+	}
+#endif
+
+	res_offset.offset = htonll(*offset);
+	res_offset.ts = htonll(*timestamp);
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, PTP_NAME,
+					  PTP_SYNC_PKT_OFFSET,
+					  sizeof(res_offset),
+					  (char *)&res_offset);
+
+	free(results);
+	return 0;
+}
+
+static int ptp_clock_sync_calc(struct tracecmd_time_sync *tsync,
+			       long long *offset, long long *scaling,
+			       long long *timestamp, unsigned int cpu)
+{
+	struct clock_sync_context *clock_context;
+	int ret;
+
+	if (!tsync || !tsync->context)
+		return -1;
+	clock_context = (struct clock_sync_context *)tsync->context;
+
+#ifdef TSYNC_DEBUG
+	if (clock_context->is_server) {
+		struct ptp_clock_sync *ptp;
+		char buff[256];
+
+		ptp = (struct ptp_clock_sync *)clock_context->proto_data;
+		if (ptp->debug_fd > 0)
+			close(ptp->debug_fd);
+		sprintf(buff, "s-cid%d_%d.txt",
+				clock_context->remote_cid, ptp->series_id+1);
+		ptp->debug_fd = open(buff, O_CREAT|O_WRONLY|O_TRUNC, 0644);
+	}
+#endif
+
+	if (scaling)
+		*scaling = 1;
+	if (clock_context->is_server)
+		ret = ptp_clock_server(tsync, offset, timestamp);
+	else
+		ret = ptp_clock_client(tsync, offset, timestamp);
+
+	return ret;
+}
+
+int ptp_clock_sync_register(void)
+{
+	return tracecmd_tsync_proto_register(PTP_NAME, PTP_ACCURACY,
+					     TRACECMD_TIME_SYNC_ROLE_GUEST |
+					     TRACECMD_TIME_SYNC_ROLE_HOST,
+					     0, TRACECMD_TSYNC_FLAG_INTERPOLATE,
+					     ptp_clock_sync_init,
+					     ptp_clock_sync_free,
+					     ptp_clock_sync_calc);
+
+}
+
+int ptp_clock_sync_unregister(void)
+{
+	return tracecmd_tsync_proto_unregister(PTP_NAME);
+}
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index c8c1f287..77d65f3b 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -64,7 +64,7 @@ static struct tsync_proto *tsync_proto_find(char *proto_name)
  */
 void tracecmd_tsync_init(void)
 {
-
+	ptp_clock_sync_register();
 }
 
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 15/16] trace-cmd: Debug scripts for PTP-like algorithm for host - guest timestamp synchronization
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (13 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 14/16] trace-cmd: [POC] PTP-like algorithm for host - guest timestamp synchronization Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  2020-10-29 11:18 ` [PATCH v25 16/16] trace-cmd [POC]: Add KVM timestamp synchronization plugin Tzvetomir Stoyanov (VMware)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

These scripts can be used to visualise debug files, written when the PTP-like algorithm
is compiled with TSYNC_DEBUG defined. The files are located in the guest machine:
    s-cid*.txt - For each offset calculation: host and guest clocks and calculated offset.
    res-cid*.txt - For each tracing session: all calculated clock offsets.

tsync_hist.py plots a histogram, using data from a s-cid*.txt file:
	"python tsync_hist.py s-cid2_1.txt"
tsync_res.py plots a line, using data from res-cid*.txt file:
	"python tsync_res.py res-cid2.txt"

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 scripts/debug/tsync_hist.py | 57 +++++++++++++++++++++++++++++++++++++
 scripts/debug/tsync_readme  | 12 ++++++++
 scripts/debug/tsync_res.py  | 46 ++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 scripts/debug/tsync_hist.py
 create mode 100644 scripts/debug/tsync_readme
 create mode 100644 scripts/debug/tsync_res.py

diff --git a/scripts/debug/tsync_hist.py b/scripts/debug/tsync_hist.py
new file mode 100644
index 00000000..819d1e8f
--- /dev/null
+++ b/scripts/debug/tsync_hist.py
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2019, VMware Inc, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+# Copyright (C) 2019, VMware Inc, Yordan Karadzhov <ykaradzhov@vmware.com>
+
+
+import matplotlib.pyplot as plt
+import matplotlib.lines as mlines
+import numpy as np
+import sys
+
+def newline(p1, p2):
+    ax = plt.gca()
+    xmin, xmax = ax.get_xbound()
+
+    if(p2[0] == p1[0]):
+        xmin = xmax = p1[0]
+        ymin, ymax = ax.get_ybound()
+    else:
+        ymax = p1[1]+(p2[1]-p1[1])/(p2[0]-p1[0])*(xmax-p1[0])
+        ymin = p1[1]+(p2[1]-p1[1])/(p2[0]-p1[0])*(xmin-p1[0])
+
+    l = mlines.Line2D([xmin,xmax], [ymin,ymax], color='red')
+    ax.add_line(l)
+    return l
+
+
+data = np.loadtxt(fname = sys.argv[1])
+selected_ts  = data[-1, 1]
+selected_ofs = data[-1, 0]
+data = data[:-1,:]
+
+x = data[:, 1] - data[:, 0]
+
+mean = x.mean()
+std = x.std()
+
+num_bins = 500
+min = x.min() #+ .4 * (x.max() - x.min())
+max = x.max() #- .4 * (x.max() - x.min())
+bins = np.linspace(min, max, num_bins, endpoint = False, dtype=int)
+
+fig, ax = plt.subplots()
+
+# the histogram of the data
+n, bins, patches = ax.hist(x, bins, histtype=u'step');
+
+ax.set_xlabel('clock offset [$\mu$s]')
+ax.set_ylabel('entries')
+ax.set_title("$\sigma$=%i" % std)
+
+x1, y1 = [selected_ofs, min], [selected_ofs, max]
+newline(x1, y1)
+
+# Tweak spacing to prevent clipping of ylabel
+fig.tight_layout()
+plt.show()
diff --git a/scripts/debug/tsync_readme b/scripts/debug/tsync_readme
new file mode 100644
index 00000000..f3ebb25d
--- /dev/null
+++ b/scripts/debug/tsync_readme
@@ -0,0 +1,12 @@
+PTP-like algorithm debug
+========================
+
+tsync_*.py scripts can be used to visualise debug files, written when the PTP-like algorithm
+is compiled with TSYNC_DEBUG defined. The files are located in the guest machine:
+    s-cid*.txt - For each offset calculation: host and guest clocks and calculated offset.
+    res-cid*.txt - For each tracing session: all calculated clock offsets.
+
+tsync_hist.py plots a histogram, using data from a s-cid*.txt file:
+	"python tsync_hist.py s-cid2_1.txt"
+tsync_res.py plots a line, using data from res-cid*.txt file:
+	"python tsync_res.py res-cid2.txt"
diff --git a/scripts/debug/tsync_res.py b/scripts/debug/tsync_res.py
new file mode 100644
index 00000000..7d109863
--- /dev/null
+++ b/scripts/debug/tsync_res.py
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2019, VMware Inc, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+# Copyright (C) 2019, VMware Inc, Yordan Karadzhov <ykaradzhov@vmware.com>
+
+
+import matplotlib.pyplot as plt
+import matplotlib.lines as mlines
+import numpy as np
+import sys
+
+def newline(p1, p2):
+    ax = plt.gca()
+    xmin, xmax = ax.get_xbound()
+
+    if(p2[0] == p1[0]):
+        xmin = xmax = p1[0]
+        ymin, ymax = ax.get_ybound()
+    else:
+        ymax = p1[1]+(p2[1]-p1[1])/(p2[0]-p1[0])*(xmax-p1[0])
+        ymin = p1[1]+(p2[1]-p1[1])/(p2[0]-p1[0])*(xmin-p1[0])
+
+    l = mlines.Line2D([xmin,xmax], [ymin,ymax], color='red')
+    ax.add_line(l)
+    return l
+
+data = np.loadtxt(fname = sys.argv[1])
+x = data[:, 0]
+y = data[:, 1]
+
+fig, ax = plt.subplots()
+
+ax.set_xlabel('samples (t)')
+ax.set_ylabel('clock offset')
+ax.set_title("$\delta$=%i ns" % (max(y) - min(y)))
+
+l = mlines.Line2D(x, y)
+ax.add_line(l)
+ax.set_xlim(min(x), max(x))
+ax.set_ylim(min(y), max(y) )
+
+print(min(y), max(y), max(y) - min(y))
+
+# Tweak spacing to prevent clipping of ylabel
+fig.tight_layout()
+plt.show()
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v25 16/16] trace-cmd [POC]: Add KVM timestamp synchronization plugin
  2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
                   ` (14 preceding siblings ...)
  2020-10-29 11:18 ` [PATCH v25 15/16] trace-cmd: Debug scripts for " Tzvetomir Stoyanov (VMware)
@ 2020-10-29 11:18 ` Tzvetomir Stoyanov (VMware)
  15 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-10-29 11:18 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Added new timestamp synchronization plugin for KVM hosts. It reads the
clock offsets directly from the KVM debug filesystem, if available.
The plugin works only with x86-tsc ftrace clock.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/Makefile                    |   1 +
 lib/trace-cmd/include/trace-tsync-local.h |   1 +
 lib/trace-cmd/trace-timesync-kvm.c        | 459 ++++++++++++++++++++++
 lib/trace-cmd/trace-timesync.c            |   1 +
 4 files changed, 462 insertions(+)
 create mode 100644 lib/trace-cmd/trace-timesync-kvm.c

diff --git a/lib/trace-cmd/Makefile b/lib/trace-cmd/Makefile
index 7cab7514..d1b8fc49 100644
--- a/lib/trace-cmd/Makefile
+++ b/lib/trace-cmd/Makefile
@@ -19,6 +19,7 @@ OBJS += trace-plugin.o
 ifeq ($(VSOCK_DEFINED), 1)
 OBJS += trace-timesync.o
 OBJS += trace-timesync-ptp.o
+OBJS += trace-timesync-kvm.o
 endif
 
 # Additional util objects
diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
index a25eae27..d6dd7ab4 100644
--- a/lib/trace-cmd/include/trace-tsync-local.h
+++ b/lib/trace-cmd/include/trace-tsync-local.h
@@ -49,5 +49,6 @@ int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
 int tracecmd_tsync_proto_unregister(char *proto_name);
 
 int ptp_clock_sync_register(void);
+int kvm_clock_sync_register(void);
 
 #endif /* _TRACE_TSYNC_LOCAL_H */
diff --git a/lib/trace-cmd/trace-timesync-kvm.c b/lib/trace-cmd/trace-timesync-kvm.c
new file mode 100644
index 00000000..ba3a958a
--- /dev/null
+++ b/lib/trace-cmd/trace-timesync-kvm.c
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2020, VMware, Tzvetomir Stoyanov tz.stoyanov@gmail.com>
+ *
+ */
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <dirent.h>
+
+#include "trace-cmd.h"
+#include "tracefs.h"
+#include "trace-tsync-local.h"
+
+#define KVM_DEBUG_FS "/sys/kernel/debug/kvm"
+#define KVM_DEBUG_OFFSET_FILE	"tsc-offset"
+#define KVM_DEBUG_SCALING_FILE	"tsc-scaling-ratio"
+#define KVM_DEBUG_VCPU_DIR	"vcpu"
+
+#define KVM_SYNC_PKT_REQUEST	1
+#define KVM_SYNC_PKT_RESPONSE	2
+
+typedef __s64 s64;
+
+// equal to /sys/kernel/debug/kvm/<VM ID>/vcpu0/tsc-offset
+#define KVM_ACCURACY	0
+#define KVM_NAME	"kvm"
+
+struct kvm_clock_sync {
+	int vcpu_count;
+	char **vcpu_offsets;
+	char **vcpu_scalings;
+	int marker_fd;
+	struct tep_handle *tep;
+	int raw_id;
+	unsigned long long ts;
+};
+
+struct kvm_clock_offset_msg {
+	s64	ts;
+	s64	offset;
+	s64	scaling;
+};
+
+static bool kvm_support_check(bool guest)
+{
+	struct stat st;
+	int ret;
+
+	if (guest)
+		return true;
+
+	ret = stat(KVM_DEBUG_FS, &st);
+	if (ret < 0)
+		return false;
+
+	if (!S_ISDIR(st.st_mode))
+		return false;
+	return true;
+}
+
+static int kvm_open_vcpu_dir(struct kvm_clock_sync *kvm, int cpu, char *dir_str)
+{
+	struct dirent *entry;
+	char path[PATH_MAX];
+	DIR *dir;
+
+	dir = opendir(dir_str);
+	if (!dir)
+		goto error;
+	while ((entry = readdir(dir))) {
+		if (entry->d_type != DT_DIR) {
+			if (!strncmp(entry->d_name, KVM_DEBUG_OFFSET_FILE,
+				     strlen(KVM_DEBUG_OFFSET_FILE))) {
+				snprintf(path, sizeof(path), "%s/%s",
+					 dir_str, entry->d_name);
+				kvm->vcpu_offsets[cpu] = strdup(path);
+			}
+			if (!strncmp(entry->d_name, KVM_DEBUG_SCALING_FILE,
+				     strlen(KVM_DEBUG_SCALING_FILE))) {
+				snprintf(path, sizeof(path), "%s/%s",
+					 dir_str, entry->d_name);
+				kvm->vcpu_scalings[cpu] = strdup(path);
+			}
+		}
+	}
+	if (!kvm->vcpu_offsets[cpu])
+		goto error;
+	closedir(dir);
+	return 0;
+
+error:
+	if (dir)
+		closedir(dir);
+	free(kvm->vcpu_offsets[cpu]);
+	kvm->vcpu_offsets[cpu] = NULL;
+	free(kvm->vcpu_scalings[cpu]);
+	kvm->vcpu_scalings[cpu] = NULL;
+	return -1;
+}
+
+static int kvm_open_debug_files(struct kvm_clock_sync *kvm, int pid)
+{
+	char *vm_dir_str = NULL;
+	struct dirent *entry;
+	char *pid_str = NULL;
+	char path[PATH_MAX];
+	long vcpu;
+	DIR *dir;
+	int i;
+
+	dir = opendir(KVM_DEBUG_FS);
+	if (!dir)
+		goto error;
+	if (asprintf(&pid_str, "%d-", pid) <= 0)
+		goto error;
+	while ((entry = readdir(dir))) {
+		if (!(entry->d_type == DT_DIR &&
+		    !strncmp(entry->d_name, pid_str, strlen(pid_str))))
+			continue;
+		asprintf(&vm_dir_str, "%s/%s", KVM_DEBUG_FS, entry->d_name);
+		break;
+	}
+	closedir(dir);
+	dir = NULL;
+	if (!vm_dir_str)
+		goto error;
+	dir = opendir(vm_dir_str);
+	if (!dir)
+		goto error;
+	while ((entry = readdir(dir))) {
+		if (!(entry->d_type == DT_DIR &&
+		    !strncmp(entry->d_name, KVM_DEBUG_VCPU_DIR, strlen(KVM_DEBUG_VCPU_DIR))))
+			continue;
+		vcpu =  strtol(entry->d_name + strlen(KVM_DEBUG_VCPU_DIR), NULL, 10);
+		if (vcpu < 0 || vcpu >= kvm->vcpu_count)
+			continue;
+		snprintf(path, sizeof(path), "%s/%s", vm_dir_str, entry->d_name);
+		if (kvm_open_vcpu_dir(kvm, vcpu, path) < 0)
+			goto error;
+	}
+	for (i = 0; i < kvm->vcpu_count; i++) {
+		if (!kvm->vcpu_offsets[i])
+			goto error;
+	}
+	closedir(dir);
+	free(pid_str);
+	free(vm_dir_str);
+	return 0;
+error:
+	free(pid_str);
+	free(vm_dir_str);
+	if (dir)
+		closedir(dir);
+	return -1;
+}
+
+static int kvm_clock_sync_init_host(struct tracecmd_time_sync *tsync,
+				    struct kvm_clock_sync *kvm)
+{
+	kvm->vcpu_count = tsync->vcpu_count;
+	kvm->vcpu_offsets = calloc(kvm->vcpu_count, sizeof(char *));
+	kvm->vcpu_scalings = calloc(kvm->vcpu_count, sizeof(char *));
+	if (!kvm->vcpu_offsets || !kvm->vcpu_scalings)
+		goto error;
+	if (kvm_open_debug_files(kvm, tsync->guest_pid) < 0)
+		goto error;
+	return 0;
+
+error:
+	free(kvm->vcpu_offsets);
+	free(kvm->vcpu_scalings);
+	return -1;
+}
+
+static int kvm_clock_sync_init_guest(struct tracecmd_time_sync *tsync,
+				     struct kvm_clock_sync *kvm)
+{
+	const char *systems[] = {"ftrace", NULL};
+	struct clock_sync_context *clock_context;
+	struct tep_event *raw;
+	char *path;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+	path = tracefs_instance_get_dir(clock_context->instance);
+	if (!path)
+		goto error;
+	kvm->tep = tracefs_local_events_system(path, systems);
+	tracefs_put_tracing_file(path);
+	if (!kvm->tep)
+		goto error;
+	raw = tep_find_event_by_name(kvm->tep, "ftrace", "raw_data");
+	if (!raw)
+		goto error;
+
+	kvm->raw_id = raw->id;
+	tep_set_file_bigendian(kvm->tep, tracecmd_host_bigendian());
+	tep_set_local_bigendian(kvm->tep, tracecmd_host_bigendian());
+
+	path = tracefs_instance_get_file(clock_context->instance, "trace_marker_raw");
+	if (!path)
+		goto error;
+	kvm->marker_fd = open(path, O_WRONLY);
+	tracefs_put_tracing_file(path);
+
+	return 0;
+
+error:
+	if (kvm->tep)
+		tep_free(kvm->tep);
+	if (kvm->marker_fd >= 0)
+		close(kvm->marker_fd);
+
+	return -1;
+}
+
+static int kvm_clock_sync_init(struct tracecmd_time_sync *tsync)
+{
+	struct clock_sync_context *clock_context;
+	struct kvm_clock_sync *kvm;
+	int ret;
+
+	if (!tsync || !tsync->context)
+		return -1;
+	clock_context = (struct clock_sync_context *)tsync->context;
+
+	if (!kvm_support_check(clock_context->is_guest))
+		return -1;
+	kvm = calloc(1, sizeof(struct kvm_clock_sync));
+	if (!kvm)
+		return -1;
+	kvm->marker_fd = -1;
+	if (clock_context->is_guest)
+		ret = kvm_clock_sync_init_guest(tsync, kvm);
+	else
+		ret = kvm_clock_sync_init_host(tsync, kvm);
+	if (ret < 0)
+		goto error;
+
+	clock_context->proto_data = kvm;
+	return 0;
+
+error:
+	free(kvm);
+	return -1;
+}
+
+static int kvm_clock_sync_free(struct tracecmd_time_sync *tsync)
+{
+	struct clock_sync_context *clock_context;
+	struct kvm_clock_sync *kvm = NULL;
+	int i;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+	if (clock_context)
+		kvm = (struct kvm_clock_sync *)clock_context->proto_data;
+	if (kvm) {
+		for (i = 0; i < kvm->vcpu_count; i++) {
+			free(kvm->vcpu_offsets[i]);
+			kvm->vcpu_offsets[i] = NULL;
+			free(kvm->vcpu_scalings[i]);
+			kvm->vcpu_scalings[i] = NULL;
+		}
+		if (kvm->tep)
+			tep_free(kvm->tep);
+		if (kvm->marker_fd >= 0)
+			close(kvm->marker_fd);
+		free(kvm);
+	}
+	return -1;
+}
+
+static int read_ll_form_file(char *file, long long *res)
+{
+	char buf[32];
+	int ret;
+	int fd;
+
+	if (!file)
+		return -1;
+	fd = open(file, O_RDONLY | O_NONBLOCK);
+	if (fd < 0)
+		return -1;
+	ret = read(fd, buf, 32);
+	close(fd);
+	if (ret <= 0)
+		return -1;
+
+	*res = strtoll(buf, NULL, 10);
+
+	return 0;
+}
+
+static int kvm_clock_host(struct tracecmd_time_sync *tsync,
+			  long long *offset, long long *scaling,
+			  long long *timestamp, unsigned int cpu)
+{
+	char sync_proto[TRACECMD_TSYNC_PNAME_LENGTH];
+	struct clock_sync_context *clock_context;
+	struct kvm_clock_offset_msg packet;
+	struct kvm_clock_sync *kvm = NULL;
+	long long kvm_scaling = 1;
+	unsigned int sync_msg;
+	long long kvm_offset;
+	unsigned int size;
+	char *msg;
+	int ret;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+	if (clock_context)
+		kvm = (struct kvm_clock_sync *)clock_context->proto_data;
+	if (!kvm || !kvm->vcpu_offsets || !kvm->vcpu_offsets[0])
+		return -1;
+	if (cpu >= kvm->vcpu_count)
+		return -1;
+	ret = read_ll_form_file(kvm->vcpu_offsets[cpu], &kvm_offset);
+	if (ret < 0)
+		return -1;
+	if (kvm->vcpu_scalings && kvm->vcpu_scalings[cpu])
+		read_ll_form_file(kvm->vcpu_scalings[cpu], &kvm_scaling);
+	msg = (char *)&packet;
+	size = sizeof(packet);
+	ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+					  sync_proto, &sync_msg,
+					  &size, &msg);
+	if (ret || strncmp(sync_proto, KVM_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != KVM_SYNC_PKT_REQUEST)
+		return -1;
+
+	packet.offset = -kvm_offset;
+	packet.scaling = kvm_scaling;
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, KVM_NAME,
+					  KVM_SYNC_PKT_RESPONSE, sizeof(packet),
+					  (char *)&packet);
+	if (ret)
+		return -1;
+
+	*scaling = packet.scaling;
+	*offset = packet.offset;
+	*timestamp = packet.ts;
+
+	return 0;
+}
+
+#define KVM_EVENT_MARKER	"kvm sync event"
+static int kvm_marker_find(struct tep_event *event, struct tep_record *record,
+			   int cpu, void *context)
+{
+	struct kvm_clock_sync *kvm = (struct kvm_clock_sync *)context;
+	struct tep_format_field *field;
+	struct tep_format_field *id;
+	char *marker;
+
+	/* Make sure this is our event */
+	if (event->id != kvm->raw_id)
+		return 0;
+	id = tep_find_field(event, "id");
+	field = tep_find_field(event, "buf");
+	if (field && id &&
+	    record->size >= (id->offset + strlen(KVM_EVENT_MARKER) + 1)) {
+		marker = (char *)(record->data + id->offset);
+		if (!strcmp(marker, KVM_EVENT_MARKER)) {
+			kvm->ts = record->ts;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+
+static int kvm_clock_guest(struct tracecmd_time_sync *tsync,
+			   long long *offset,
+			   long long *scaling,
+			   long long *timestamp)
+{
+	char sync_proto[TRACECMD_TSYNC_PNAME_LENGTH];
+	struct clock_sync_context *clock_context;
+	struct kvm_clock_offset_msg packet;
+	struct kvm_clock_sync *kvm = NULL;
+	unsigned int sync_msg;
+	unsigned int size;
+	char *msg;
+	int ret;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+	if (clock_context)
+		kvm = (struct kvm_clock_sync *)clock_context->proto_data;
+	if (!kvm)
+		return -1;
+	kvm->ts = 0;
+	memset(&packet, 0, sizeof(packet));
+	tracefs_instance_file_write(clock_context->instance, "trace", "\0");
+	write(kvm->marker_fd, KVM_EVENT_MARKER, strlen(KVM_EVENT_MARKER) + 1);
+	kvm->ts = 0;
+	tracefs_iterate_raw_events(kvm->tep, clock_context->instance,
+				   kvm_marker_find, kvm);
+	packet.ts = kvm->ts;
+	ret = tracecmd_msg_send_time_sync(tsync->msg_handle, KVM_NAME,
+					  KVM_SYNC_PKT_REQUEST, sizeof(packet),
+					  (char *)&packet);
+	if (ret)
+		return -1;
+	msg = (char *)&packet;
+	size = sizeof(packet);
+	ret = tracecmd_msg_recv_time_sync(tsync->msg_handle,
+					  sync_proto, &sync_msg,
+					  &size, &msg);
+	if (ret || strncmp(sync_proto, KVM_NAME, TRACECMD_TSYNC_PNAME_LENGTH) ||
+	    sync_msg != KVM_SYNC_PKT_RESPONSE)
+		return -1;
+
+	*scaling = packet.scaling;
+	*offset = packet.offset;
+	*timestamp = packet.ts;
+	return 0;
+}
+
+static int kvm_clock_sync_calc(struct tracecmd_time_sync *tsync,
+			       long long *offset, long long *scaling,
+			       long long *timestamp, unsigned int cpu)
+{
+	struct clock_sync_context *clock_context;
+	int ret;
+
+	if (!tsync || !tsync->context)
+		return -1;
+
+	clock_context = (struct clock_sync_context *)tsync->context;
+
+	if (clock_context->is_guest)
+		ret = kvm_clock_guest(tsync, offset, scaling, timestamp);
+	else
+		ret = kvm_clock_host(tsync, offset, scaling, timestamp, cpu);
+	return ret;
+}
+
+int kvm_clock_sync_register(void)
+{
+	int role = TRACECMD_TIME_SYNC_ROLE_GUEST;
+	int clock = 0;
+
+	if (kvm_support_check(false)) {
+		role |= TRACECMD_TIME_SYNC_ROLE_HOST;
+		clock = TRACECMD_CLOCK_X86_TSC;
+	}
+	return tracecmd_tsync_proto_register(KVM_NAME, KVM_ACCURACY,
+					     role, clock, 0,
+					     kvm_clock_sync_init,
+					     kvm_clock_sync_free,
+					     kvm_clock_sync_calc);
+}
+
+int kvm_clock_sync_unregister(void)
+{
+	return tracecmd_tsync_proto_unregister(KVM_NAME);
+}
diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c
index 77d65f3b..31e0e2e0 100644
--- a/lib/trace-cmd/trace-timesync.c
+++ b/lib/trace-cmd/trace-timesync.c
@@ -65,6 +65,7 @@ static struct tsync_proto *tsync_proto_find(char *proto_name)
 void tracecmd_tsync_init(void)
 {
 	ptp_clock_sync_register();
+	kvm_clock_sync_register();
 }
 
 int tracecmd_tsync_proto_register(char *proto_name, int accuracy, int roles,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string
  2020-10-29 11:18 ` [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string Tzvetomir Stoyanov (VMware)
@ 2020-11-05 14:52   ` Steven Rostedt
  2020-12-02 22:43   ` Steven Rostedt
  1 sibling, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2020-11-05 14:52 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:01 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Different timestamp synchronization algorithms will be implemented as
> trace-cmd plugins. Using IDs for identifying these algorithms is
> a problem, as these IDs must be defined in a single place. In order
> to be more flexible, protocol IDs are replaced by protocol names -
> strings that are part of the plugin code and are registered with
> plugin initialisation.
> The plugin names are limited up to 15 symbols, in order to simplify
> the structure of sync messages.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h             |  32 +++---
>  lib/trace-cmd/include/trace-tsync-local.h |  12 +-
>  lib/trace-cmd/trace-msg.c                 | 102 ++++++++++------
>  lib/trace-cmd/trace-timesync.c            | 134 ++++++++++------------
>  tracecmd/include/trace-local.h            |   5 +-
>  tracecmd/trace-agent.c                    |   8 +-
>  tracecmd/trace-record.c                   |  19 +--
>  tracecmd/trace-tsync.c                    |  19 ++-
>  8 files changed, 170 insertions(+), 161 deletions(-)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index f3c95f30..8d3bea73 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -385,50 +385,44 @@ void tracecmd_msg_set_done(struct tracecmd_msg_handle *msg_handle);
>  int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
>  				int argc, char **argv, bool use_fifos,
>  				unsigned long long trace_id,
> -				char *tsync_protos,
> -				int tsync_protos_size);
> +				char **tsync_protos);

Probably should make these const char * const *
(I'll have to check if I place those correctly ;-)

As I believe this is passed in and not modified.

>  int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
>  				int *argc, char ***argv, bool *use_fifos,
>  				unsigned long long *trace_id,
> -				char **tsync_protos,
> -				unsigned int *tsync_protos_size);
> +				char ***tsync_protos);

Hmm, maybe we need to make this into its own structure that handles the
array, because char *** is starting to get too complex. Prehaps we should
just abstract it out!

struct tracecmd_tsync_protos;

int tracecmd_msg_send_trace_req(struct tracecmd_msg_handle *msg_handle,
				int argc, char **argv, bool use_fifos,
				unsigned long long trace_id,
				struct tracecmd_tsync_protos *tsync_protos);

int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
				int *argc, char ***argv, bool *use_fifos,
				unsigned long long *trace_id,
				struct tracecmd_tsync_protos **tsync_protos);

Then we have internally:

struct tracecmd_tsync_protos {
	char **protos;
}

That we play with, and have helper functions to extract the list for cases
that want to see all the protos. This would also give us flexibility to add
more complex operations to this later.

>  
>  int tracecmd_msg_send_trace_resp(struct tracecmd_msg_handle *msg_handle,
>  				 int nr_cpus, int page_size,
>  				 unsigned int *ports, bool use_fifos,
>  				 unsigned long long trace_id,
> -				 unsigned int tsync_proto,
> -				 unsigned int tsync_port);
> +				 char *tsync_proto, unsigned int tsync_port);
>  int tracecmd_msg_recv_trace_resp(struct tracecmd_msg_handle *msg_handle,
>  				 int *nr_cpus, int *page_size,
>  				 unsigned int **ports, bool *use_fifos,
>  				 unsigned long long *trace_id,
> -				 unsigned int *tsync_proto,
> +				 char **tsync_proto,
>  				 unsigned int *tsync_port);
>  
>  int tracecmd_msg_send_time_sync(struct tracecmd_msg_handle *msg_handle,
> -				unsigned int sync_protocol,
> -				unsigned int sync_msg_id,
> +				char *sync_protocol, unsigned int sync_msg_id,
>  				unsigned int payload_size, char *payload);
>  int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle,
> -				unsigned int *sync_protocol,
> +				char *sync_protocol,
>  				unsigned int *sync_msg_id,
>  				unsigned int *payload_size, char **payload);
>  
>  /* --- Timestamp synchronization --- */
>  
> -enum{
> -	TRACECMD_TIME_SYNC_PROTO_NONE	= 0,
> -};
> +#define TRACECMD_TSYNC_PNAME_LENGTH	16
> +#define TRACECMD_TSYNC_PROTO_NONE	"none"
> +
>  enum{
>  	TRACECMD_TIME_SYNC_CMD_PROBE	= 1,
>  	TRACECMD_TIME_SYNC_CMD_STOP	= 2,
>  };
>  
> -#define TRACECMD_TIME_SYNC_PROTO_PTP_WEIGHT	10
> -
>  struct tracecmd_time_sync {
> -	unsigned int			sync_proto;
> +	char				*proto_name;

	cost char			*proto_name;

>  	int				loop_interval;
>  	pthread_mutex_t			lock;
>  	pthread_cond_t			cond;
> @@ -438,9 +432,9 @@ struct tracecmd_time_sync {
>  };
>  
>  void tracecmd_tsync_init(void);
> -int tracecmd_tsync_proto_getall(char **proto_mask, int *words);
> -unsigned int tracecmd_tsync_proto_select(char *proto_mask, int words);
> -bool tsync_proto_is_supported(unsigned int proto_id);
> +int tracecmd_tsync_proto_getall(char ***protos);
> +char *tracecmd_tsync_proto_select(char **protos);

const char *

> +bool tsync_proto_is_supported(char *proto_name);
>  void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
>  void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
>  int tracecmd_tsync_get_offsets(struct tracecmd_time_sync *tsync,
> diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
> index 1de9d5e5..1f3bc443 100644
> --- a/lib/trace-cmd/include/trace-tsync-local.h
> +++ b/lib/trace-cmd/include/trace-tsync-local.h
> @@ -26,12 +26,12 @@ struct clock_sync_context {
>  	unsigned int			remote_port;
>  };
>  
> -static int get_trace_req_protos(char *buf, int length,
> -				char **tsync_protos,
> -				unsigned int *tsync_protos_size)
> +static int get_trace_req_protos(char *buf, int length, char ***tsync_protos)
>  {
> -	*tsync_protos = malloc(length);
> -	if (!*tsync_protos)
> +	int count = 0;
> +	char *p;
> +	int i, j;
> +
> +	i = length;
> +	p = buf;
> +	while (i > 0) {
> +		i -= strlen(p) + 1;
> +		count++;
> +		p -= strlen(p) + 1;

Do you really want p to go backwards here?

> +	}
> +
> +	*tsync_protos = calloc(count + 1, sizeof(char *));
> +	if (!(*tsync_protos))
>  		return -1;
> -	memcpy(*tsync_protos, buf, length);
> -	*tsync_protos_size = length;
> +
> +	i = length;
> +	p = buf;
> +	j = 0;
> +	while (i > 0 && j < (count - 1)) {
> +		i -= strlen(p) + 1;
> +		(*tsync_protos)[j++] = strdup(p);
> +		p -= strlen(p) + 1;

And here too?

-- Steve

> +	}
>  
>  	return 0;
>  }
> @@ -1026,8 +1054,7 @@ out:
>  int tracecmd_msg_recv_trace_req(struct tracecmd_msg_handle *msg_handle,
>  				int *argc, char ***argv, bool *use_fifos,
>  				unsigned long long *trace_id,
> -				char **tsync_protos,
> -				unsigned int *tsync_protos_size)
> +				char ***tsync_protos)
>  {
>  	struct tracecmd_msg msg;
>  	unsigned int param_id;

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string
  2020-10-29 11:18 ` [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string Tzvetomir Stoyanov (VMware)
  2020-11-05 14:52   ` Steven Rostedt
@ 2020-12-02 22:43   ` Steven Rostedt
  1 sibling, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2020-12-02 22:43 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:01 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> +static int make_trace_req_protos(char **buf, int *size, char **tsync_protos)
>  {
> +	int protos_size = 1;
>  	size_t buf_size;
> +	char **protos;
>  	char *nbuf;
>  	char *p;
>  
> +	protos = tsync_protos;
> +	while (*protos) {
> +		protos_size += strlen(*protos) + 1;
> +		protos++;
> +	}
> +
>  	buf_size = TRACE_REQ_PARAM_SIZE + protos_size;
>  	nbuf = realloc(*buf, *size + buf_size);
>  	if (!nbuf)
> @@ -867,7 +875,13 @@ static int make_trace_req_protos(char **buf, int *size,
>  	*(unsigned int *)p = htonl(protos_size);
>  	p += sizeof(int);
>  
> -	memcpy(p, tsync_protos, protos_size);
> +	protos = tsync_protos;
> +	while (*protos) {
> +		memcpy(p, *protos, strlen(*protos) + 1);

The above can be replaced with:

		strcpy(p, *protos);

-- Steve

> +		p += strlen(*protos) + 1;
> +		protos++;
> +	}
> +	p = NULL;
>  
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins
  2020-10-29 11:18 ` [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins Tzvetomir Stoyanov (VMware)
@ 2020-12-02 22:48   ` Steven Rostedt
  2020-12-03 13:09     ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2020-12-02 22:48 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:05 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Some timestamp synchronization plugins may not support all ftrace
> clocks. Added logic to timestamp synchronization plugins to declare what
> ftace clocks they support. Added logic to select plugin depending on the

 "ftrace clocks"

> ftrace clock used in the current trace session and supported clocks.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h             |  4 ++--
>  lib/trace-cmd/include/trace-tsync-local.h |  1 +
>  lib/trace-cmd/trace-timesync.c            | 29 +++++++++++++++++++----
>  tracecmd/trace-record.c                   | 10 +++++++-
>  tracecmd/trace-tsync.c                    |  2 +-
>  5 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index b48381cd..21028940 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -448,8 +448,8 @@ struct tracecmd_time_sync {
>  };
>  
>  void tracecmd_tsync_init(void);
> -int tracecmd_tsync_proto_getall(char ***protos);
> -char *tracecmd_tsync_proto_select(char **protos);
> +int tracecmd_tsync_proto_getall(char ***protos, const char *clock);
> +char *tracecmd_tsync_proto_select(char **protos, char *clock);
>  bool tsync_proto_is_supported(char *proto_name);
>  void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
>  void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
> diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
> index 1f3bc443..25d5b4e8 100644
> --- a/lib/trace-cmd/include/trace-tsync-local.h
> +++ b/lib/trace-cmd/include/trace-tsync-local.h
> @@ -27,6 +27,7 @@ struct clock_sync_context {
>  };
>  
>  int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
> +				  int supported_clocks,

Is this a bitmask? How is this used?

-- Steve

>  				  int (*init)(struct tracecmd_time_sync *),
>  				  int (*free)(struct tracecmd_time_sync *),
>  				  int (*calc)(struct tracecmd_time_sync *,
> diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 06/16] trace-cmd: Add role parameter to timestamp synchronization plugins
  2020-10-29 11:18 ` [PATCH v25 06/16] trace-cmd: Add role " Tzvetomir Stoyanov (VMware)
@ 2020-12-02 23:04   ` Steven Rostedt
  0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2020-12-02 23:04 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:06 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Defined HOST and GUEST roles in timestamp synchronization context.
> Some timestamp synchronization plugins may not support running in both
> host and guest roles. This could happen in nested virtualization use
> case, where the same plugin can be used as a host and as a guest.
> Added logic to timestamp synchronization plugins to declare what roles
> they support. Added logic to select plugin depending on supported roles
> and currently requested role.
> 
> clocks. Added logic to timestamp synchronization plugins to declare what
> ftace clocks they support. Added logic to select plugin depending on the
> ftrace clock used in the current trace session and supported clocks.

 Extra paragraph?

> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h             | 10 ++++++++--
>  lib/trace-cmd/include/trace-tsync-local.h |  2 +-
>  lib/trace-cmd/trace-timesync.c            | 17 ++++++++++++++---
>  tracecmd/trace-record.c                   |  3 ++-
>  tracecmd/trace-tsync.c                    |  3 ++-
>  5 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 21028940..1ca2da10 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -437,6 +437,11 @@ enum{
>  	TRACECMD_TIME_SYNC_CMD_STOP	= 2,
>  };
>  
> +enum tracecmd_time_sync_role {
> +	TRACECMD_TIME_SYNC_ROLE_HOST	= 0x01,
> +	TRACECMD_TIME_SYNC_ROLE_GUEST	= 0x02

If this is going to be used as a bitmask, it's best to demonstrate that
with:

	(1 << 0),
	(1 << 1)

-- Steve


> +};
> +
>  struct tracecmd_time_sync {
>  	char				*proto_name;
>  	int				loop_interval;

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 07/16] trace-cmd: Add host / guest role in timestamp synchronization context
  2020-10-29 11:18 ` [PATCH v25 07/16] trace-cmd: Add host / guest role in timestamp synchronization context Tzvetomir Stoyanov (VMware)
@ 2020-12-02 23:07   ` Steven Rostedt
  0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2020-12-02 23:07 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:07 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> @@ -298,8 +298,11 @@ static int clock_context_init(struct tracecmd_time_sync *tsync, bool server)
>  	clock = calloc(1, sizeof(struct clock_sync_context));
>  	if (!clock)
>  		return -1;
> -
> -	clock->is_server = server;
> +	clock->is_guest = guest;
> +	if (clock->is_guest)
> +		clock->is_server = true;
> +	else
> +		clock->is_server = false;

Wait? You want them to equal?

Then you should have:

	clock->is_server = guest;

If you want them opposite:

	clock->is_server = !guest;

-- Steve

>  	if (get_vsocket_params(tsync->msg_handle->fd, &clock->local_cid,
>  			       &clock->local_port, &clock->remote_cid,
>  			       &clock->remote_port))

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction
  2020-10-29 11:18 ` [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction Tzvetomir Stoyanov (VMware)
@ 2020-12-03  1:47   ` Steven Rostedt
  2020-12-03 12:56     ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2020-12-03  1:47 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:09 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> @@ -2181,7 +2184,9 @@ static void tsync_offset_load(struct tracecmd_input *handle, char *buf)
>  		host->ts_samples[i].time = tep_read_number(handle->pevent,
>  							   buf8 + i, 8);
>  		host->ts_samples[i].offset = tep_read_number(handle->pevent,
> -						buf8 + host->ts_samples_count+i, 8);
> +						buf8 + host->ts_samples_count + i, 8);
> +		host->ts_samples[i].scaling = tep_read_number(handle->pevent,
> +						buf8 + (2 * host->ts_samples_count) + i, 8);
>  	}


I was thinking the above code may read better if we changed it to:

	long long *time_buf;
	long long *offset_buf;
	long long *scaling_buf;

	time_buf = (long long *)buf;
	offset_buf = time_buf + host->ts_samples_count;
	scaling_buf = offset_buf + host->ts_samples_count;

	for (i = 0; i < host->ts_samples_count; i++) {
		host->ts_samples[i].time = tep_read_number(handle->pevent,
							   time_buf + i, 8);
		host->ts_samples[i].offset = tep_read_number(handle->	pevent,
							   offset_buf + i, 8);
		host->ts_samples[i].scaling = tep_read_number(handle->pevent,
						           scaling_buf + i, 8);
	}

-- Steve



>  	qsort(host->ts_samples, host->ts_samples_count,
>  	      sizeof(struct ts_offset_sample), tsync_offset_cmp);
> @@ -2534,6 +2539,7 @@ static int handle_options(struct tracecmd_input *handle)
>  			 * long long array of size [count] of times,
>  			 *      when the offsets were calculated.
>  			 * long long array of size [count] of timestamp offsets.
> +			 * long long array of size [count] of timestamp scaling ratios.*
>  			 */
>  			if (size < 12 || handle->flags & TRACECMD_FL_IGNORE_DATE)
>  				break;

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 10/16] trace-cmd: Add time sync protocol flags
  2020-10-29 11:18 ` [PATCH v25 10/16] trace-cmd: Add time sync protocol flags Tzvetomir Stoyanov (VMware)
@ 2020-12-03  2:09   ` Steven Rostedt
  2020-12-03 12:59     ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2020-12-03  2:09 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 29 Oct 2020 13:18:10 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Added 32bit flags for the time synchronization protocols. The first added
> flag is TRACECMD_TSYNC_FLAG_INTERPOLATE, used to specify how the
> timestamps must be corrected.
>  - If the flag is set, an interpolation is performed:
>    Find the (min, max) interval from the offsets array and calculate
> offset specific to the given timestamp using interpolation in that
> interval.
>  - If the flag is not set, do not interpolate:
>    Find the (min, max) interval from the offsets array and use the
>    min offset for all timespamps within the interval.

    "timestamps"

> 
> These flags are set by the timestamp synchronization protocols at the
> protocol initialization time.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
> --- a/tracecmd/trace-tsync.c
> +++ b/tracecmd/trace-tsync.c
> @@ -132,7 +132,8 @@ out:
>  static void write_guest_time_shift(struct buffer_instance *instance)
>  {
>  	struct tracecmd_output *handle;
> -	struct iovec vector[5];
> +	struct iovec vector[6];
> +	unsigned int flags;
>  	long long *scalings = NULL;
>  	long long *offsets = NULL;
>  	long long *ts = NULL;
> @@ -145,6 +146,9 @@ static void write_guest_time_shift(struct buffer_instance *instance)
>  					 &ts, &offsets, &scalings);
>  	if (ret < 0 || !count || !ts || !offsets || !scalings)
>  		return;
> +	ret = tracecmd_tsync_get_proto_flags(&instance->tsync, &flags);
> +	if (ret < 0)
> +		return;
>  
>  	file = instance->output_file;
>  	fd = open(file, O_RDWR);
> @@ -154,14 +158,16 @@ static void write_guest_time_shift(struct buffer_instance *instance)
>  	vector[0].iov_len = 8;
>  	vector[0].iov_base = &top_instance.trace_id;
>  	vector[1].iov_len = 4;
> -	vector[1].iov_base = &count;
> -	vector[2].iov_len = 8 * count;
> -	vector[2].iov_base = ts;
> +	vector[1].iov_base = &flags;
> +	vector[2].iov_len = 4;
> +	vector[2].iov_base = &count;
>  	vector[3].iov_len = 8 * count;
> -	vector[3].iov_base = offsets;
> +	vector[3].iov_base = ts;
>  	vector[4].iov_len = 8 * count;
> -	vector[4].iov_base = scalings;
> -	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 5);
> +	vector[4].iov_base = offsets;
> +	vector[5].iov_len = 8 * count;
> +	vector[5].iov_base = scalings;
> +	tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 6);
>  	tracecmd_append_options(handle);
>  	tracecmd_output_close(handle);
>  #ifdef TSYNC_DEBUG

To make the above cleaner, I would use an enum to define the vector
indexes:

enum {
	VECTOR_TRACE_ID,
	VECTOR_FLAGS,
	VECTOR_COUNT,
	VECTOR_TIMES,
	VECTOR_OFFSETS,
	VECTOR_SCALINGS,
}

And then you can make it:

	vector[VECTOR_TRACE_ID].iov_len = 8;
	vector[VECTOR_TRACE_ID].iov_base = &top_instance.trace_id;
	vector[VECTOR_FLAGS].iov_len = 4;
	vector[VECTOR_FLAGS].iov_base = &flags;
	vector[VECTOR_COUNT].iov_len = 4;
	vector[VECTOR_COUNT].iov_base = &count;
	vector[VECTOR_TIMES].iov_len = 8 * count;
	vector[VECTOR_TIMES].iov_base = ts;
	vector[VECTOR_OFFSETS].iov_len = 8 * count;
	vector[VECTOR_OFFSETS].iov_base = offsets;
	vector[VECTOR_SCALINGS].iov_len = 8 * count;
	vector[VECTOR_SCALINGS].iov_base = scalings;

It makes it obvious what each vector is used for.

This is just an opinion. You don't need to implement it. I just hate
hard coded numbers ;-)

-- Steve

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction
  2020-12-03  1:47   ` Steven Rostedt
@ 2020-12-03 12:56     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2020-12-03 12:56 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel

On Thu, Dec 3, 2020 at 3:47 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 29 Oct 2020 13:18:09 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> > @@ -2181,7 +2184,9 @@ static void tsync_offset_load(struct tracecmd_input *handle, char *buf)
> >               host->ts_samples[i].time = tep_read_number(handle->pevent,
> >                                                          buf8 + i, 8);
> >               host->ts_samples[i].offset = tep_read_number(handle->pevent,
> > -                                             buf8 + host->ts_samples_count+i, 8);
> > +                                             buf8 + host->ts_samples_count + i, 8);
> > +             host->ts_samples[i].scaling = tep_read_number(handle->pevent,
> > +                                             buf8 + (2 * host->ts_samples_count) + i, 8);
> >       }
>
>
> I was thinking the above code may read better if we changed it to:
>
>         long long *time_buf;
>         long long *offset_buf;
>         long long *scaling_buf;
>
>         time_buf = (long long *)buf;
>         offset_buf = time_buf + host->ts_samples_count;
>         scaling_buf = offset_buf + host->ts_samples_count;
>
>         for (i = 0; i < host->ts_samples_count; i++) {
>                 host->ts_samples[i].time = tep_read_number(handle->pevent,
>                                                            time_buf + i, 8);
>                 host->ts_samples[i].offset = tep_read_number(handle->   pevent,
>                                                            offset_buf + i, 8);
>                 host->ts_samples[i].scaling = tep_read_number(handle->pevent,
>                                                            scaling_buf + i, 8);
>         }
>
This code is rewritten in "trace-cmd: Add timestamp synchronization
per vCPU" patch
in a better and safer way.

> -- Steve
>
>
>
> >       qsort(host->ts_samples, host->ts_samples_count,
> >             sizeof(struct ts_offset_sample), tsync_offset_cmp);
> > @@ -2534,6 +2539,7 @@ static int handle_options(struct tracecmd_input *handle)
> >                        * long long array of size [count] of times,
> >                        *      when the offsets were calculated.
> >                        * long long array of size [count] of timestamp offsets.
> > +                      * long long array of size [count] of timestamp scaling ratios.*
> >                        */
> >                       if (size < 12 || handle->flags & TRACECMD_FL_IGNORE_DATE)
> >                               break;



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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 10/16] trace-cmd: Add time sync protocol flags
  2020-12-03  2:09   ` Steven Rostedt
@ 2020-12-03 12:59     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2020-12-03 12:59 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel

On Thu, Dec 3, 2020 at 4:09 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 29 Oct 2020 13:18:10 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Added 32bit flags for the time synchronization protocols. The first added
> > flag is TRACECMD_TSYNC_FLAG_INTERPOLATE, used to specify how the
> > timestamps must be corrected.
> >  - If the flag is set, an interpolation is performed:
> >    Find the (min, max) interval from the offsets array and calculate
> > offset specific to the given timestamp using interpolation in that
> > interval.
> >  - If the flag is not set, do not interpolate:
> >    Find the (min, max) interval from the offsets array and use the
> >    min offset for all timespamps within the interval.
>
>     "timestamps"
>
> >
> > These flags are set by the timestamp synchronization protocols at the
> > protocol initialization time.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> > --- a/tracecmd/trace-tsync.c
> > +++ b/tracecmd/trace-tsync.c
> > @@ -132,7 +132,8 @@ out:
> >  static void write_guest_time_shift(struct buffer_instance *instance)
> >  {
> >       struct tracecmd_output *handle;
> > -     struct iovec vector[5];
> > +     struct iovec vector[6];
> > +     unsigned int flags;
> >       long long *scalings = NULL;
> >       long long *offsets = NULL;
> >       long long *ts = NULL;
> > @@ -145,6 +146,9 @@ static void write_guest_time_shift(struct buffer_instance *instance)
> >                                        &ts, &offsets, &scalings);
> >       if (ret < 0 || !count || !ts || !offsets || !scalings)
> >               return;
> > +     ret = tracecmd_tsync_get_proto_flags(&instance->tsync, &flags);
> > +     if (ret < 0)
> > +             return;
> >
> >       file = instance->output_file;
> >       fd = open(file, O_RDWR);
> > @@ -154,14 +158,16 @@ static void write_guest_time_shift(struct buffer_instance *instance)
> >       vector[0].iov_len = 8;
> >       vector[0].iov_base = &top_instance.trace_id;
> >       vector[1].iov_len = 4;
> > -     vector[1].iov_base = &count;
> > -     vector[2].iov_len = 8 * count;
> > -     vector[2].iov_base = ts;
> > +     vector[1].iov_base = &flags;
> > +     vector[2].iov_len = 4;
> > +     vector[2].iov_base = &count;
> >       vector[3].iov_len = 8 * count;
> > -     vector[3].iov_base = offsets;
> > +     vector[3].iov_base = ts;
> >       vector[4].iov_len = 8 * count;
> > -     vector[4].iov_base = scalings;
> > -     tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 5);
> > +     vector[4].iov_base = offsets;
> > +     vector[5].iov_len = 8 * count;
> > +     vector[5].iov_base = scalings;
> > +     tracecmd_add_option_v(handle, TRACECMD_OPTION_TIME_SHIFT, vector, 6);
> >       tracecmd_append_options(handle);
> >       tracecmd_output_close(handle);
> >  #ifdef TSYNC_DEBUG
>
> To make the above cleaner, I would use an enum to define the vector
> indexes:
>
> enum {
>         VECTOR_TRACE_ID,
>         VECTOR_FLAGS,
>         VECTOR_COUNT,
>         VECTOR_TIMES,
>         VECTOR_OFFSETS,
>         VECTOR_SCALINGS,
> }
>
> And then you can make it:
>
>         vector[VECTOR_TRACE_ID].iov_len = 8;
>         vector[VECTOR_TRACE_ID].iov_base = &top_instance.trace_id;
>         vector[VECTOR_FLAGS].iov_len = 4;
>         vector[VECTOR_FLAGS].iov_base = &flags;
>         vector[VECTOR_COUNT].iov_len = 4;
>         vector[VECTOR_COUNT].iov_base = &count;
>         vector[VECTOR_TIMES].iov_len = 8 * count;
>         vector[VECTOR_TIMES].iov_base = ts;
>         vector[VECTOR_OFFSETS].iov_len = 8 * count;
>         vector[VECTOR_OFFSETS].iov_base = offsets;
>         vector[VECTOR_SCALINGS].iov_len = 8 * count;
>         vector[VECTOR_SCALINGS].iov_base = scalings;
>
> It makes it obvious what each vector is used for.
>
> This is just an opinion. You don't need to implement it. I just hate
> hard coded numbers ;-)

The code is changed in "trace-cmd: Add timestamp synchronization per
vCPU" patch,
mo more hard coded numbers. The vector size is dynamic, depending on
the VCPU count.

>
> -- Steve



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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins
  2020-12-02 22:48   ` Steven Rostedt
@ 2020-12-03 13:09     ` Tzvetomir Stoyanov
  0 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2020-12-03 13:09 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Trace Devel

On Thu, Dec 3, 2020 at 12:48 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 29 Oct 2020 13:18:05 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Some timestamp synchronization plugins may not support all ftrace
> > clocks. Added logic to timestamp synchronization plugins to declare what
> > ftace clocks they support. Added logic to select plugin depending on the
>
>  "ftrace clocks"
>
> > ftrace clock used in the current trace session and supported clocks.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  include/trace-cmd/trace-cmd.h             |  4 ++--
> >  lib/trace-cmd/include/trace-tsync-local.h |  1 +
> >  lib/trace-cmd/trace-timesync.c            | 29 +++++++++++++++++++----
> >  tracecmd/trace-record.c                   | 10 +++++++-
> >  tracecmd/trace-tsync.c                    |  2 +-
> >  5 files changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> > index b48381cd..21028940 100644
> > --- a/include/trace-cmd/trace-cmd.h
> > +++ b/include/trace-cmd/trace-cmd.h
> > @@ -448,8 +448,8 @@ struct tracecmd_time_sync {
> >  };
> >
> >  void tracecmd_tsync_init(void);
> > -int tracecmd_tsync_proto_getall(char ***protos);
> > -char *tracecmd_tsync_proto_select(char **protos);
> > +int tracecmd_tsync_proto_getall(char ***protos, const char *clock);
> > +char *tracecmd_tsync_proto_select(char **protos, char *clock);
> >  bool tsync_proto_is_supported(char *proto_name);
> >  void tracecmd_tsync_with_host(struct tracecmd_time_sync *tsync);
> >  void tracecmd_tsync_with_guest(struct tracecmd_time_sync *tsync);
> > diff --git a/lib/trace-cmd/include/trace-tsync-local.h b/lib/trace-cmd/include/trace-tsync-local.h
> > index 1f3bc443..25d5b4e8 100644
> > --- a/lib/trace-cmd/include/trace-tsync-local.h
> > +++ b/lib/trace-cmd/include/trace-tsync-local.h
> > @@ -27,6 +27,7 @@ struct clock_sync_context {
> >  };
> >
> >  int tracecmd_tsync_proto_register(char *proto_name, int accuracy,
> > +                               int supported_clocks,
>
> Is this a bitmask? How is this used?
Yes, it is a bitmask of "enum tracecmd_clocks" values, or just
TRACECMD_CLOCK_UNKNOWN for all clocks supported.

>
> -- Steve
>
> >                                 int (*init)(struct tracecmd_time_sync *),
> >                                 int (*free)(struct tracecmd_time_sync *),
> >                                 int (*calc)(struct tracecmd_time_sync *,
> > diff --git a/lib/trace-cmd/trace-timesync.c b/lib/trace-cmd/trace-timesync.c



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

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-12-03 13:10 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29 11:18 [PATCH v25 00/16] Timestamp synchronization of host - guest tracing session Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 01/16] trace-cmd: Replace time sync protocol ID with string Tzvetomir Stoyanov (VMware)
2020-11-05 14:52   ` Steven Rostedt
2020-12-02 22:43   ` Steven Rostedt
2020-10-29 11:18 ` [PATCH v25 02/16] trace-cmd: Add trace-cmd library APIs for ftrace clock name Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 03/16] trace-cmd: Move VM related logic in a separate file Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 04/16] trace-cmd: Add new libtrasefs API to get the current trace clock Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 05/16] trace-cmd: Add clock parameter to timestamp synchronization plugins Tzvetomir Stoyanov (VMware)
2020-12-02 22:48   ` Steven Rostedt
2020-12-03 13:09     ` Tzvetomir Stoyanov
2020-10-29 11:18 ` [PATCH v25 06/16] trace-cmd: Add role " Tzvetomir Stoyanov (VMware)
2020-12-02 23:04   ` Steven Rostedt
2020-10-29 11:18 ` [PATCH v25 07/16] trace-cmd: Add host / guest role in timestamp synchronization context Tzvetomir Stoyanov (VMware)
2020-12-02 23:07   ` Steven Rostedt
2020-10-29 11:18 ` [PATCH v25 08/16] trace-cmd: Add guest CPU count PID in tracecmd_time_sync struct Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 09/16] trace-cmd: Add scaling ratio for timestamp correction Tzvetomir Stoyanov (VMware)
2020-12-03  1:47   ` Steven Rostedt
2020-12-03 12:56     ` Tzvetomir Stoyanov
2020-10-29 11:18 ` [PATCH v25 10/16] trace-cmd: Add time sync protocol flags Tzvetomir Stoyanov (VMware)
2020-12-03  2:09   ` Steven Rostedt
2020-12-03 12:59     ` Tzvetomir Stoyanov
2020-10-29 11:18 ` [PATCH v25 11/16] trace-cmd: Add timestamp synchronization per vCPU Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 12/16] trace-cmd: Define a macro for packed structures Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 13/16] trace-cmd: Add dummy function to initialize timestamp sync logic Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 14/16] trace-cmd: [POC] PTP-like algorithm for host - guest timestamp synchronization Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 15/16] trace-cmd: Debug scripts for " Tzvetomir Stoyanov (VMware)
2020-10-29 11:18 ` [PATCH v25 16/16] trace-cmd [POC]: Add KVM timestamp synchronization plugin Tzvetomir Stoyanov (VMware)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.