linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Various enhancements, related to reading trace.dat file
@ 2020-04-16 16:00 Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 1/5] kernel-shark-2.alpha: Adjust the width of marker buttons Tzvetomir Stoyanov (VMware)
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-16 16:00 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

From: "Tzvetomir (VMware)  Stoyanov" <tz.stoyanov@gmail.com>

Few new tracemcd APIs were introduced recently, realted to opening
trace.dat file on stages, pairing files from the same tracing session,
getting extended guest VM information from the host trace file.
These patches allows KernelShark to leverage these tracecmd enhansments.

Tzvetomir (VMware)  Stoyanov (4):
  kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file
  kernel-shark-2.alpha: Print the plugin's file name in case of loading
    error
  kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C
  kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping
    information from the trace files

Tzvetomir (VMware) Stoyanov (1):
  kernel-shark-2.alpha: Adjust the width of marker buttons

 src/KsDualMarker.cpp     |   4 +-
 src/libkshark-plugin.c   |   3 +-
 src/libkshark-tepdata.c  | 175 ++++++++++++++++++++++++++++++++++++++-
 src/libkshark-tepdata.h  |  10 +++
 src/plugins/KVMCombo.cpp | 160 +++++++++++++++--------------------
 src/plugins/KVMCombo.hpp |  16 ++--
 6 files changed, 260 insertions(+), 108 deletions(-)

-- 
2.25.1


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

* [PATCH v3 1/5] kernel-shark-2.alpha: Adjust the width of marker buttons
  2020-04-16 16:00 [PATCH v3 0/5] Various enhancements, related to reading trace.dat file Tzvetomir Stoyanov (VMware)
@ 2020-04-16 16:00 ` Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 2/5] kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file Tzvetomir Stoyanov (VMware)
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-16 16:00 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

From: Tzvetomir (VMware) Stoyanov <tz.stoyanov@gmail.com>

Both "Marker A" and "Marker B" buttons were too tight to fit the button's label on them.
Add some extra space.

Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
---
 src/KsDualMarker.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/KsDualMarker.cpp b/src/KsDualMarker.cpp
index 3e0b462..e2b8ac1 100644
--- a/src/KsDualMarker.cpp
+++ b/src/KsDualMarker.cpp
@@ -145,8 +145,8 @@ KsDualMarkerSM::KsDualMarkerSM(QWidget *parent)
 {
 	QString styleSheetA, styleSheetB;
 
-	_buttonA.setFixedWidth(STRING_WIDTH(" Marker A "));
-	_buttonB.setFixedWidth(STRING_WIDTH(" Marker B "));
+	_buttonA.setFixedWidth(STRING_WIDTH(" Marker A ") + FONT_WIDTH);
+	_buttonB.setFixedWidth(STRING_WIDTH(" Marker B ") + FONT_WIDTH);
 
 	for (auto const &l: {&_labelMA, &_labelMB, &_labelDelta}) {
 		l->setFrameStyle(QFrame::Panel | QFrame::Sunken);
-- 
2.25.1


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

* [PATCH v3 2/5] kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file
  2020-04-16 16:00 [PATCH v3 0/5] Various enhancements, related to reading trace.dat file Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 1/5] kernel-shark-2.alpha: Adjust the width of marker buttons Tzvetomir Stoyanov (VMware)
@ 2020-04-16 16:00 ` Tzvetomir Stoyanov (VMware)
  2020-04-22 13:39   ` Yordan Karadzhov (VMware)
  2020-04-16 16:00 ` [PATCH v3 3/5] kernel-shark-2.alpha: Print the plugin's file name in case of loading error Tzvetomir Stoyanov (VMware)
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-16 16:00 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>

The new tracecmd API tracecmd_open_head() allows opening a trace.dat
file on stages. First, only the headers from the file are read and parsed,
without reading the tracing data.
The tracecmd_pair_peer() API is used to bind a tracing peer to
this file, before parsing the trace data. This affects events timestamps
correction when the tracing data is loaded.
The tracecmd_get_guest_cpumap() API is used to find dependencies between
all files and find a possible tracing peers.

This change depends on these trace-cmd patch sets:
  "Useful APIs for merging tracing files"
https://patchwork.kernel.org/project/linux-trace-devel/list/?series=268745
  "Split reading the trace.dat options from trace data"
https://patchwork.kernel.org/project/linux-trace-devel/list/?series=268743

Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
---
 src/libkshark-tepdata.c | 59 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index 8678e12..f280e57 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -985,6 +985,45 @@ const char *tep_plugin_names[] = {"sched_events",
 
 #define LINUX_IDLE_TASK_PID	0
 
+/** Find a host stream from the same tracing session, that has guest information */
+struct tracecmd_input *kshark_tep_find_merge_peer(struct kshark_context *kshark_ctx,
+						  struct tracecmd_input *handle)
+{
+	struct tracecmd_input *peer_handle = NULL;
+	struct kshark_data_stream *peer_stream;
+	unsigned long long trace_id;
+	int *streamIds = NULL;
+	int ret;
+	int i;
+
+	trace_id = tracecmd_get_traceid(handle);
+	if (!trace_id)
+		goto out;
+
+	streamIds = kshark_all_streams(kshark_ctx);
+	if (!streamIds)
+		goto out;
+	for (i = 0; i < kshark_ctx->n_streams; i++) {
+		peer_stream = kshark_get_data_stream(kshark_ctx, streamIds[i]);
+		if (!peer_stream || peer_stream->format != KS_TEP_DATA)
+			continue;
+		peer_handle = kshark_get_tep_input(peer_stream);
+		if (!peer_handle)
+			continue;
+		ret = tracecmd_get_guest_cpumap(peer_handle, trace_id,
+						NULL, NULL, NULL);
+		if (!ret)
+			break;
+	}
+
+	if (i == kshark_ctx->n_streams)
+		peer_handle = NULL;
+
+out:
+	free(streamIds);
+	return peer_handle;
+}
+
 /** Initialize the FTRACE data input (from file). */
 int kshark_tep_init_input(struct kshark_data_stream *stream,
 			  const char *file)
@@ -992,8 +1031,10 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
 	struct kshark_context *kshark_ctx = NULL;
 	struct tepdata_handle *tep_handle;
 	struct kshark_plugin_list *plugin;
+	struct tracecmd_input *merge_peer;
 	struct tep_event *event;
 	int i, n_tep_plugins;
+	int ret;
 
 	if (!kshark_instance(&kshark_ctx) || !init_thread_seq())
 		return -EEXIST;
@@ -1009,13 +1050,29 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
 	if (!tep_handle)
 		return -EFAULT;
 
-	tep_handle->input = tracecmd_open(file);
+	/** Open the tracing file, parse headers and create trace input context */
+	tep_handle->input = tracecmd_open_head(file);
 	if (!tep_handle->input) {
 		free(tep_handle);
 		stream->interface.handle = NULL;
 		return -EEXIST;
 	}
 
+	/** Find a merge peer from the same tracing session */
+	merge_peer = kshark_tep_find_merge_peer(kshark_ctx, tep_handle->input);
+	if (merge_peer)
+		tracecmd_pair_peer(tep_handle->input, merge_peer);
+
+	/** Read the racing data from the file */
+	ret = tracecmd_init_data(tep_handle->input);
+
+	if (ret < 0) {
+		tracecmd_close(tep_handle->input);
+		free(tep_handle);
+		stream->interface.handle = NULL;
+		return -EEXIST;
+	}
+
 	tep_handle->tep = tracecmd_get_pevent(tep_handle->input);
 
 	tep_handle->sched_switch_event_id = -EINVAL;
-- 
2.25.1


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

* [PATCH v3 3/5] kernel-shark-2.alpha: Print the plugin's file name in case of loading error
  2020-04-16 16:00 [PATCH v3 0/5] Various enhancements, related to reading trace.dat file Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 1/5] kernel-shark-2.alpha: Adjust the width of marker buttons Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 2/5] kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file Tzvetomir Stoyanov (VMware)
@ 2020-04-16 16:00 ` Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 4/5] kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C Tzvetomir Stoyanov (VMware)
  2020-04-16 16:00 ` [PATCH v3 5/5] kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping information from the trace files Tzvetomir Stoyanov (VMware)
  4 siblings, 0 replies; 9+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-16 16:00 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>

When dlopen() fails to load the plugin, "plugin->file" is still not initialized.
It is useful to print the file name of the problematic plugin, "file", instead of not
yet initialized "plugin->file".

Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
---
 src/libkshark-plugin.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/libkshark-plugin.c b/src/libkshark-plugin.c
index f9a721f..1cea87b 100644
--- a/src/libkshark-plugin.c
+++ b/src/libkshark-plugin.c
@@ -331,8 +331,7 @@ kshark_register_plugin(struct kshark_context *kshark_ctx,
 	return plugin;
 
  fail:
-	fprintf(stderr, "cannot load plugin '%s'\n",
-		plugin->file);
+	fprintf(stderr, "cannot load plugin '%s'\n", file);
 
 	if (plugin->handle)
 		dlclose(plugin->handle);
-- 
2.25.1


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

* [PATCH v3 4/5] kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C
  2020-04-16 16:00 [PATCH v3 0/5] Various enhancements, related to reading trace.dat file Tzvetomir Stoyanov (VMware)
                   ` (2 preceding siblings ...)
  2020-04-16 16:00 ` [PATCH v3 3/5] kernel-shark-2.alpha: Print the plugin's file name in case of loading error Tzvetomir Stoyanov (VMware)
@ 2020-04-16 16:00 ` Tzvetomir Stoyanov (VMware)
  2020-04-22 13:40   ` Yordan Karadzhov (VMware)
  2020-04-16 16:00 ` [PATCH v3 5/5] kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping information from the trace files Tzvetomir Stoyanov (VMware)
  4 siblings, 1 reply; 9+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-16 16:00 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>

C++ compiler uses name mangling to handle function overloading. As there is no function overloading in C, function names are not mangled. This breaks the linking of C library into C++ binary.
Declare functions from trace-cmd.h as pure C, so the loader will not mangle the names when resolving them.

Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
---
 src/plugins/KVMCombo.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/plugins/KVMCombo.cpp b/src/plugins/KVMCombo.cpp
index 3ff9ca5..1ae03aa 100644
--- a/src/plugins/KVMCombo.cpp
+++ b/src/plugins/KVMCombo.cpp
@@ -13,7 +13,9 @@
 #include<iostream>
 
 // trace-cmd
+extern "C" {
 #include "trace-cmd/trace-cmd.h"
+}
 
 // KernelShark
 #include "libkshark.h"
-- 
2.25.1


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

* [PATCH v3 5/5] kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping information from the trace files
  2020-04-16 16:00 [PATCH v3 0/5] Various enhancements, related to reading trace.dat file Tzvetomir Stoyanov (VMware)
                   ` (3 preceding siblings ...)
  2020-04-16 16:00 ` [PATCH v3 4/5] kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C Tzvetomir Stoyanov (VMware)
@ 2020-04-16 16:00 ` Tzvetomir Stoyanov (VMware)
  2020-04-22 14:42   ` Yordan Karadzhov (VMware)
  4 siblings, 1 reply; 9+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-04-16 16:00 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>

In the upcoming trace-cmd 2.9, extra information about host and guest tracing
will be stored in the trace.dat files. Information about host CPU - guest vCPU
mapping will be saved in host trace file. This mapping is mandatory for KVMCombo
plugin. Currently, the mapping is extracted for the tracing data.
Getting the information from the trace files is more reliable and solves the use
case with more than one guest.

Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
---
 src/libkshark-tepdata.c  | 116 ++++++++++++++++++++++++++++
 src/libkshark-tepdata.h  |  10 +++
 src/plugins/KVMCombo.cpp | 158 ++++++++++++++++-----------------------
 src/plugins/KVMCombo.hpp |  16 ++--
 4 files changed, 197 insertions(+), 103 deletions(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index f280e57..048981e 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -22,6 +22,7 @@
 #include "tracefs/tracefs.h"
 
 // KernelShark
+#include "libkshark.h"
 #include "libkshark-plugin.h"
 #include "libkshark-tepdata.h"
 
@@ -1293,3 +1294,118 @@ char **kshark_tracecmd_local_plugins()
 {
 	return tracefs_tracers(tracefs_get_tracing_dir());
 }
+
+/**
+ * @brief Free an array, allocated by kshark_tracecmd_get_hostguest_mapping() API
+ *
+ *
+ * @param map: Array, allocated by kshark_tracecmd_get_hostguest_mapping() API
+ * @param count: Number of entries in the array
+ *
+ */
+void kshark_tracecmd_free_hostguest_map(struct kshark_host_guest_map *map, int count)
+{
+	int i;
+
+	if (!map)
+		return;
+	for (i = 0; i < count; i++) {
+		free(map[i].guest_name);
+		free(map[i].cpu_pid);
+		memset(&map[i], 0, sizeof(*map));
+	}
+	free(map);
+}
+
+/**
+ * @brief Get mapping of guest VCPU to host task, running that VCPU.
+ *	  Array of mappings for each guest is allocated and returned
+ *	  in map input parameter.
+ *
+ *
+ * @param map: Returns allocated array of kshark_host_guest_map structures, each
+ *	       one describing VCPUs mapping of one guest.
+ *
+ * @return The number of entries in the *map array, or a negative error code on
+ *	   failure.
+ */
+int kshark_tracecmd_get_hostguest_mapping(struct kshark_host_guest_map **map)
+{
+	struct kshark_host_guest_map *gmap = NULL;
+	struct tracecmd_input *peer_handle = NULL;
+	struct kshark_data_stream *peer_stream;
+	struct tracecmd_input *guest_handle = NULL;
+	struct kshark_data_stream *guest_stream;
+	struct kshark_context *kshark_ctx = NULL;
+	unsigned long long trace_id;
+	const char *name;
+	int vcpu_count;
+	const int *cpu_pid;
+	int *streamIds;
+	int i, j, k;
+	int count = 0;
+	int ret;
+
+	if (!map || !kshark_instance(&kshark_ctx))
+		return -EFAULT;
+	if (*map)
+		return -EEXIST;
+
+	streamIds = kshark_all_streams(kshark_ctx);
+	for (i = 0; i < kshark_ctx->n_streams; i++) {
+		guest_stream = kshark_get_data_stream(kshark_ctx, streamIds[i]);
+		if (!guest_stream || guest_stream->format != KS_TEP_DATA)
+			continue;
+		guest_handle = kshark_get_tep_input(guest_stream);
+		if (!guest_handle)
+			continue;
+		trace_id = tracecmd_get_traceid(guest_handle);
+		if (!trace_id)
+			continue;
+		for (j = 0; j < kshark_ctx->n_streams; j++) {
+			if (streamIds[i] == streamIds[j])
+				continue;
+			peer_stream = kshark_get_data_stream(kshark_ctx, streamIds[j]);
+			if (!peer_stream || peer_stream->format != KS_TEP_DATA)
+				continue;
+			peer_handle = kshark_get_tep_input(peer_stream);
+			if (!peer_handle)
+				continue;
+			ret = tracecmd_get_guest_cpumap(peer_handle, trace_id,
+							&name, &vcpu_count, &cpu_pid);
+			if (!ret && vcpu_count) {
+				gmap = realloc(*map,
+					       (count + 1) * sizeof(struct kshark_host_guest_map));
+				if (!gmap)
+					goto mem_error;
+				*map = gmap;
+				memset(&gmap[count], 0, sizeof(struct kshark_host_guest_map));
+				count++;
+				gmap[count - 1].guest_id = streamIds[i];
+				gmap[count - 1].host_id = streamIds[j];
+				gmap[count - 1].guest_name = strdup(name);
+				if (!gmap[count - 1].guest_name)
+					goto mem_error;
+				gmap[count - 1].vcpu_count = vcpu_count;
+				gmap[count - 1].cpu_pid = malloc(sizeof(int) * vcpu_count);
+				if (!gmap[count - 1].cpu_pid)
+					goto mem_error;
+				for (k = 0; k < vcpu_count; k++)
+					gmap[count - 1].cpu_pid[k] = cpu_pid[k];
+				break;
+			}
+		}
+	}
+
+	free(streamIds);
+	return count;
+
+mem_error:
+	free(streamIds);
+	if (*map) {
+		kshark_tracecmd_free_hostguest_map(*map, count);
+		*map = NULL;
+	}
+
+	return -ENOMEM;
+}
diff --git a/src/libkshark-tepdata.h b/src/libkshark-tepdata.h
index 53f6aff..76f488e 100644
--- a/src/libkshark-tepdata.h
+++ b/src/libkshark-tepdata.h
@@ -59,6 +59,16 @@ struct tep_record;
 ssize_t kshark_load_tep_records(struct kshark_context *kshark_ctx, int sd,
 				struct tep_record ***data_rows);
 
+struct kshark_host_guest_map {
+	int guest_id;		/* ID of guest stream */
+	int host_id;		/* ID of host stream */
+	char *guest_name;	/* Guest name */
+	int vcpu_count;		/* Number of guest's CPUs in *cpu_pid array */
+	int *cpu_pid;		/* Array of host task PIDs, index is the VCPU id */
+};
+void kshark_tracecmd_free_hostguest_map(struct kshark_host_guest_map *map, int count);
+int kshark_tracecmd_get_hostguest_mapping(struct kshark_host_guest_map **map);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/plugins/KVMCombo.cpp b/src/plugins/KVMCombo.cpp
index 1ae03aa..66bfab9 100644
--- a/src/plugins/KVMCombo.cpp
+++ b/src/plugins/KVMCombo.cpp
@@ -69,20 +69,30 @@ KsVCPUCheckBoxWidget::KsVCPUCheckBoxWidget(QWidget *parent)
 	_initTree();
 }
 
-void KsVCPUCheckBoxWidget::update(int sdHost, VCPUVector vcpus)
+void KsVCPUCheckBoxWidget::update(int GuestId,
+				  struct kshark_host_guest_map *gMap, int gMapCount)
 {
-	int nVCPUs = vcpus.count();
 	KsPlot::ColorTable colors;
+	int j;
+
+	for (j = 0; j < gMapCount; j++)
+		if (gMap[j].guest_id == GuestId)
+			break;
+	if (j == gMapCount)
+		return;
 
 	_tree.clear();
-	_id.resize(nVCPUs);
-	_cb.resize(nVCPUs);
+	_id.resize(gMap[j].vcpu_count);
+	_cb.resize(gMap[j].vcpu_count);
 	colors = KsPlot::getCPUColorTable();
 
-	for (int i = 0; i < nVCPUs; ++i) {
+	for (int i = 0; i < gMap[j].vcpu_count; ++i) {
+		QString strCPU = QLatin1String("vCPU ") + QString::number(i);
+		strCPU += (QLatin1String("\t<") + QLatin1String(gMap[j].guest_name) + QLatin1Char('>'));
+
 		QTreeWidgetItem *cpuItem = new QTreeWidgetItem;
 		cpuItem->setText(0, "  ");
-		cpuItem->setText(1, QString("vCPU %1").arg(vcpus.at(i).second));
+		cpuItem->setText(1, strCPU);
 		cpuItem->setCheckState(0, Qt::Checked);
 		cpuItem->setBackgroundColor(0, QColor(colors[i].r(),
 						      colors[i].g(),
@@ -168,26 +178,43 @@ KsComboPlotDialog::KsComboPlotDialog(QWidget *parent)
 		this,			SLOT(_guestStreamChanged(const QString &)));
 
 	setLayout(&_topLayout);
+
+	_guestMapCount = 0;
+	_guestMap = NULL;
 }
 
-void KsComboPlotDialog::update(int sdHost, VCPUVector vcpus)
+KsComboPlotDialog::~KsComboPlotDialog()
+{
+	kshark_tracecmd_free_hostguest_map(_guestMap, _guestMapCount);
+	_guestMap = NULL;
+	_guestMapCount = 0;
+}
+
+void KsComboPlotDialog::update()
 {
 	kshark_context *kshark_ctx(nullptr);
-	int sd, *streamIds;
+	int ret;
+	int sd;
+	int i;
 
 	if (!kshark_instance(&kshark_ctx))
 		return;
 
-	_sdHost = sdHost;
-	_vcpus = vcpus;
+	kshark_tracecmd_free_hostguest_map(_guestMap, _guestMapCount);
+	_guestMap = NULL;
+	_guestMapCount = 0;
+	ret = kshark_tracecmd_get_hostguest_mapping(&_guestMap);
+	if (ret > 0)
+		_guestMapCount = ret;
+
 	KsUtils::setElidedText(&_hostFileLabel,
-			       kshark_ctx->stream[sdHost]->file,
+			       kshark_ctx->stream[_guestMap[0].host_id]->file,
 			       Qt::ElideLeft, LABEL_WIDTH);
 
-	streamIds = kshark_all_streams(kshark_ctx);
-	for (int i = 0; i < kshark_ctx->n_streams; ++i) {
-		sd = streamIds[i];
-		if (sd == sdHost)
+	_guestStreamComboBox.clear();
+	for (i = 0; i < _guestMapCount; i++) {
+		sd = _guestMap[i].guest_id;
+		if (sd >= kshark_ctx->n_streams)
 			continue;
 
 		_guestStreamComboBox.addItem(kshark_ctx->stream[sd]->file,
@@ -200,8 +227,8 @@ void KsComboPlotDialog::update(int sdHost, VCPUVector vcpus)
 				this,		&KsComboPlotDialog::_applyPress);
 	}
 
-	_vcpuTree.update(sdHost, vcpus);
-	free(streamIds);
+	sd = _guestStreamComboBox.currentData().toInt();
+	_vcpuTree.update(sd, _guestMap, _guestMapCount);
 }
 
 void KsComboPlotDialog::_applyPress()
@@ -210,6 +237,16 @@ void KsComboPlotDialog::_applyPress()
 	QVector<int> allCombosVec;
 	KsComboPlot combo(2);
 	int nPlots(0);
+	int GuestId;
+	int j;
+
+	GuestId = _guestStreamComboBox.currentData().toInt();
+	for (j = 0; j < _guestMapCount; j++)
+		if (_guestMap[j].guest_id == GuestId)
+			break;
+	if (j == _guestMapCount)
+		return;
+
 
 	/*
 	 * Disconnect _applyButton. This is done in order to protect
@@ -218,17 +255,20 @@ void KsComboPlotDialog::_applyPress()
 	disconnect(_applyButtonConnection);
 
 	for (auto const &i: cbVec) {
+		if (i >= _guestMap[j].vcpu_count)
+			continue;
+
 		allCombosVec.append(2);
 
-		combo[0]._streamId = _guestStreamComboBox.currentData().toInt();
-		combo[0]._id = _vcpus.at(i).second;
+		combo[0]._streamId = _guestMap[j].guest_id;
+		combo[0]._id = i;
 		combo[0]._type = KsPlot::KSHARK_CPU_DRAW |
 				 KsPlot::KSHARK_GUEST_DRAW;
 
 		combo[0] >> allCombosVec;
 
-		combo[1]._streamId = _sdHost;
-		combo[1]._id = _vcpus.at(i).first;
+		combo[1]._streamId = _guestMap[j].host_id;
+		combo[1]._id = _guestMap[j].cpu_pid[i];
 		combo[1]._type = KsPlot::KSHARK_TASK_DRAW |
 				 KsPlot::KSHARK_HOST_DRAW;
 
@@ -241,66 +281,8 @@ void KsComboPlotDialog::_applyPress()
 
 void KsComboPlotDialog::_guestStreamChanged(const QString &sdStr)
 {
-
-}
-
-static int getVCPU(plugin_kvm_context *plugin_ctx,
-		   kshark_trace_histo *histo,
-		   int sdHost, int pid)
-{
-	int values[2] = {plugin_ctx->vm_entry_id, pid};
-	const kshark_entry *entry;
-	unsigned long long vcpu;
-
-	for (int b = 0; b < histo->n_bins; ++b) {
-		entry = ksmodel_get_entry_front(histo,
-						b, false,
-						kshark_match_event_and_pid,
-						sdHost, values,
-						nullptr,
-						nullptr);
-		if (!entry)
-			continue;
-
-		if (kshark_read_event_field(entry, "vcpu_id", &vcpu) >= 0)
-			return vcpu;
-	}
-
-	return -1;
-}
-
-HostMap getVCPUPids(kshark_context *kshark_ctx, kshark_trace_histo *histo)
-{
-	int sd, n_vcpus, *streamIds, *pids;
-	plugin_kvm_context *plugin_ctx;
-	HostMap hMap;
-
-	streamIds = kshark_all_streams(kshark_ctx);
-	for (int i = 0; i < kshark_ctx->n_streams; ++i) {
-		sd = streamIds[i];
-		plugin_ctx = get_kvm_context(sd);
-		if (!plugin_ctx)
-			continue;
-
-		/* This stream contains KVM events. */
-		n_vcpus = plugin_ctx->vcpu_pids->count;
-		if (n_vcpus) {
-			VCPUVector vcpus(n_vcpus);
-			pids = kshark_hash_ids(plugin_ctx->vcpu_pids);
-			for (int j = 0; j < n_vcpus; ++j) {
-				vcpus[j].first = pids[j];
-				vcpus[j].second = getVCPU(plugin_ctx,
-							  histo,
-							  sd, pids[j]);
-			}
-
-			free(pids);
-			hMap[sd] = vcpus;
-		}
-	}
-
-	free(streamIds);
-	return hMap;
+	int GuestId = _guestStreamComboBox.currentData().toInt();
+	_vcpuTree.update(GuestId, _guestMap, _guestMapCount);
 }
 
 KsComboPlotDialog dialog;
@@ -309,18 +291,11 @@ QMetaObject::Connection dialogConnection;
 static void showDialog(KsMainWindow *ks)
 {
 	kshark_context *kshark_ctx(nullptr);
-	kshark_trace_histo *histo;
-	VCPUVector vcpus;
-	HostMap hMap;
-	int sdHost;
 
 	if (!kshark_instance(&kshark_ctx))
 		return;
 
-	histo = ks->graphPtr()->glPtr()->model()->histo();
-	hMap = getVCPUPids(kshark_ctx, histo);
-
-	if (kshark_ctx->n_streams < 2 || hMap.count() != 1) {
+	if (kshark_ctx->n_streams < 2) {
 		QString err("Data from one Host and at least one Guest is required.");
 		QMessageBox msgBox;
 		msgBox.critical(nullptr, "Error", err);
@@ -328,10 +303,7 @@ static void showDialog(KsMainWindow *ks)
 		return;
 	}
 
-	sdHost = hMap.begin().key();
-	vcpus = hMap.begin().value();
-
-	dialog.update(sdHost, vcpus);
+	dialog.update();
 
 	if (!dialogConnection) {
 		dialogConnection =
diff --git a/src/plugins/KVMCombo.hpp b/src/plugins/KVMCombo.hpp
index b47f557..ecb9aeb 100644
--- a/src/plugins/KVMCombo.hpp
+++ b/src/plugins/KVMCombo.hpp
@@ -15,10 +15,6 @@
 #include "KsMainWindow.hpp"
 #include "KsWidgetsLib.hpp"
 
-typedef QVector<QPair<int, int>> VCPUVector;
-
-typedef QMap<int, VCPUVector> HostMap;
-
 /**
  * The KsVCPUCheckBoxWidget class provides a widget for selecting CPU plots to
  * show.
@@ -27,7 +23,8 @@ struct KsVCPUCheckBoxWidget : public KsWidgetsLib::KsCheckBoxTreeWidget
 {
 	explicit KsVCPUCheckBoxWidget(QWidget *parent = nullptr);
 
-	void update(int sdHost, VCPUVector vcpus);
+	void update(int GuestId,
+		    struct kshark_host_guest_map *gMap, int gMapCount);
 };
 
 /**
@@ -39,17 +36,16 @@ class KsComboPlotDialog : public QDialog
 	Q_OBJECT
 public:
 	explicit KsComboPlotDialog(QWidget *parent = nullptr);
-
-	void update(int sdHost, VCPUVector vcpus);
+	~KsComboPlotDialog();
+	void update();
 
 signals:
 	/** Signal emitted when the "Apply" button is pressed. */
 	void apply(int sd, QVector<int>);
 
 private:
-	int				_sdHost;
-
-	VCPUVector			_vcpus;
+	int				_guestMapCount;
+	struct kshark_host_guest_map 	*_guestMap;
 
 	KsVCPUCheckBoxWidget		_vcpuTree;
 
-- 
2.25.1


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

* Re: [PATCH v3 2/5] kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file
  2020-04-16 16:00 ` [PATCH v3 2/5] kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file Tzvetomir Stoyanov (VMware)
@ 2020-04-22 13:39   ` Yordan Karadzhov (VMware)
  0 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2020-04-22 13:39 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware), rostedt; +Cc: linux-trace-devel



On 16.04.20 г. 19:00 ч., Tzvetomir Stoyanov (VMware) wrote:
> From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> 
> The new tracecmd API tracecmd_open_head() allows opening a trace.dat
> file on stages. First, only the headers from the file are read and parsed,
> without reading the tracing data.
> The tracecmd_pair_peer() API is used to bind a tracing peer to
> this file, before parsing the trace data. This affects events timestamps
> correction when the tracing data is loaded.
> The tracecmd_get_guest_cpumap() API is used to find dependencies between
> all files and find a possible tracing peers.
> 
> This change depends on these trace-cmd patch sets:
>    "Useful APIs for merging tracing files"
> https://patchwork.kernel.org/project/linux-trace-devel/list/?series=268745
>    "Split reading the trace.dat options from trace data"
> https://patchwork.kernel.org/project/linux-trace-devel/list/?series=268743
> 
> Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> ---
>   src/libkshark-tepdata.c | 59 ++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> index 8678e12..f280e57 100644
> --- a/src/libkshark-tepdata.c
> +++ b/src/libkshark-tepdata.c
> @@ -985,6 +985,45 @@ const char *tep_plugin_names[] = {"sched_events",
>   
>   #define LINUX_IDLE_TASK_PID	0
>   
> +/** Find a host stream from the same tracing session, that has guest information */
> +struct tracecmd_input *kshark_tep_find_merge_peer(struct kshark_context *kshark_ctx,
> +						  struct tracecmd_input *handle)

This function can be defined static.

> +{
> +	struct tracecmd_input *peer_handle = NULL;
> +	struct kshark_data_stream *peer_stream;
> +	unsigned long long trace_id;
> +	int *streamIds = NULL;

Nit: please stick to "snake_case" in the C code and "camelCase in the 
C++ code.

No need to send new version. I am applying patchs 1-3 from this patch-set.

Thanks a lot!
Yordan

> +	int ret;
> +	int i;
> +
> +	trace_id = tracecmd_get_traceid(handle);
> +	if (!trace_id)
> +		goto out;
> +
> +	streamIds = kshark_all_streams(kshark_ctx);
> +	if (!streamIds)
> +		goto out;
> +	for (i = 0; i < kshark_ctx->n_streams; i++) {
> +		peer_stream = kshark_get_data_stream(kshark_ctx, streamIds[i]);
> +		if (!peer_stream || peer_stream->format != KS_TEP_DATA)
> +			continue;
> +		peer_handle = kshark_get_tep_input(peer_stream);
> +		if (!peer_handle)
> +			continue;
> +		ret = tracecmd_get_guest_cpumap(peer_handle, trace_id,
> +						NULL, NULL, NULL);
> +		if (!ret)
> +			break;
> +	}
> +
> +	if (i == kshark_ctx->n_streams)
> +		peer_handle = NULL;
> +
> +out:
> +	free(streamIds);
> +	return peer_handle;
> +}
> +
>   /** Initialize the FTRACE data input (from file). */
>   int kshark_tep_init_input(struct kshark_data_stream *stream,
>   			  const char *file)
> @@ -992,8 +1031,10 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
>   	struct kshark_context *kshark_ctx = NULL;
>   	struct tepdata_handle *tep_handle;
>   	struct kshark_plugin_list *plugin;
> +	struct tracecmd_input *merge_peer;
>   	struct tep_event *event;
>   	int i, n_tep_plugins;
> +	int ret;
>   
>   	if (!kshark_instance(&kshark_ctx) || !init_thread_seq())
>   		return -EEXIST;
> @@ -1009,13 +1050,29 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
>   	if (!tep_handle)
>   		return -EFAULT;
>   
> -	tep_handle->input = tracecmd_open(file);
> +	/** Open the tracing file, parse headers and create trace input context */
> +	tep_handle->input = tracecmd_open_head(file);
>   	if (!tep_handle->input) {
>   		free(tep_handle);
>   		stream->interface.handle = NULL;
>   		return -EEXIST;
>   	}
>   
> +	/** Find a merge peer from the same tracing session */
> +	merge_peer = kshark_tep_find_merge_peer(kshark_ctx, tep_handle->input);
> +	if (merge_peer)
> +		tracecmd_pair_peer(tep_handle->input, merge_peer);
> +
> +	/** Read the racing data from the file */
> +	ret = tracecmd_init_data(tep_handle->input);
> +
> +	if (ret < 0) {
> +		tracecmd_close(tep_handle->input);
> +		free(tep_handle);
> +		stream->interface.handle = NULL;
> +		return -EEXIST;
> +	}
> +
>   	tep_handle->tep = tracecmd_get_pevent(tep_handle->input);
>   
>   	tep_handle->sched_switch_event_id = -EINVAL;
> 

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

* Re: [PATCH v3 4/5] kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C
  2020-04-16 16:00 ` [PATCH v3 4/5] kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C Tzvetomir Stoyanov (VMware)
@ 2020-04-22 13:40   ` Yordan Karadzhov (VMware)
  0 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2020-04-22 13:40 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware), rostedt; +Cc: linux-trace-devel



On 16.04.20 г. 19:00 ч., Tzvetomir Stoyanov (VMware) wrote:
> From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> 
> C++ compiler uses name mangling to handle function overloading. As there is no function overloading in C, function names are not mangled. This breaks the linking of C library into C++ binary.
> Declare functions from trace-cmd.h as pure C, so the loader will not mangle the names when resolving them.
> 
> Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> ---
>   src/plugins/KVMCombo.cpp | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/src/plugins/KVMCombo.cpp b/src/plugins/KVMCombo.cpp
> index 3ff9ca5..1ae03aa 100644
> --- a/src/plugins/KVMCombo.cpp
> +++ b/src/plugins/KVMCombo.cpp
> @@ -13,7 +13,9 @@
>   #include<iostream>
>   
>   // trace-cmd
> +extern "C" {
>   #include "trace-cmd/trace-cmd.h"
> +}
>   

I think we do not need to include trace-cmd.h at all.
Thanks,
Yordan


>   // KernelShark
>   #include "libkshark.h"
> 

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

* Re: [PATCH v3 5/5] kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping information from the trace files
  2020-04-16 16:00 ` [PATCH v3 5/5] kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping information from the trace files Tzvetomir Stoyanov (VMware)
@ 2020-04-22 14:42   ` Yordan Karadzhov (VMware)
  0 siblings, 0 replies; 9+ messages in thread
From: Yordan Karadzhov (VMware) @ 2020-04-22 14:42 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: Linux Trace Devel



On 16.04.20 г. 19:00 ч., Tzvetomir Stoyanov (VMware) wrote:
> From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> 
> In the upcoming trace-cmd 2.9, extra information about host and guest tracing
> will be stored in the trace.dat files. Information about host CPU - guest vCPU
> mapping will be saved in host trace file. This mapping is mandatory for KVMCombo
> plugin. Currently, the mapping is extracted for the tracing data.
> Getting the information from the trace files is more reliable and solves the use
> case with more than one guest.

Hi Ceco,

I started testing the patch-set and the synchronization of the 
timestamps seems to be broken. I already reinstalled the latest version
of trace-cmd libs from kernel.org but when I append one of the guest 
data files it appears to be completely out of sync with the host data.
I am using the data files you sent me.

Apart from the sync problem, the plugin dialog seams to work fine. You 
can find below few nits.

> 
> Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> ---
>   src/libkshark-tepdata.c  | 116 ++++++++++++++++++++++++++++
>   src/libkshark-tepdata.h  |  10 +++
>   src/plugins/KVMCombo.cpp | 158 ++++++++++++++++-----------------------
>   src/plugins/KVMCombo.hpp |  16 ++--
>   4 files changed, 197 insertions(+), 103 deletions(-)
> 
> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> index f280e57..048981e 100644
> --- a/src/libkshark-tepdata.c
> +++ b/src/libkshark-tepdata.c
> @@ -22,6 +22,7 @@
>   #include "tracefs/tracefs.h"
>   
>   // KernelShark
> +#include "libkshark.h"
>   #include "libkshark-plugin.h"
>   #include "libkshark-tepdata.h"
>   
> @@ -1293,3 +1294,118 @@ char **kshark_tracecmd_local_plugins()
>   {
>   	return tracefs_tracers(tracefs_get_tracing_dir());
>   }
> +
> +/**
> + * @brief Free an array, allocated by kshark_tracecmd_get_hostguest_mapping() API
> + *
> + *
> + * @param map: Array, allocated by kshark_tracecmd_get_hostguest_mapping() API
> + * @param count: Number of entries in the array
> + *
> + */
> +void kshark_tracecmd_free_hostguest_map(struct kshark_host_guest_map *map, int count)
> +{
> +	int i;
> +
> +	if (!map)
> +		return;
> +	for (i = 0; i < count; i++) {
> +		free(map[i].guest_name);
> +		free(map[i].cpu_pid);
> +		memset(&map[i], 0, sizeof(*map));
> +	}
> +	free(map);
> +}
> +
> +/**
> + * @brief Get mapping of guest VCPU to host task, running that VCPU.
> + *	  Array of mappings for each guest is allocated and returned
> + *	  in map input parameter.
> + *
> + *
> + * @param map: Returns allocated array of kshark_host_guest_map structures, each
> + *	       one describing VCPUs mapping of one guest.
> + *
> + * @return The number of entries in the *map array, or a negative error code on
> + *	   failure.
> + */
> +int kshark_tracecmd_get_hostguest_mapping(struct kshark_host_guest_map **map)
> +{
> +	struct kshark_host_guest_map *gmap = NULL;
> +	struct tracecmd_input *peer_handle = NULL;
> +	struct kshark_data_stream *peer_stream;
> +	struct tracecmd_input *guest_handle = NULL;
> +	struct kshark_data_stream *guest_stream;
> +	struct kshark_context *kshark_ctx = NULL;
> +	unsigned long long trace_id;
> +	const char *name;
> +	int vcpu_count;
> +	const int *cpu_pid;
> +	int *streamIds;

Please use "snake_case" here.

> +	int i, j, k;
> +	int count = 0;
> +	int ret;
> +
> +	if (!map || !kshark_instance(&kshark_ctx))
> +		return -EFAULT;
> +	if (*map)
> +		return -EEXIST;
> +
> +	streamIds = kshark_all_streams(kshark_ctx);
> +	for (i = 0; i < kshark_ctx->n_streams; i++) {
> +		guest_stream = kshark_get_data_stream(kshark_ctx, streamIds[i]);
> +		if (!guest_stream || guest_stream->format != KS_TEP_DATA)
> +			continue;
> +		guest_handle = kshark_get_tep_input(guest_stream);
> +		if (!guest_handle)
> +			continue;
> +		trace_id = tracecmd_get_traceid(guest_handle);
> +		if (!trace_id)
> +			continue;
> +		for (j = 0; j < kshark_ctx->n_streams; j++) {
> +			if (streamIds[i] == streamIds[j])
> +				continue;
> +			peer_stream = kshark_get_data_stream(kshark_ctx, streamIds[j]);
> +			if (!peer_stream || peer_stream->format != KS_TEP_DATA)
> +				continue;
> +			peer_handle = kshark_get_tep_input(peer_stream);
> +			if (!peer_handle)
> +				continue;
> +			ret = tracecmd_get_guest_cpumap(peer_handle, trace_id,
> +							&name, &vcpu_count, &cpu_pid);
> +			if (!ret && vcpu_count) {
> +				gmap = realloc(*map,
> +					       (count + 1) * sizeof(struct kshark_host_guest_map));
> +				if (!gmap)
> +					goto mem_error;
> +				*map = gmap;
> +				memset(&gmap[count], 0, sizeof(struct kshark_host_guest_map));
> +				count++;
> +				gmap[count - 1].guest_id = streamIds[i];
> +				gmap[count - 1].host_id = streamIds[j];
> +				gmap[count - 1].guest_name = strdup(name);
> +				if (!gmap[count - 1].guest_name)
> +					goto mem_error;
> +				gmap[count - 1].vcpu_count = vcpu_count;
> +				gmap[count - 1].cpu_pid = malloc(sizeof(int) * vcpu_count);
> +				if (!gmap[count - 1].cpu_pid)
> +					goto mem_error;
> +				for (k = 0; k < vcpu_count; k++)
> +					gmap[count - 1].cpu_pid[k] = cpu_pid[k];
> +				break;
> +			}
> +		}
> +	}
> +
> +	free(streamIds);
> +	return count;
> +
> +mem_error:
> +	free(streamIds);
> +	if (*map) {
> +		kshark_tracecmd_free_hostguest_map(*map, count);
> +		*map = NULL;
> +	}
> +
> +	return -ENOMEM;
> +}
> diff --git a/src/libkshark-tepdata.h b/src/libkshark-tepdata.h
> index 53f6aff..76f488e 100644
> --- a/src/libkshark-tepdata.h
> +++ b/src/libkshark-tepdata.h
> @@ -59,6 +59,16 @@ struct tep_record;
>   ssize_t kshark_load_tep_records(struct kshark_context *kshark_ctx, int sd,
>   				struct tep_record ***data_rows);
>   
> +struct kshark_host_guest_map {
> +	int guest_id;		/* ID of guest stream */

In order to make Doxygen happy, you have to format the comments like this:

	/** ID of guest stream */
	int guest_id;	

and please add blank line between the fields of the struct.

> +	int host_id;		/* ID of host stream */
> +	char *guest_name;	/* Guest name */
> +	int vcpu_count;		/* Number of guest's CPUs in *cpu_pid array */
> +	int *cpu_pid;		/* Array of host task PIDs, index is the VCPU id */
> +};
> +void kshark_tracecmd_free_hostguest_map(struct kshark_host_guest_map *map, int count);
> +int kshark_tracecmd_get_hostguest_mapping(struct kshark_host_guest_map **map);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/src/plugins/KVMCombo.cpp b/src/plugins/KVMCombo.cpp
> index 1ae03aa..66bfab9 100644
> --- a/src/plugins/KVMCombo.cpp
> +++ b/src/plugins/KVMCombo.cpp
> @@ -69,20 +69,30 @@ KsVCPUCheckBoxWidget::KsVCPUCheckBoxWidget(QWidget *parent)
>   	_initTree();
>   }
>   
> -void KsVCPUCheckBoxWidget::update(int sdHost, VCPUVector vcpus)
> +void KsVCPUCheckBoxWidget::update(int GuestId,
> +				  struct kshark_host_guest_map *gMap, int gMapCount)

Since this is C++, you don't need to put "struct" in front of the user 
types like kshark_host_guest_map.

>   {
> -	int nVCPUs = vcpus.count();
>   	KsPlot::ColorTable colors;
> +	int j;
> +
> +	for (j = 0; j < gMapCount; j++)
> +		if (gMap[j].guest_id == GuestId)
> +			break;
> +	if (j == gMapCount)
> +		return;
>   
>   	_tree.clear();
> -	_id.resize(nVCPUs);
> -	_cb.resize(nVCPUs);
> +	_id.resize(gMap[j].vcpu_count);
> +	_cb.resize(gMap[j].vcpu_count);
>   	colors = KsPlot::getCPUColorTable();
>   
> -	for (int i = 0; i < nVCPUs; ++i) {
> +	for (int i = 0; i < gMap[j].vcpu_count; ++i) {
> +		QString strCPU = QLatin1String("vCPU ") + QString::number(i);
> +		strCPU += (QLatin1String("\t<") + QLatin1String(gMap[j].guest_name) + QLatin1Char('>'));
> +
>   		QTreeWidgetItem *cpuItem = new QTreeWidgetItem;
>   		cpuItem->setText(0, "  ");
> -		cpuItem->setText(1, QString("vCPU %1").arg(vcpus.at(i).second));
> +		cpuItem->setText(1, strCPU);
>   		cpuItem->setCheckState(0, Qt::Checked);
>   		cpuItem->setBackgroundColor(0, QColor(colors[i].r(),
>   						      colors[i].g(),
> @@ -168,26 +178,43 @@ KsComboPlotDialog::KsComboPlotDialog(QWidget *parent)
>   		this,			SLOT(_guestStreamChanged(const QString &)));
>   
>   	setLayout(&_topLayout);
> +
> +	_guestMapCount = 0;
> +	_guestMap = NULL;
>   }
>   
> -void KsComboPlotDialog::update(int sdHost, VCPUVector vcpus)
> +KsComboPlotDialog::~KsComboPlotDialog()
> +{
> +	kshark_tracecmd_free_hostguest_map(_guestMap, _guestMapCount);
> +	_guestMap = NULL;
> +	_guestMapCount = 0;

Do we need to set the values of _guestMap and _guestMapCount, if the 
object is going to be destroyed anyway.

Also please use nullptr instead of NULL in the C++ code.

Thanks!
Yordan

> +}
> +
> +void KsComboPlotDialog::update()
>   {
>   	kshark_context *kshark_ctx(nullptr);
> -	int sd, *streamIds;
> +	int ret;
> +	int sd;
> +	int i;
>   
>   	if (!kshark_instance(&kshark_ctx))
>   		return;
>   
> -	_sdHost = sdHost;
> -	_vcpus = vcpus;
> +	kshark_tracecmd_free_hostguest_map(_guestMap, _guestMapCount);
> +	_guestMap = NULL;
> +	_guestMapCount = 0;
> +	ret = kshark_tracecmd_get_hostguest_mapping(&_guestMap);
> +	if (ret > 0)
> +		_guestMapCount = ret;
> +
>   	KsUtils::setElidedText(&_hostFileLabel,
> -			       kshark_ctx->stream[sdHost]->file,
> +			       kshark_ctx->stream[_guestMap[0].host_id]->file,
>   			       Qt::ElideLeft, LABEL_WIDTH);
>   
> -	streamIds = kshark_all_streams(kshark_ctx);
> -	for (int i = 0; i < kshark_ctx->n_streams; ++i) {
> -		sd = streamIds[i];
> -		if (sd == sdHost)
> +	_guestStreamComboBox.clear();
> +	for (i = 0; i < _guestMapCount; i++) {
> +		sd = _guestMap[i].guest_id;
> +		if (sd >= kshark_ctx->n_streams)
>   			continue;
>   
>   		_guestStreamComboBox.addItem(kshark_ctx->stream[sd]->file,
> @@ -200,8 +227,8 @@ void KsComboPlotDialog::update(int sdHost, VCPUVector vcpus)
>   				this,		&KsComboPlotDialog::_applyPress);
>   	}
>   
> -	_vcpuTree.update(sdHost, vcpus);
> -	free(streamIds);
> +	sd = _guestStreamComboBox.currentData().toInt();
> +	_vcpuTree.update(sd, _guestMap, _guestMapCount);
>   }
>   
>   void KsComboPlotDialog::_applyPress()
> @@ -210,6 +237,16 @@ void KsComboPlotDialog::_applyPress()
>   	QVector<int> allCombosVec;
>   	KsComboPlot combo(2);
>   	int nPlots(0);
> +	int GuestId;
> +	int j;
> +
> +	GuestId = _guestStreamComboBox.currentData().toInt();
> +	for (j = 0; j < _guestMapCount; j++)
> +		if (_guestMap[j].guest_id == GuestId)
> +			break;
> +	if (j == _guestMapCount)
> +		return;
> +
>   
>   	/*
>   	 * Disconnect _applyButton. This is done in order to protect
> @@ -218,17 +255,20 @@ void KsComboPlotDialog::_applyPress()
>   	disconnect(_applyButtonConnection);
>   
>   	for (auto const &i: cbVec) {
> +		if (i >= _guestMap[j].vcpu_count)
> +			continue;
> +
>   		allCombosVec.append(2);
>   
> -		combo[0]._streamId = _guestStreamComboBox.currentData().toInt();
> -		combo[0]._id = _vcpus.at(i).second;
> +		combo[0]._streamId = _guestMap[j].guest_id;
> +		combo[0]._id = i;
>   		combo[0]._type = KsPlot::KSHARK_CPU_DRAW |
>   				 KsPlot::KSHARK_GUEST_DRAW;
>   
>   		combo[0] >> allCombosVec;
>   
> -		combo[1]._streamId = _sdHost;
> -		combo[1]._id = _vcpus.at(i).first;
> +		combo[1]._streamId = _guestMap[j].host_id;
> +		combo[1]._id = _guestMap[j].cpu_pid[i];
>   		combo[1]._type = KsPlot::KSHARK_TASK_DRAW |
>   				 KsPlot::KSHARK_HOST_DRAW;
>   
> @@ -241,66 +281,8 @@ void KsComboPlotDialog::_applyPress()
>   
>   void KsComboPlotDialog::_guestStreamChanged(const QString &sdStr)
>   {
> -
> -}
> -
> -static int getVCPU(plugin_kvm_context *plugin_ctx,
> -		   kshark_trace_histo *histo,
> -		   int sdHost, int pid)
> -{
> -	int values[2] = {plugin_ctx->vm_entry_id, pid};
> -	const kshark_entry *entry;
> -	unsigned long long vcpu;
> -
> -	for (int b = 0; b < histo->n_bins; ++b) {
> -		entry = ksmodel_get_entry_front(histo,
> -						b, false,
> -						kshark_match_event_and_pid,
> -						sdHost, values,
> -						nullptr,
> -						nullptr);
> -		if (!entry)
> -			continue;
> -
> -		if (kshark_read_event_field(entry, "vcpu_id", &vcpu) >= 0)
> -			return vcpu;
> -	}
> -
> -	return -1;
> -}
> -
> -HostMap getVCPUPids(kshark_context *kshark_ctx, kshark_trace_histo *histo)
> -{
> -	int sd, n_vcpus, *streamIds, *pids;
> -	plugin_kvm_context *plugin_ctx;
> -	HostMap hMap;
> -
> -	streamIds = kshark_all_streams(kshark_ctx);
> -	for (int i = 0; i < kshark_ctx->n_streams; ++i) {
> -		sd = streamIds[i];
> -		plugin_ctx = get_kvm_context(sd);
> -		if (!plugin_ctx)
> -			continue;
> -
> -		/* This stream contains KVM events. */
> -		n_vcpus = plugin_ctx->vcpu_pids->count;
> -		if (n_vcpus) {
> -			VCPUVector vcpus(n_vcpus);
> -			pids = kshark_hash_ids(plugin_ctx->vcpu_pids);
> -			for (int j = 0; j < n_vcpus; ++j) {
> -				vcpus[j].first = pids[j];
> -				vcpus[j].second = getVCPU(plugin_ctx,
> -							  histo,
> -							  sd, pids[j]);
> -			}
> -
> -			free(pids);
> -			hMap[sd] = vcpus;
> -		}
> -	}
> -
> -	free(streamIds);
> -	return hMap;
> +	int GuestId = _guestStreamComboBox.currentData().toInt();
> +	_vcpuTree.update(GuestId, _guestMap, _guestMapCount);
>   }
>   
>   KsComboPlotDialog dialog;
> @@ -309,18 +291,11 @@ QMetaObject::Connection dialogConnection;
>   static void showDialog(KsMainWindow *ks)
>   {
>   	kshark_context *kshark_ctx(nullptr);
> -	kshark_trace_histo *histo;
> -	VCPUVector vcpus;
> -	HostMap hMap;
> -	int sdHost;
>   
>   	if (!kshark_instance(&kshark_ctx))
>   		return;
>   
> -	histo = ks->graphPtr()->glPtr()->model()->histo();
> -	hMap = getVCPUPids(kshark_ctx, histo);
> -
> -	if (kshark_ctx->n_streams < 2 || hMap.count() != 1) {
> +	if (kshark_ctx->n_streams < 2) {
>   		QString err("Data from one Host and at least one Guest is required.");
>   		QMessageBox msgBox;
>   		msgBox.critical(nullptr, "Error", err);
> @@ -328,10 +303,7 @@ static void showDialog(KsMainWindow *ks)
>   		return;
>   	}
>   
> -	sdHost = hMap.begin().key();
> -	vcpus = hMap.begin().value();
> -
> -	dialog.update(sdHost, vcpus);
> +	dialog.update();
>   
>   	if (!dialogConnection) {
>   		dialogConnection =
> diff --git a/src/plugins/KVMCombo.hpp b/src/plugins/KVMCombo.hpp
> index b47f557..ecb9aeb 100644
> --- a/src/plugins/KVMCombo.hpp
> +++ b/src/plugins/KVMCombo.hpp
> @@ -15,10 +15,6 @@
>   #include "KsMainWindow.hpp"
>   #include "KsWidgetsLib.hpp"
>   
> -typedef QVector<QPair<int, int>> VCPUVector;
> -
> -typedef QMap<int, VCPUVector> HostMap;
> -
>   /**
>    * The KsVCPUCheckBoxWidget class provides a widget for selecting CPU plots to
>    * show.
> @@ -27,7 +23,8 @@ struct KsVCPUCheckBoxWidget : public KsWidgetsLib::KsCheckBoxTreeWidget
>   {
>   	explicit KsVCPUCheckBoxWidget(QWidget *parent = nullptr);
>   
> -	void update(int sdHost, VCPUVector vcpus);
> +	void update(int GuestId,
> +		    struct kshark_host_guest_map *gMap, int gMapCount);
>   };
>   
>   /**
> @@ -39,17 +36,16 @@ class KsComboPlotDialog : public QDialog
>   	Q_OBJECT
>   public:
>   	explicit KsComboPlotDialog(QWidget *parent = nullptr);
> -
> -	void update(int sdHost, VCPUVector vcpus);
> +	~KsComboPlotDialog();
> +	void update();
>   
>   signals:
>   	/** Signal emitted when the "Apply" button is pressed. */
>   	void apply(int sd, QVector<int>);
>   
>   private:
> -	int				_sdHost;
> -
> -	VCPUVector			_vcpus;
> +	int				_guestMapCount;
> +	struct kshark_host_guest_map 	*_guestMap;
>   
>   	KsVCPUCheckBoxWidget		_vcpuTree;
>   
> 

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

end of thread, other threads:[~2020-04-22 14:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 16:00 [PATCH v3 0/5] Various enhancements, related to reading trace.dat file Tzvetomir Stoyanov (VMware)
2020-04-16 16:00 ` [PATCH v3 1/5] kernel-shark-2.alpha: Adjust the width of marker buttons Tzvetomir Stoyanov (VMware)
2020-04-16 16:00 ` [PATCH v3 2/5] kernel-shark-2.alpha: Use new tracecmd APIs to open guest tracing file Tzvetomir Stoyanov (VMware)
2020-04-22 13:39   ` Yordan Karadzhov (VMware)
2020-04-16 16:00 ` [PATCH v3 3/5] kernel-shark-2.alpha: Print the plugin's file name in case of loading error Tzvetomir Stoyanov (VMware)
2020-04-16 16:00 ` [PATCH v3 4/5] kernel-shark-2.alpha: Force trace-cmd.h to be used as plain C Tzvetomir Stoyanov (VMware)
2020-04-22 13:40   ` Yordan Karadzhov (VMware)
2020-04-16 16:00 ` [PATCH v3 5/5] kernel-shark-2.alpha: Restructure KVMCombo plugin to use CPU mapping information from the trace files Tzvetomir Stoyanov (VMware)
2020-04-22 14:42   ` Yordan Karadzhov (VMware)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).