[PATCH v5 00/16] perf kvm: Support histograms and TUI mode

[PATCH v5 00/16] perf kvm: Support histograms and TUI mode

[Leo Yan]

This patch set is to enable histograms and (partially) TUI mode in perf
kvm tool; patch set v1 [1] gives brief introduction for the change.

We can use below commands for testing this series:

In a terminal, you could launch a virtual machine with qemu command; in
below case, I downloaded a Ubuntu (or Debian) iso file and used it as
the file system image:

  $ qemu-system-x86_64 -M pc -enable-kvm -cpu host -m 4096 -hda ubuntu-22.04-desktop-amd64.iso

Then in another terminal, I can use below command to capture KVM trace
data and report the result:

  # cd linux/tools/perf
  # ./perf kvm stat record
  # ./perf kvm stat report          => Output in TUI mode
  # ./perf kvm stat report --stdio  => Output in stdio mode

Changes from v4:
* Fixed building failure in patch 04; compiled successfully for every
  patch to ensure bisection.

Changes from v3:
* Updated patch 04 to change comparison function return type from int to
  int64_t to avoid overflow;
* Added a refactoring patch 05 "perf kvm: Use macro to replace variable
  'decode_str_len'";
* Refined to use hists__add_entry_ops() and hist_entry__cmp() to compare
  hist entry and add new entries, to support comparison between hist
  entry, added a new patch 09; (Namhyung Kim)
* Updated patch 10 for comparison event name; (Namhyung Kim)
* Refined patch 11 to move init_stats() and hists__inc_nr_samples()
  into kvm_he_zalloc(); (Namhyung Kim)
* Minor refactoring for sort_result() and kvm_hists__init() in patch 15.

Changes from v2:
* Found building failure with command 'make VF=1 DEBUG=1 NO_SLANG=1',
  fixed it in the patch 13. (James Clark)

Changes from v1:
* Updated the patch 13 'perf kvm: Add TUI mode for stat report' to avoid
  building failure if no support HAVE_SLANG_SUPPORT.

[1] https://lore.kernel.org/lkml/20230226042053.1492409-1-leo.yan@linaro.org/


Leo Yan (16):
  perf kvm: Refactor overall statistics
  perf kvm: Add pointer to 'perf_kvm_stat' in kvm event
  perf kvm: Move up metrics helpers
  perf kvm: Use subtraction for comparison metrics
  perf kvm: Use macro to replace variable 'decode_str_len'
  perf kvm: Introduce histograms data structures
  perf kvm: Pass argument 'sample' to kvm_alloc_init_event()
  perf kvm: Parse address location for samples
  perf hist: Add 'kvm_info' field in histograms entry
  perf kvm: Add dimensions for KVM event statistics
  perf kvm: Use histograms list to replace cached list
  perf kvm: Polish sorting key
  perf kvm: Support printing attributions for dimensions
  perf kvm: Add dimensions for percentages
  perf kvm: Add TUI mode for stat report
  perf kvm: Update documentation to reflect new changes

 tools/perf/Documentation/perf-kvm.txt   |   9 +-
 tools/perf/arch/arm64/util/kvm-stat.c   |   1 -
 tools/perf/arch/powerpc/util/kvm-stat.c |   3 +-
 tools/perf/arch/s390/util/kvm-stat.c    |   1 -
 tools/perf/arch/x86/util/kvm-stat.c     |   7 +-
 tools/perf/builtin-annotate.c           |   2 +-
 tools/perf/builtin-c2c.c                |   4 +-
 tools/perf/builtin-diff.c               |   4 +-
 tools/perf/builtin-kvm.c                | 859 +++++++++++++++++++-----
 tools/perf/tests/hists_link.c           |   4 +-
 tools/perf/util/hist.c                  |  19 +-
 tools/perf/util/hist.h                  |   3 +
 tools/perf/util/kvm-stat.h              |  33 +-
 tools/perf/util/sort.h                  |   1 +
 14 files changed, 745 insertions(+), 205 deletions(-)

-- 
2.34.1

[PATCH v5 01/16] perf kvm: Refactor overall statistics

[Leo Yan]

Currently the tool computes overall statistics when sort the results.
This patch refactors overall statistics during events processing,
therefore, the function update_total_coun() is not needed anymore, an
extra benefit is we can de-couple code between the statistics and the
sorting.

This patch is not expected any functionality changes.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 641e739c717c..0172e5b0d26e 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -278,9 +278,14 @@ static double kvm_event_rel_stddev(int vcpu_id, struct kvm_event *event)
 				avg_stats(&kvm_stats->stats));
 }
 
-static bool update_kvm_event(struct kvm_event *event, int vcpu_id,
+static bool update_kvm_event(struct perf_kvm_stat *kvm,
+			     struct kvm_event *event, int vcpu_id,
 			     u64 time_diff)
 {
+	/* Update overall statistics */
+	kvm->total_count++;
+	kvm->total_time += time_diff;
+
 	if (vcpu_id == -1) {
 		kvm_update_event_stats(&event->total, time_diff);
 		return true;
@@ -399,7 +404,7 @@ static bool handle_end_event(struct perf_kvm_stat *kvm,
 		}
 	}
 
-	return update_kvm_event(event, vcpu, time_diff);
+	return update_kvm_event(kvm, event, vcpu, time_diff);
 }
 
 static
@@ -526,15 +531,6 @@ static void insert_to_result(struct rb_root *result, struct kvm_event *event,
 	rb_insert_color(&event->rb, result);
 }
 
-static void
-update_total_count(struct perf_kvm_stat *kvm, struct kvm_event *event)
-{
-	int vcpu = kvm->trace_vcpu;
-
-	kvm->total_count += get_event_count(event, vcpu);
-	kvm->total_time += get_event_time(event, vcpu);
-}
-
 static bool event_is_valid(struct kvm_event *event, int vcpu)
 {
 	return !!get_event_count(event, vcpu);
@@ -549,7 +545,6 @@ static void sort_result(struct perf_kvm_stat *kvm)
 	for (i = 0; i < EVENTS_CACHE_SIZE; i++) {
 		list_for_each_entry(event, &kvm->kvm_events_cache[i], hash_entry) {
 			if (event_is_valid(event, vcpu)) {
-				update_total_count(kvm, event);
 				insert_to_result(&kvm->result, event,
 						 kvm->compare, vcpu);
 			}
-- 
2.34.1

[PATCH v5 02/16] perf kvm: Add pointer to 'perf_kvm_stat' in kvm event

[Leo Yan]

Sometimes, handling kvm events needs to base on global variables, e.g.
when read event counts we need to know the target vcpu ID; the global
variables are stored in structure perf_kvm_stat.

This patch adds add a 'perf_kvm_stat' pointer in kvm event structure,
it is to be used by later refactoring.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c   | 6 ++++--
 tools/perf/util/kvm-stat.h | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 0172e5b0d26e..3d2560ec6b37 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -209,7 +209,8 @@ static bool kvm_event_expand(struct kvm_event *event, int vcpu_id)
 	return true;
 }
 
-static struct kvm_event *kvm_alloc_init_event(struct event_key *key)
+static struct kvm_event *kvm_alloc_init_event(struct perf_kvm_stat *kvm,
+					      struct event_key *key)
 {
 	struct kvm_event *event;
 
@@ -219,6 +220,7 @@ static struct kvm_event *kvm_alloc_init_event(struct event_key *key)
 		return NULL;
 	}
 
+	event->perf_kvm = kvm;
 	event->key = *key;
 	init_stats(&event->total.stats);
 	return event;
@@ -238,7 +240,7 @@ static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
 			return event;
 	}
 
-	event = kvm_alloc_init_event(key);
+	event = kvm_alloc_init_event(kvm, key);
 	if (!event)
 		return NULL;
 
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 6f0fa05b62b6..40a4b66cfee6 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -24,10 +24,13 @@ struct kvm_event_stats {
 	struct stats stats;
 };
 
+struct perf_kvm_stat;
+
 struct kvm_event {
 	struct list_head hash_entry;
 	struct rb_node rb;
 
+	struct perf_kvm_stat *perf_kvm;
 	struct event_key key;
 
 	struct kvm_event_stats total;
@@ -44,8 +47,6 @@ struct kvm_event_key {
 	key_cmp_fun key;
 };
 
-struct perf_kvm_stat;
-
 struct child_event_ops {
 	void (*get_key)(struct evsel *evsel,
 			struct perf_sample *sample,
-- 
2.34.1

[PATCH v5 03/16] perf kvm: Move up metrics helpers

[Leo Yan]

This patch moves up the helper functions of event's metrics for later
adding code to call them.

No any functionality changes, but has a function renaming from
compare_kvm_event_{metric}() to cmp_event_{metric}().

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c | 72 ++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 3d2560ec6b37..62c097a37da9 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -49,6 +49,42 @@
 #include <math.h>
 #include <perf/mmap.h>
 
+#define GET_EVENT_KEY(func, field)					\
+static u64 get_event_ ##func(struct kvm_event *event, int vcpu)		\
+{									\
+	if (vcpu == -1)							\
+		return event->total.field;				\
+									\
+	if (vcpu >= event->max_vcpu)					\
+		return 0;						\
+									\
+	return event->vcpu[vcpu].field;					\
+}
+
+#define COMPARE_EVENT_KEY(func, field)					\
+GET_EVENT_KEY(func, field)						\
+static int cmp_event_ ## func(struct kvm_event *one,			\
+			      struct kvm_event *two, int vcpu)		\
+{									\
+	return get_event_ ##func(one, vcpu) >				\
+	       get_event_ ##func(two, vcpu);				\
+}
+
+GET_EVENT_KEY(time, time);
+GET_EVENT_KEY(max, stats.max);
+GET_EVENT_KEY(min, stats.min);
+COMPARE_EVENT_KEY(count, stats.n);
+COMPARE_EVENT_KEY(mean, stats.mean);
+
+#define DEF_SORT_NAME_KEY(name, compare_key)				\
+	{ #name, cmp_event_ ## compare_key }
+
+static struct kvm_event_key keys[] = {
+	DEF_SORT_NAME_KEY(sample, count),
+	DEF_SORT_NAME_KEY(time, mean),
+	{ NULL, NULL }
+};
+
 static const char *get_filename_for_perf_kvm(void)
 {
 	const char *filename;
@@ -461,42 +497,6 @@ static bool handle_kvm_event(struct perf_kvm_stat *kvm,
 	return true;
 }
 
-#define GET_EVENT_KEY(func, field)					\
-static u64 get_event_ ##func(struct kvm_event *event, int vcpu)		\
-{									\
-	if (vcpu == -1)							\
-		return event->total.field;				\
-									\
-	if (vcpu >= event->max_vcpu)					\
-		return 0;						\
-									\
-	return event->vcpu[vcpu].field;					\
-}
-
-#define COMPARE_EVENT_KEY(func, field)					\
-GET_EVENT_KEY(func, field)						\
-static int compare_kvm_event_ ## func(struct kvm_event *one,		\
-					struct kvm_event *two, int vcpu)\
-{									\
-	return get_event_ ##func(one, vcpu) >				\
-				get_event_ ##func(two, vcpu);		\
-}
-
-GET_EVENT_KEY(time, time);
-COMPARE_EVENT_KEY(count, stats.n);
-COMPARE_EVENT_KEY(mean, stats.mean);
-GET_EVENT_KEY(max, stats.max);
-GET_EVENT_KEY(min, stats.min);
-
-#define DEF_SORT_NAME_KEY(name, compare_key)				\
-	{ #name, compare_kvm_event_ ## compare_key }
-
-static struct kvm_event_key keys[] = {
-	DEF_SORT_NAME_KEY(sample, count),
-	DEF_SORT_NAME_KEY(time, mean),
-	{ NULL, NULL }
-};
-
 static bool select_key(struct perf_kvm_stat *kvm)
 {
 	int i;
-- 
2.34.1

Re: [PATCH v5 03/16] perf kvm: Move up metrics helpers

[Arnaldo Carvalho de Melo]

Em Wed, Mar 15, 2023 at 10:50:59PM +0800, Leo Yan escreveu:
> This patch moves up the helper functions of event's metrics for later
> adding code to call them.
> 
> No any functionality changes, but has a function renaming from
> compare_kvm_event_{metric}() to cmp_event_{metric}().

    Those helper functions are only used if this is true:
    
      if defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
    
    So keep them enclosed with that.

Did it here


diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 62c097a37da9b9fe..c11f5454f35c544a 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -49,6 +49,7 @@
 #include <math.h>
 #include <perf/mmap.h>
 
+#if defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
 #define GET_EVENT_KEY(func, field)					\
 static u64 get_event_ ##func(struct kvm_event *event, int vcpu)		\
 {									\
@@ -84,6 +85,7 @@ static struct kvm_event_key keys[] = {
 	DEF_SORT_NAME_KEY(time, mean),
 	{ NULL, NULL }
 };
+#endif // defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
 
 static const char *get_filename_for_perf_kvm(void)
 {
 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Reviewed-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/builtin-kvm.c | 72 ++++++++++++++++++++--------------------
>  1 file changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 3d2560ec6b37..62c097a37da9 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -49,6 +49,42 @@
>  #include <math.h>
>  #include <perf/mmap.h>
>  
> +#define GET_EVENT_KEY(func, field)					\
> +static u64 get_event_ ##func(struct kvm_event *event, int vcpu)		\
> +{									\
> +	if (vcpu == -1)							\
> +		return event->total.field;				\
> +									\
> +	if (vcpu >= event->max_vcpu)					\
> +		return 0;						\
> +									\
> +	return event->vcpu[vcpu].field;					\
> +}
> +
> +#define COMPARE_EVENT_KEY(func, field)					\
> +GET_EVENT_KEY(func, field)						\
> +static int cmp_event_ ## func(struct kvm_event *one,			\
> +			      struct kvm_event *two, int vcpu)		\
> +{									\
> +	return get_event_ ##func(one, vcpu) >				\
> +	       get_event_ ##func(two, vcpu);				\
> +}
> +
> +GET_EVENT_KEY(time, time);
> +GET_EVENT_KEY(max, stats.max);
> +GET_EVENT_KEY(min, stats.min);
> +COMPARE_EVENT_KEY(count, stats.n);
> +COMPARE_EVENT_KEY(mean, stats.mean);
> +
> +#define DEF_SORT_NAME_KEY(name, compare_key)				\
> +	{ #name, cmp_event_ ## compare_key }
> +
> +static struct kvm_event_key keys[] = {
> +	DEF_SORT_NAME_KEY(sample, count),
> +	DEF_SORT_NAME_KEY(time, mean),
> +	{ NULL, NULL }
> +};
> +
>  static const char *get_filename_for_perf_kvm(void)
>  {
>  	const char *filename;
> @@ -461,42 +497,6 @@ static bool handle_kvm_event(struct perf_kvm_stat *kvm,
>  	return true;
>  }
>  
> -#define GET_EVENT_KEY(func, field)					\
> -static u64 get_event_ ##func(struct kvm_event *event, int vcpu)		\
> -{									\
> -	if (vcpu == -1)							\
> -		return event->total.field;				\
> -									\
> -	if (vcpu >= event->max_vcpu)					\
> -		return 0;						\
> -									\
> -	return event->vcpu[vcpu].field;					\
> -}
> -
> -#define COMPARE_EVENT_KEY(func, field)					\
> -GET_EVENT_KEY(func, field)						\
> -static int compare_kvm_event_ ## func(struct kvm_event *one,		\
> -					struct kvm_event *two, int vcpu)\
> -{									\
> -	return get_event_ ##func(one, vcpu) >				\
> -				get_event_ ##func(two, vcpu);		\
> -}
> -
> -GET_EVENT_KEY(time, time);
> -COMPARE_EVENT_KEY(count, stats.n);
> -COMPARE_EVENT_KEY(mean, stats.mean);
> -GET_EVENT_KEY(max, stats.max);
> -GET_EVENT_KEY(min, stats.min);
> -
> -#define DEF_SORT_NAME_KEY(name, compare_key)				\
> -	{ #name, compare_kvm_event_ ## compare_key }
> -
> -static struct kvm_event_key keys[] = {
> -	DEF_SORT_NAME_KEY(sample, count),
> -	DEF_SORT_NAME_KEY(time, mean),
> -	{ NULL, NULL }
> -};
> -
>  static bool select_key(struct perf_kvm_stat *kvm)
>  {
>  	int i;
> -- 
> 2.34.1
> 

-- 

- Arnaldo

[PATCH v5 04/16] perf kvm: Use subtraction for comparison metrics

[Leo Yan]

Currently the metrics comparison uses greater operator (>), it returns
the boolean value (0 or 1).

This patch changes to use subtraction as comparison result, which can
be used by histograms sorting.  Since the subtraction result is u64
type, we change key_cmp_fun's return type to int64_t to avoid overflow.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-kvm.c   | 6 +++---
 tools/perf/util/kvm-stat.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 62c097a37da9..4e9519390da6 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -63,10 +63,10 @@ static u64 get_event_ ##func(struct kvm_event *event, int vcpu)		\
 
 #define COMPARE_EVENT_KEY(func, field)					\
 GET_EVENT_KEY(func, field)						\
-static int cmp_event_ ## func(struct kvm_event *one,			\
+static int64_t cmp_event_ ## func(struct kvm_event *one,		\
 			      struct kvm_event *two, int vcpu)		\
 {									\
-	return get_event_ ##func(one, vcpu) >				\
+	return get_event_ ##func(one, vcpu) -				\
 	       get_event_ ##func(two, vcpu);				\
 }
 
@@ -523,7 +523,7 @@ static void insert_to_result(struct rb_root *result, struct kvm_event *event,
 		p = container_of(*rb, struct kvm_event, rb);
 		parent = *rb;
 
-		if (bigger(event, p, vcpu))
+		if (bigger(event, p, vcpu) > 0)
 			rb = &(*rb)->rb_left;
 		else
 			rb = &(*rb)->rb_right;
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 40a4b66cfee6..0c2400d9b227 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -40,7 +40,7 @@ struct kvm_event {
 	struct kvm_event_stats *vcpu;
 };
 
-typedef int (*key_cmp_fun)(struct kvm_event*, struct kvm_event*, int);
+typedef int64_t (*key_cmp_fun)(struct kvm_event*, struct kvm_event*, int);
 
 struct kvm_event_key {
 	const char *name;
-- 
2.34.1

[PATCH v5 05/16] perf kvm: Use macro to replace variable 'decode_str_len'

[Leo Yan]

The variable 'decode_str_len' defines the string length for KVM event
name and every arch defines its own values.

This introduces complexity that the variable definition are spreading in
multiple source files under arch folder.  This patch refactors code to
use a macro KVM_EVENT_NAME_LEN to define event name length and thus
remove the definitions in arch files.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/arch/arm64/util/kvm-stat.c   |  1 -
 tools/perf/arch/powerpc/util/kvm-stat.c |  3 +--
 tools/perf/arch/s390/util/kvm-stat.c    |  1 -
 tools/perf/arch/x86/util/kvm-stat.c     |  7 +++----
 tools/perf/builtin-kvm.c                | 10 +++++-----
 tools/perf/util/kvm-stat.h              |  3 ++-
 6 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/tools/perf/arch/arm64/util/kvm-stat.c b/tools/perf/arch/arm64/util/kvm-stat.c
index 73d18e0ed6f6..72ca9bb45804 100644
--- a/tools/perf/arch/arm64/util/kvm-stat.c
+++ b/tools/perf/arch/arm64/util/kvm-stat.c
@@ -11,7 +11,6 @@ define_exit_reasons_table(arm64_trap_exit_reasons, kvm_arm_exception_class);
 
 const char *kvm_trap_exit_reason = "esr_ec";
 const char *vcpu_id_str = "id";
-const int decode_str_len = 20;
 const char *kvm_exit_reason = "ret";
 const char *kvm_entry_trace = "kvm:kvm_entry";
 const char *kvm_exit_trace = "kvm:kvm_exit";
diff --git a/tools/perf/arch/powerpc/util/kvm-stat.c b/tools/perf/arch/powerpc/util/kvm-stat.c
index 1a9b40ea92a5..d04a08c9fd19 100644
--- a/tools/perf/arch/powerpc/util/kvm-stat.c
+++ b/tools/perf/arch/powerpc/util/kvm-stat.c
@@ -14,7 +14,6 @@
 #define NR_TPS 4
 
 const char *vcpu_id_str = "vcpu_id";
-const int decode_str_len = 40;
 const char *kvm_entry_trace = "kvm_hv:kvm_guest_enter";
 const char *kvm_exit_trace = "kvm_hv:kvm_guest_exit";
 
@@ -80,7 +79,7 @@ static void hcall_event_decode_key(struct perf_kvm_stat *kvm __maybe_unused,
 {
 	const char *hcall_reason = get_hcall_exit_reason(key->key);
 
-	scnprintf(decode, decode_str_len, "%s", hcall_reason);
+	scnprintf(decode, KVM_EVENT_NAME_LEN, "%s", hcall_reason);
 }
 
 static struct kvm_events_ops hcall_events = {
diff --git a/tools/perf/arch/s390/util/kvm-stat.c b/tools/perf/arch/s390/util/kvm-stat.c
index 34da89ced29a..0aed92df51ba 100644
--- a/tools/perf/arch/s390/util/kvm-stat.c
+++ b/tools/perf/arch/s390/util/kvm-stat.c
@@ -19,7 +19,6 @@ define_exit_reasons_table(sie_diagnose_codes, diagnose_codes);
 define_exit_reasons_table(sie_icpt_prog_codes, icpt_prog_codes);
 
 const char *vcpu_id_str = "id";
-const int decode_str_len = 40;
 const char *kvm_exit_reason = "icptcode";
 const char *kvm_entry_trace = "kvm:kvm_s390_sie_enter";
 const char *kvm_exit_trace = "kvm:kvm_s390_sie_exit";
diff --git a/tools/perf/arch/x86/util/kvm-stat.c b/tools/perf/arch/x86/util/kvm-stat.c
index c5dd54f6ef5e..ef513def03ba 100644
--- a/tools/perf/arch/x86/util/kvm-stat.c
+++ b/tools/perf/arch/x86/util/kvm-stat.c
@@ -18,7 +18,6 @@ static struct kvm_events_ops exit_events = {
 };
 
 const char *vcpu_id_str = "vcpu_id";
-const int decode_str_len = 20;
 const char *kvm_exit_reason = "exit_reason";
 const char *kvm_entry_trace = "kvm:kvm_entry";
 const char *kvm_exit_trace = "kvm:kvm_exit";
@@ -77,7 +76,7 @@ static void mmio_event_decode_key(struct perf_kvm_stat *kvm __maybe_unused,
 				  struct event_key *key,
 				  char *decode)
 {
-	scnprintf(decode, decode_str_len, "%#lx:%s",
+	scnprintf(decode, KVM_EVENT_NAME_LEN, "%#lx:%s",
 		  (unsigned long)key->key,
 		  key->info == KVM_TRACE_MMIO_WRITE ? "W" : "R");
 }
@@ -121,7 +120,7 @@ static void ioport_event_decode_key(struct perf_kvm_stat *kvm __maybe_unused,
 				    struct event_key *key,
 				    char *decode)
 {
-	scnprintf(decode, decode_str_len, "%#llx:%s",
+	scnprintf(decode, KVM_EVENT_NAME_LEN, "%#llx:%s",
 		  (unsigned long long)key->key,
 		  key->info ? "POUT" : "PIN");
 }
@@ -165,7 +164,7 @@ static void msr_event_decode_key(struct perf_kvm_stat *kvm __maybe_unused,
 				    struct event_key *key,
 				    char *decode)
 {
-	scnprintf(decode, decode_str_len, "%#llx:%s",
+	scnprintf(decode, KVM_EVENT_NAME_LEN, "%#llx:%s",
 		  (unsigned long long)key->key,
 		  key->info ? "W" : "R");
 }
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 4e9519390da6..d400434aa137 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -159,7 +159,7 @@ void exit_event_decode_key(struct perf_kvm_stat *kvm,
 	const char *exit_reason = get_exit_reason(kvm, key->exit_reasons,
 						  key->key);
 
-	scnprintf(decode, decode_str_len, "%s", exit_reason);
+	scnprintf(decode, KVM_EVENT_NAME_LEN, "%s", exit_reason);
 }
 
 static bool register_kvm_events_ops(struct perf_kvm_stat *kvm)
@@ -432,7 +432,7 @@ static bool handle_end_event(struct perf_kvm_stat *kvm,
 	time_diff = sample->time - time_begin;
 
 	if (kvm->duration && time_diff > kvm->duration) {
-		char decode[decode_str_len];
+		char decode[KVM_EVENT_NAME_LEN];
 
 		kvm->events_ops->decode_key(kvm, &event->key, decode);
 		if (!skip_event(decode)) {
@@ -603,7 +603,7 @@ static void show_timeofday(void)
 
 static void print_result(struct perf_kvm_stat *kvm)
 {
-	char decode[decode_str_len];
+	char decode[KVM_EVENT_NAME_LEN];
 	struct kvm_event *event;
 	int vcpu = kvm->trace_vcpu;
 
@@ -614,7 +614,7 @@ static void print_result(struct perf_kvm_stat *kvm)
 
 	pr_info("\n\n");
 	print_vcpu_info(kvm);
-	pr_info("%*s ", decode_str_len, kvm->events_ops->name);
+	pr_info("%*s ", KVM_EVENT_NAME_LEN, kvm->events_ops->name);
 	pr_info("%10s ", "Samples");
 	pr_info("%9s ", "Samples%");
 
@@ -633,7 +633,7 @@ static void print_result(struct perf_kvm_stat *kvm)
 		min = get_event_min(event, vcpu);
 
 		kvm->events_ops->decode_key(kvm, &event->key, decode);
-		pr_info("%*s ", decode_str_len, decode);
+		pr_info("%*s ", KVM_EVENT_NAME_LEN, decode);
 		pr_info("%10llu ", (unsigned long long)ecount);
 		pr_info("%8.2f%% ", (double)ecount / kvm->total_count * 100);
 		pr_info("%8.2f%% ", (double)etime / kvm->total_time * 100);
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 0c2400d9b227..841b3174c211 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -8,6 +8,8 @@
 #include "stat.h"
 #include "record.h"
 
+#define KVM_EVENT_NAME_LEN	40
+
 struct evsel;
 struct evlist;
 struct perf_session;
@@ -143,7 +145,6 @@ extern const char *kvm_events_tp[];
 extern struct kvm_reg_events_ops kvm_reg_events_ops[];
 extern const char * const kvm_skip_events[];
 extern const char *vcpu_id_str;
-extern const int decode_str_len;
 extern const char *kvm_exit_reason;
 extern const char *kvm_entry_trace;
 extern const char *kvm_exit_trace;
-- 
2.34.1

[PATCH v5 06/16] perf kvm: Introduce histograms data structures

[Leo Yan]

This is a preparation to support histograms in perf kvm tool.  As first
step, this patch defines histograms data structures and initialize them.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c   | 18 ++++++++++++++++++
 tools/perf/util/kvm-stat.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index d400434aa137..384992c8a01a 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -85,6 +85,20 @@ static struct kvm_event_key keys[] = {
 	{ NULL, NULL }
 };
 
+struct kvm_hists {
+	struct hists		hists;
+	struct perf_hpp_list	list;
+};
+
+static struct kvm_hists kvm_hists;
+
+static int kvm_hists__init(void)
+{
+	__hists__init(&kvm_hists.hists, &kvm_hists.list);
+	perf_hpp_list__init(&kvm_hists.list);
+	return 0;
+}
+
 static const char *get_filename_for_perf_kvm(void)
 {
 	const char *filename;
@@ -957,6 +971,8 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 	set_term_quiet_input(&save);
 	init_kvm_event_record(kvm);
 
+	kvm_hists__init();
+
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
@@ -1152,6 +1168,8 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 	init_kvm_event_record(kvm);
 	setup_pager();
 
+	kvm_hists__init();
+
 	ret = read_events(kvm);
 	if (ret)
 		goto exit;
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 841b3174c211..e2c17662bac7 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -5,6 +5,7 @@
 #ifdef HAVE_KVM_STAT_SUPPORT
 
 #include "tool.h"
+#include "sort.h"
 #include "stat.h"
 #include "record.h"
 
-- 
2.34.1

Re: [PATCH v5 06/16] perf kvm: Introduce histograms data structures

[Arnaldo Carvalho de Melo]

Em Wed, Mar 15, 2023 at 10:51:02PM +0800, Leo Yan escreveu:
> This is a preparation to support histograms in perf kvm tool.  As first
> step, this patch defines histograms data structures and initialize them.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Reviewed-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/builtin-kvm.c   | 18 ++++++++++++++++++
>  tools/perf/util/kvm-stat.h |  1 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index d400434aa137..384992c8a01a 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -85,6 +85,20 @@ static struct kvm_event_key keys[] = {
>  	{ NULL, NULL }
>  };
>  
> +struct kvm_hists {
> +	struct hists		hists;
> +	struct perf_hpp_list	list;
> +};
> +
> +static struct kvm_hists kvm_hists;
> +
> +static int kvm_hists__init(void)
> +{
> +	__hists__init(&kvm_hists.hists, &kvm_hists.list);
> +	perf_hpp_list__init(&kvm_hists.list);
> +	return 0;
> +}
> +

Had to add:


diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index c4cb34df155fec67..b06c11d306a11cab 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -85,7 +85,6 @@ static struct kvm_event_key keys[] = {
 	DEF_SORT_NAME_KEY(time, mean),
 	{ NULL, NULL }
 };
-#endif // defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
 
 struct kvm_hists {
 	struct hists		hists;
@@ -100,6 +99,7 @@ static int kvm_hists__init(void)
 	perf_hpp_list__init(&kvm_hists.list);
 	return 0;
 }
+#endif // defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
 
 static const char *get_filename_for_perf_kvm(void)
 {

>  static const char *get_filename_for_perf_kvm(void)
>  {
>  	const char *filename;
> @@ -957,6 +971,8 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
>  	set_term_quiet_input(&save);
>  	init_kvm_event_record(kvm);
>  
> +	kvm_hists__init();
> +
>  	signal(SIGINT, sig_handler);
>  	signal(SIGTERM, sig_handler);
>  
> @@ -1152,6 +1168,8 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
>  	init_kvm_event_record(kvm);
>  	setup_pager();
>  
> +	kvm_hists__init();
> +
>  	ret = read_events(kvm);
>  	if (ret)
>  		goto exit;
> diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
> index 841b3174c211..e2c17662bac7 100644
> --- a/tools/perf/util/kvm-stat.h
> +++ b/tools/perf/util/kvm-stat.h
> @@ -5,6 +5,7 @@
>  #ifdef HAVE_KVM_STAT_SUPPORT
>  
>  #include "tool.h"
> +#include "sort.h"
>  #include "stat.h"
>  #include "record.h"
>  
> -- 
> 2.34.1
> 

-- 

- Arnaldo

Re: [PATCH v5 06/16] perf kvm: Introduce histograms data structures

[Leo Yan]

Hi Arnaldo,

On Wed, Mar 15, 2023 at 04:45:53PM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> Had to add:
> 
> 
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index c4cb34df155fec67..b06c11d306a11cab 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -85,7 +85,6 @@ static struct kvm_event_key keys[] = {
>  	DEF_SORT_NAME_KEY(time, mean),
>  	{ NULL, NULL }
>  };
> -#endif // defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
>  
>  struct kvm_hists {
>  	struct hists		hists;
> @@ -100,6 +99,7 @@ static int kvm_hists__init(void)
>  	perf_hpp_list__init(&kvm_hists.list);
>  	return 0;
>  }
> +#endif // defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)

Thanks for fixing up, this is fine for me.

Seems to me it is not a good practice to use macros (and nested macros) to
mute or unmute big chunk code.  I will try to refine the building
builtin-kvm.c and prepare patches based on current code base.

P.s. I saw this series has been picked up into your branch
acme/tmp.perf-tools-next, if you want me to follow up anything, please
let me know.  Thank you!

Leo

[PATCH v5 07/16] perf kvm: Pass argument 'sample' to kvm_alloc_init_event()

[Leo Yan]

This patch adds an argument 'sample' for kvm_alloc_init_event(), and its
caller functions are updated as well for passing down the 'sample'
pointer.

This is a preparation change to allow later patch to create histograms
entries for kvm event, no any functionality changes.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 384992c8a01a..5ea723fd45a4 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -260,7 +260,8 @@ static bool kvm_event_expand(struct kvm_event *event, int vcpu_id)
 }
 
 static struct kvm_event *kvm_alloc_init_event(struct perf_kvm_stat *kvm,
-					      struct event_key *key)
+					      struct event_key *key,
+					      struct perf_sample *sample __maybe_unused)
 {
 	struct kvm_event *event;
 
@@ -277,7 +278,8 @@ static struct kvm_event *kvm_alloc_init_event(struct perf_kvm_stat *kvm,
 }
 
 static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
-					       struct event_key *key)
+					       struct event_key *key,
+					       struct perf_sample *sample)
 {
 	struct kvm_event *event;
 	struct list_head *head;
@@ -290,7 +292,7 @@ static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
 			return event;
 	}
 
-	event = kvm_alloc_init_event(kvm, key);
+	event = kvm_alloc_init_event(kvm, key, sample);
 	if (!event)
 		return NULL;
 
@@ -300,15 +302,16 @@ static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
 
 static bool handle_begin_event(struct perf_kvm_stat *kvm,
 			       struct vcpu_event_record *vcpu_record,
-			       struct event_key *key, u64 timestamp)
+			       struct event_key *key,
+			       struct perf_sample *sample)
 {
 	struct kvm_event *event = NULL;
 
 	if (key->key != INVALID_KEY)
-		event = find_create_kvm_event(kvm, key);
+		event = find_create_kvm_event(kvm, key, sample);
 
 	vcpu_record->last_event = event;
-	vcpu_record->start_time = timestamp;
+	vcpu_record->start_time = sample->time;
 	return true;
 }
 
@@ -375,12 +378,12 @@ static bool is_child_event(struct perf_kvm_stat *kvm,
 static bool handle_child_event(struct perf_kvm_stat *kvm,
 			       struct vcpu_event_record *vcpu_record,
 			       struct event_key *key,
-			       struct perf_sample *sample __maybe_unused)
+			       struct perf_sample *sample)
 {
 	struct kvm_event *event = NULL;
 
 	if (key->key != INVALID_KEY)
-		event = find_create_kvm_event(kvm, key);
+		event = find_create_kvm_event(kvm, key, sample);
 
 	vcpu_record->last_event = event;
 
@@ -429,7 +432,7 @@ static bool handle_end_event(struct perf_kvm_stat *kvm,
 		return true;
 
 	if (!event)
-		event = find_create_kvm_event(kvm, key);
+		event = find_create_kvm_event(kvm, key, sample);
 
 	if (!event)
 		return false;
@@ -500,7 +503,7 @@ static bool handle_kvm_event(struct perf_kvm_stat *kvm,
 		return true;
 
 	if (kvm->events_ops->is_begin_event(evsel, sample, &key))
-		return handle_begin_event(kvm, vcpu_record, &key, sample->time);
+		return handle_begin_event(kvm, vcpu_record, &key, sample);
 
 	if (is_child_event(kvm, evsel, sample, &key))
 		return handle_child_event(kvm, vcpu_record, &key, sample);
-- 
2.34.1

[PATCH v5 08/16] perf kvm: Parse address location for samples

[Leo Yan]

Parse address location for samples and save it into the structure
'perf_kvm_stat', it is to be used by histograms entry.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c   | 5 +++++
 tools/perf/util/kvm-stat.h | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 5ea723fd45a4..fa91c8deb628 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -704,6 +704,11 @@ static int process_sample_event(struct perf_tool *tool,
 	if (skip_sample(kvm, sample))
 		return 0;
 
+	if (machine__resolve(machine, &kvm->al, sample) < 0) {
+		pr_warning("Fail to resolve address location, skip sample.\n");
+		return 0;
+	}
+
 	thread = machine__findnew_thread(machine, sample->pid, sample->tid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index e2c17662bac7..3b2eab91a9be 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -7,6 +7,7 @@
 #include "tool.h"
 #include "sort.h"
 #include "stat.h"
+#include "symbol.h"
 #include "record.h"
 
 #define KVM_EVENT_NAME_LEN	40
@@ -88,6 +89,9 @@ struct perf_kvm_stat {
 	const char *sort_key;
 	int trace_vcpu;
 
+	/* Used when process events */
+	struct addr_location al;
+
 	struct exit_reasons_table *exit_reasons;
 	const char *exit_reasons_isa;
 
-- 
2.34.1

[PATCH v5 09/16] perf hist: Add 'kvm_info' field in histograms entry

[Leo Yan]

__hists__add_entry() creates a temporary entry and compare it with
existed histograms entries, if any existed entry equals to the
temporary entry it skips to allocation to avoid duplication.

The problem for support KVM event in histograms is it doesn't contain
any info to identify KVM event and can be used for comparison entries.

This patch adds 'kvm_info' field in the histograms entry which contains
the KVM event's key, this identifier will be used for comparison
histograms entries in later change.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-annotate.c |  2 +-
 tools/perf/builtin-c2c.c      |  4 ++--
 tools/perf/builtin-diff.c     |  4 ++--
 tools/perf/tests/hists_link.c |  4 ++--
 tools/perf/util/hist.c        | 19 ++++++++++++-------
 tools/perf/util/hist.h        |  3 +++
 tools/perf/util/kvm-stat.h    |  4 ++++
 tools/perf/util/sort.h        |  1 +
 8 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 90458ca6933f..4750fac7bf93 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -252,7 +252,7 @@ static int evsel__add_sample(struct evsel *evsel, struct perf_sample *sample,
 	if (ann->has_br_stack && has_annotation(ann))
 		return process_branch_callback(evsel, sample, al, ann, machine);
 
-	he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
+	he = hists__add_entry(hists, al, NULL, NULL, NULL, NULL, sample, true);
 	if (he == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 56974eae0638..d3181fee4d3d 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -315,7 +315,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 	c2c_decode_stats(&stats, mi);
 
 	he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
-				  &al, NULL, NULL, mi,
+				  &al, NULL, NULL, mi, NULL,
 				  sample, true);
 	if (he == NULL)
 		goto free_mi;
@@ -349,7 +349,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 			goto free_mi;
 
 		he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
-					  &al, NULL, NULL, mi,
+					  &al, NULL, NULL, mi, NULL,
 					  sample, true);
 		if (he == NULL)
 			goto free_mi;
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index ed07cc6cca56..22b526766e14 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -423,7 +423,7 @@ static int diff__process_sample_event(struct perf_tool *tool,
 	switch (compute) {
 	case COMPUTE_CYCLES:
 		if (!hists__add_entry_ops(hists, &block_hist_ops, &al, NULL,
-					  NULL, NULL, sample, true)) {
+					  NULL, NULL, NULL, sample, true)) {
 			pr_warning("problem incrementing symbol period, "
 				   "skipping event\n");
 			goto out_put;
@@ -442,7 +442,7 @@ static int diff__process_sample_event(struct perf_tool *tool,
 		break;
 
 	default:
-		if (!hists__add_entry(hists, &al, NULL, NULL, NULL, sample,
+		if (!hists__add_entry(hists, &al, NULL, NULL, NULL, NULL, sample,
 				      true)) {
 			pr_warning("problem incrementing symbol period, "
 				   "skipping event\n");
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 14b2ff808b5e..e7e4ee57ce04 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -87,7 +87,7 @@ static int add_hist_entries(struct evlist *evlist, struct machine *machine)
 				goto out;
 
 			he = hists__add_entry(hists, &al, NULL,
-						NULL, NULL, &sample, true);
+					      NULL, NULL, NULL, &sample, true);
 			if (he == NULL) {
 				addr_location__put(&al);
 				goto out;
@@ -106,7 +106,7 @@ static int add_hist_entries(struct evlist *evlist, struct machine *machine)
 				goto out;
 
 			he = hists__add_entry(hists, &al, NULL,
-						NULL, NULL, &sample, true);
+					      NULL, NULL, NULL, &sample, true);
 			if (he == NULL) {
 				addr_location__put(&al);
 				goto out;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b6e4b4edde43..3670136a0074 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -4,6 +4,7 @@
 #include "dso.h"
 #include "build-id.h"
 #include "hist.h"
+#include "kvm-stat.h"
 #include "map.h"
 #include "map_symbol.h"
 #include "branch.h"
@@ -698,6 +699,7 @@ __hists__add_entry(struct hists *hists,
 		   struct symbol *sym_parent,
 		   struct branch_info *bi,
 		   struct mem_info *mi,
+		   struct kvm_info *ki,
 		   struct block_info *block_info,
 		   struct perf_sample *sample,
 		   bool sample_self,
@@ -733,6 +735,7 @@ __hists__add_entry(struct hists *hists,
 		.hists	= hists,
 		.branch_info = bi,
 		.mem_info = mi,
+		.kvm_info = ki,
 		.block_info = block_info,
 		.transaction = sample->transaction,
 		.raw_data = sample->raw_data,
@@ -756,10 +759,11 @@ struct hist_entry *hists__add_entry(struct hists *hists,
 				    struct symbol *sym_parent,
 				    struct branch_info *bi,
 				    struct mem_info *mi,
+				    struct kvm_info *ki,
 				    struct perf_sample *sample,
 				    bool sample_self)
 {
-	return __hists__add_entry(hists, al, sym_parent, bi, mi, NULL,
+	return __hists__add_entry(hists, al, sym_parent, bi, mi, ki, NULL,
 				  sample, sample_self, NULL);
 }
 
@@ -769,10 +773,11 @@ struct hist_entry *hists__add_entry_ops(struct hists *hists,
 					struct symbol *sym_parent,
 					struct branch_info *bi,
 					struct mem_info *mi,
+					struct kvm_info *ki,
 					struct perf_sample *sample,
 					bool sample_self)
 {
-	return __hists__add_entry(hists, al, sym_parent, bi, mi, NULL,
+	return __hists__add_entry(hists, al, sym_parent, bi, mi, ki, NULL,
 				  sample, sample_self, ops);
 }
 
@@ -846,7 +851,7 @@ iter_add_single_mem_entry(struct hist_entry_iter *iter, struct addr_location *al
 	 */
 	sample->period = cost;
 
-	he = hists__add_entry(hists, al, iter->parent, NULL, mi,
+	he = hists__add_entry(hists, al, iter->parent, NULL, mi, NULL,
 			      sample, true);
 	if (!he)
 		return -ENOMEM;
@@ -949,7 +954,7 @@ iter_add_next_branch_entry(struct hist_entry_iter *iter, struct addr_location *a
 	sample->period = 1;
 	sample->weight = bi->flags.cycles ? bi->flags.cycles : 1;
 
-	he = hists__add_entry(hists, al, iter->parent, &bi[i], NULL,
+	he = hists__add_entry(hists, al, iter->parent, &bi[i], NULL, NULL,
 			      sample, true);
 	if (he == NULL)
 		return -ENOMEM;
@@ -987,7 +992,7 @@ iter_add_single_normal_entry(struct hist_entry_iter *iter, struct addr_location
 	struct hist_entry *he;
 
 	he = hists__add_entry(evsel__hists(evsel), al, iter->parent, NULL, NULL,
-			      sample, true);
+			      NULL, sample, true);
 	if (he == NULL)
 		return -ENOMEM;
 
@@ -1047,7 +1052,7 @@ iter_add_single_cumulative_entry(struct hist_entry_iter *iter,
 	struct hist_entry *he;
 	int err = 0;
 
-	he = hists__add_entry(hists, al, iter->parent, NULL, NULL,
+	he = hists__add_entry(hists, al, iter->parent, NULL, NULL, NULL,
 			      sample, true);
 	if (he == NULL)
 		return -ENOMEM;
@@ -1148,7 +1153,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter,
 	}
 
 	he = hists__add_entry(evsel__hists(evsel), al, iter->parent, NULL, NULL,
-			      sample, false);
+			      NULL, sample, false);
 	if (he == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index d93a4e510dc7..86a677954279 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -14,6 +14,7 @@ struct hist_entry_ops;
 struct addr_location;
 struct map_symbol;
 struct mem_info;
+struct kvm_info;
 struct branch_info;
 struct branch_stack;
 struct block_info;
@@ -150,6 +151,7 @@ struct hist_entry *hists__add_entry(struct hists *hists,
 				    struct symbol *parent,
 				    struct branch_info *bi,
 				    struct mem_info *mi,
+				    struct kvm_info *ki,
 				    struct perf_sample *sample,
 				    bool sample_self);
 
@@ -159,6 +161,7 @@ struct hist_entry *hists__add_entry_ops(struct hists *hists,
 					struct symbol *sym_parent,
 					struct branch_info *bi,
 					struct mem_info *mi,
+					struct kvm_info *ki,
 					struct perf_sample *sample,
 					bool sample_self);
 
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 3b2eab91a9be..0cf704333c4c 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -23,6 +23,10 @@ struct event_key {
 	struct exit_reasons_table *exit_reasons;
 };
 
+struct kvm_info {
+	char name[KVM_EVENT_NAME_LEN];
+};
+
 struct kvm_event_stats {
 	u64 time;
 	struct stats stats;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index d79a100e5999..22f437c3476f 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -144,6 +144,7 @@ struct hist_entry {
 	struct hists		*hists;
 	struct mem_info		*mem_info;
 	struct block_info	*block_info;
+	struct kvm_info		*kvm_info;
 	void			*raw_data;
 	u32			raw_size;
 	int			num_res;
-- 
2.34.1

[PATCH v5 10/16] perf kvm: Add dimensions for KVM event statistics

[Leo Yan]

To support KVM event statistics, this patch firstly registers histograms
columns and sorting fields; every column or field has its own format
structure, the format structure is dereferenced to access the dimension,
finally the dimension provides the comparison callback for sorting
result.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-kvm.c   | 232 ++++++++++++++++++++++++++++++++++++-
 tools/perf/util/kvm-stat.h |   2 +
 2 files changed, 230 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index fa91c8deb628..3f601ccb7aab 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -70,9 +70,9 @@ static int64_t cmp_event_ ## func(struct kvm_event *one,		\
 	       get_event_ ##func(two, vcpu);				\
 }
 
-GET_EVENT_KEY(time, time);
-GET_EVENT_KEY(max, stats.max);
-GET_EVENT_KEY(min, stats.min);
+COMPARE_EVENT_KEY(time, time);
+COMPARE_EVENT_KEY(max, stats.max);
+COMPARE_EVENT_KEY(min, stats.min);
 COMPARE_EVENT_KEY(count, stats.n);
 COMPARE_EVENT_KEY(mean, stats.mean);
 
@@ -90,13 +90,237 @@ struct kvm_hists {
 	struct perf_hpp_list	list;
 };
 
+struct kvm_dimension {
+	const char *name;
+	int64_t (*cmp)(struct perf_hpp_fmt *fmt, struct hist_entry *left,
+		       struct hist_entry *right);
+};
+
+struct kvm_fmt {
+	struct perf_hpp_fmt	fmt;
+	struct kvm_dimension	*dim;
+};
+
 static struct kvm_hists kvm_hists;
 
+static int64_t ev_name_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
+			   struct hist_entry *left,
+			   struct hist_entry *right)
+{
+	/* Return opposite number for sorting in alphabetical order */
+	return -strcmp(left->kvm_info->name, right->kvm_info->name);
+}
+
+static struct kvm_dimension dim_event = {
+	.name		= "ev_name",
+	.cmp		= ev_name_cmp,
+};
+
+#define EV_METRIC_CMP(metric)						\
+static int64_t ev_cmp_##metric(struct perf_hpp_fmt *fmt __maybe_unused,	\
+			       struct hist_entry *left,			\
+			       struct hist_entry *right)		\
+{									\
+	struct kvm_event *event_left;					\
+	struct kvm_event *event_right;					\
+	struct perf_kvm_stat *perf_kvm;					\
+									\
+	event_left  = container_of(left, struct kvm_event, he);		\
+	event_right = container_of(right, struct kvm_event, he);	\
+									\
+	perf_kvm = event_left->perf_kvm;				\
+	return cmp_event_##metric(event_left, event_right,		\
+				  perf_kvm->trace_vcpu);		\
+}
+
+EV_METRIC_CMP(time)
+EV_METRIC_CMP(count)
+EV_METRIC_CMP(max)
+EV_METRIC_CMP(min)
+EV_METRIC_CMP(mean)
+
+static struct kvm_dimension dim_time = {
+	.name		= "time",
+	.cmp		= ev_cmp_time,
+};
+
+static struct kvm_dimension dim_count = {
+	.name		= "sample",
+	.cmp		= ev_cmp_count,
+};
+
+static struct kvm_dimension dim_max_time = {
+	.name		= "max_t",
+	.cmp		= ev_cmp_max,
+};
+
+static struct kvm_dimension dim_min_time = {
+	.name		= "min_t",
+	.cmp		= ev_cmp_min,
+};
+
+static struct kvm_dimension dim_mean_time = {
+	.name		= "mean_t",
+	.cmp		= ev_cmp_mean,
+};
+
+static struct kvm_dimension *dimensions[] = {
+	&dim_event,
+	&dim_time,
+	&dim_count,
+	&dim_max_time,
+	&dim_min_time,
+	&dim_mean_time,
+	NULL,
+};
+
+static bool fmt_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
+{
+	struct kvm_fmt *kvm_fmt_a = container_of(a, struct kvm_fmt, fmt);
+	struct kvm_fmt *kvm_fmt_b = container_of(b, struct kvm_fmt, fmt);
+
+	return kvm_fmt_a->dim == kvm_fmt_b->dim;
+}
+
+static void fmt_free(struct perf_hpp_fmt *fmt)
+{
+	struct kvm_fmt *kvm_fmt;
+
+	kvm_fmt = container_of(fmt, struct kvm_fmt, fmt);
+	free(kvm_fmt);
+}
+
+static struct kvm_dimension *get_dimension(const char *name)
+{
+	unsigned int i;
+
+	for (i = 0; dimensions[i] != NULL; i++) {
+		if (!strcmp(dimensions[i]->name, name))
+			return dimensions[i];
+	}
+
+	return NULL;
+}
+
+static struct kvm_fmt *get_format(const char *name)
+{
+	struct kvm_dimension *dim = get_dimension(name);
+	struct kvm_fmt *kvm_fmt;
+	struct perf_hpp_fmt *fmt;
+
+	if (!dim)
+		return NULL;
+
+	kvm_fmt = zalloc(sizeof(*kvm_fmt));
+	if (!kvm_fmt)
+		return NULL;
+
+	kvm_fmt->dim = dim;
+
+	fmt = &kvm_fmt->fmt;
+	INIT_LIST_HEAD(&fmt->list);
+	INIT_LIST_HEAD(&fmt->sort_list);
+	fmt->cmp	= dim->cmp;
+	fmt->sort	= dim->cmp;
+	fmt->color	= NULL;
+	fmt->entry	= NULL;
+	fmt->header	= NULL;
+	fmt->width	= NULL;
+	fmt->collapse	= dim->cmp;
+	fmt->equal	= fmt_equal;
+	fmt->free	= fmt_free;
+
+	return kvm_fmt;
+}
+
+static int kvm_hists__init_output(struct perf_hpp_list *hpp_list, char *name)
+{
+	struct kvm_fmt *kvm_fmt = get_format(name);
+
+	if (!kvm_fmt) {
+		pr_warning("Fail to find format for output field %s.\n", name);
+		return -EINVAL;
+	}
+
+	perf_hpp_list__column_register(hpp_list, &kvm_fmt->fmt);
+	return 0;
+}
+
+static int kvm_hists__init_sort(struct perf_hpp_list *hpp_list, char *name)
+{
+	struct kvm_fmt *kvm_fmt = get_format(name);
+
+	if (!kvm_fmt) {
+		pr_warning("Fail to find format for sorting %s.\n", name);
+		return -EINVAL;
+	}
+
+	perf_hpp_list__register_sort_field(hpp_list, &kvm_fmt->fmt);
+	return 0;
+}
+
+static int kvm_hpp_list__init(char *list,
+			      struct perf_hpp_list *hpp_list,
+			      int (*fn)(struct perf_hpp_list *hpp_list,
+					char *name))
+{
+	char *tmp, *tok;
+	int ret;
+
+	if (!list || !fn)
+		return 0;
+
+	for (tok = strtok_r(list, ", ", &tmp); tok;
+	     tok = strtok_r(NULL, ", ", &tmp)) {
+		ret = fn(hpp_list, tok);
+		if (!ret)
+			continue;
+
+		/* Handle errors */
+		if (ret == -EINVAL)
+			pr_err("Invalid field key: '%s'", tok);
+		else if (ret == -ESRCH)
+			pr_err("Unknown field key: '%s'", tok);
+		else
+			pr_err("Fail to initialize for field key: '%s'", tok);
+
+		break;
+	}
+
+	return ret;
+}
+
+static int kvm_hpp_list__parse(struct perf_hpp_list *hpp_list,
+			       const char *output_, const char *sort_)
+{
+	char *output = output_ ? strdup(output_) : NULL;
+	char *sort = sort_ ? strdup(sort_) : NULL;
+	int ret;
+
+	ret = kvm_hpp_list__init(output, hpp_list, kvm_hists__init_output);
+	if (ret)
+		goto out;
+
+	ret = kvm_hpp_list__init(sort, hpp_list, kvm_hists__init_sort);
+	if (ret)
+		goto out;
+
+	/* Copy sort keys to output fields */
+	perf_hpp__setup_output_field(hpp_list);
+
+	/* and then copy output fields to sort keys */
+	perf_hpp__append_sort_keys(hpp_list);
+out:
+	free(output);
+	free(sort);
+	return ret;
+}
+
 static int kvm_hists__init(void)
 {
 	__hists__init(&kvm_hists.hists, &kvm_hists.list);
 	perf_hpp_list__init(&kvm_hists.list);
-	return 0;
+	return kvm_hpp_list__parse(&kvm_hists.list, NULL, "ev_name");
 }
 
 static const char *get_filename_for_perf_kvm(void)
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 0cf704333c4c..fc30a72dfac1 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -46,6 +46,8 @@ struct kvm_event {
 	#define DEFAULT_VCPU_NUM 8
 	int max_vcpu;
 	struct kvm_event_stats *vcpu;
+
+	struct hist_entry he;
 };
 
 typedef int64_t (*key_cmp_fun)(struct kvm_event*, struct kvm_event*, int);
-- 
2.34.1

[PATCH v5 11/16] perf kvm: Use histograms list to replace cached list

[Leo Yan]

perf kvm tool defines its own cached list which is managed with RB tree,
histograms also provide RB tree to manage data entries.  Since now we
have introduced histograms in the tool, it's not necessary to use the
self defined list and we can directly use histograms list to manage
KVM events.

This patch changes to use histograms list to track KVM events, and it
invokes the common function hists__output_resort_cb() to sort result,
this also give us flexibility to extend more sorting key words easily.

After histograms list supported, the cached list is redundant so remove
the relevant code for it.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-kvm.c   | 189 +++++++++++++++++++------------------
 tools/perf/util/kvm-stat.h |   7 --
 2 files changed, 95 insertions(+), 101 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 3f601ccb7aab..ba3134613bcb 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -323,6 +323,12 @@ static int kvm_hists__init(void)
 	return kvm_hpp_list__parse(&kvm_hists.list, NULL, "ev_name");
 }
 
+static int kvm_hists__reinit(const char *output, const char *sort)
+{
+	perf_hpp__reset_output_field(&kvm_hists.list);
+	return kvm_hpp_list__parse(&kvm_hists.list, output, sort);
+}
+
 static const char *get_filename_for_perf_kvm(void)
 {
 	const char *filename;
@@ -420,44 +426,37 @@ struct vcpu_event_record {
 	struct kvm_event *last_event;
 };
 
-
-static void init_kvm_event_record(struct perf_kvm_stat *kvm)
-{
-	unsigned int i;
-
-	for (i = 0; i < EVENTS_CACHE_SIZE; i++)
-		INIT_LIST_HEAD(&kvm->kvm_events_cache[i]);
-}
-
 #ifdef HAVE_TIMERFD_SUPPORT
-static void clear_events_cache_stats(struct list_head *kvm_events_cache)
+static void clear_events_cache_stats(void)
 {
-	struct list_head *head;
+	struct rb_root_cached *root;
+	struct rb_node *nd;
 	struct kvm_event *event;
-	unsigned int i;
-	int j;
-
-	for (i = 0; i < EVENTS_CACHE_SIZE; i++) {
-		head = &kvm_events_cache[i];
-		list_for_each_entry(event, head, hash_entry) {
-			/* reset stats for event */
-			event->total.time = 0;
-			init_stats(&event->total.stats);
-
-			for (j = 0; j < event->max_vcpu; ++j) {
-				event->vcpu[j].time = 0;
-				init_stats(&event->vcpu[j].stats);
-			}
+	int i;
+
+	if (hists__has(&kvm_hists.hists, need_collapse))
+		root = &kvm_hists.hists.entries_collapsed;
+	else
+		root = kvm_hists.hists.entries_in;
+
+	for (nd = rb_first_cached(root); nd; nd = rb_next(nd)) {
+		struct hist_entry *he;
+
+		he = rb_entry(nd, struct hist_entry, rb_node_in);
+		event = container_of(he, struct kvm_event, he);
+
+		/* reset stats for event */
+		event->total.time = 0;
+		init_stats(&event->total.stats);
+
+		for (i = 0; i < event->max_vcpu; ++i) {
+			event->vcpu[i].time = 0;
+			init_stats(&event->vcpu[i].stats);
 		}
 	}
 }
 #endif
 
-static int kvm_events_hash_fn(u64 key)
-{
-	return key & (EVENTS_CACHE_SIZE - 1);
-}
-
 static bool kvm_event_expand(struct kvm_event *event, int vcpu_id)
 {
 	int old_max_vcpu = event->max_vcpu;
@@ -483,44 +482,64 @@ static bool kvm_event_expand(struct kvm_event *event, int vcpu_id)
 	return true;
 }
 
-static struct kvm_event *kvm_alloc_init_event(struct perf_kvm_stat *kvm,
-					      struct event_key *key,
-					      struct perf_sample *sample __maybe_unused)
+static void *kvm_he_zalloc(size_t size)
 {
-	struct kvm_event *event;
+	struct kvm_event *kvm_ev;
 
-	event = zalloc(sizeof(*event));
-	if (!event) {
-		pr_err("Not enough memory\n");
+	kvm_ev = zalloc(size + sizeof(*kvm_ev));
+	if (!kvm_ev)
 		return NULL;
-	}
 
-	event->perf_kvm = kvm;
-	event->key = *key;
-	init_stats(&event->total.stats);
-	return event;
+	init_stats(&kvm_ev->total.stats);
+	hists__inc_nr_samples(&kvm_hists.hists, 0);
+	return &kvm_ev->he;
+}
+
+static void kvm_he_free(void *he)
+{
+	struct kvm_event *kvm_ev;
+
+	free(((struct hist_entry *)he)->kvm_info);
+	kvm_ev = container_of(he, struct kvm_event, he);
+	free(kvm_ev);
 }
 
+static struct hist_entry_ops kvm_ev_entry_ops = {
+	.new	= kvm_he_zalloc,
+	.free	= kvm_he_free,
+};
+
 static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
 					       struct event_key *key,
 					       struct perf_sample *sample)
 {
 	struct kvm_event *event;
-	struct list_head *head;
+	struct hist_entry *he;
+	struct kvm_info *ki;
 
 	BUG_ON(key->key == INVALID_KEY);
 
-	head = &kvm->kvm_events_cache[kvm_events_hash_fn(key->key)];
-	list_for_each_entry(event, head, hash_entry) {
-		if (event->key.key == key->key && event->key.info == key->info)
-			return event;
+	ki = zalloc(sizeof(*ki));
+	if (!ki) {
+		pr_err("Failed to allocate kvm info\n");
+		return NULL;
 	}
 
-	event = kvm_alloc_init_event(kvm, key, sample);
-	if (!event)
+	kvm->events_ops->decode_key(kvm, key, ki->name);
+	he = hists__add_entry_ops(&kvm_hists.hists, &kvm_ev_entry_ops,
+				  &kvm->al, NULL, NULL, NULL, ki, sample, true);
+	if (he == NULL) {
+		pr_err("Failed to allocate hist entry\n");
+		free(ki);
 		return NULL;
+	}
+
+	event = container_of(he, struct kvm_event, he);
+	if (!event->perf_kvm) {
+		event->perf_kvm = kvm;
+		event->key = *key;
+	}
 
-	list_add(&event->hash_entry, head);
 	return event;
 }
 
@@ -753,58 +772,32 @@ static bool select_key(struct perf_kvm_stat *kvm)
 	return false;
 }
 
-static void insert_to_result(struct rb_root *result, struct kvm_event *event,
-			     key_cmp_fun bigger, int vcpu)
-{
-	struct rb_node **rb = &result->rb_node;
-	struct rb_node *parent = NULL;
-	struct kvm_event *p;
-
-	while (*rb) {
-		p = container_of(*rb, struct kvm_event, rb);
-		parent = *rb;
-
-		if (bigger(event, p, vcpu) > 0)
-			rb = &(*rb)->rb_left;
-		else
-			rb = &(*rb)->rb_right;
-	}
-
-	rb_link_node(&event->rb, parent, rb);
-	rb_insert_color(&event->rb, result);
-}
-
 static bool event_is_valid(struct kvm_event *event, int vcpu)
 {
 	return !!get_event_count(event, vcpu);
 }
 
-static void sort_result(struct perf_kvm_stat *kvm)
+static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
 {
-	unsigned int i;
-	int vcpu = kvm->trace_vcpu;
 	struct kvm_event *event;
+	struct perf_kvm_stat *perf_kvm;
 
-	for (i = 0; i < EVENTS_CACHE_SIZE; i++) {
-		list_for_each_entry(event, &kvm->kvm_events_cache[i], hash_entry) {
-			if (event_is_valid(event, vcpu)) {
-				insert_to_result(&kvm->result, event,
-						 kvm->compare, vcpu);
-			}
-		}
-	}
+	event = container_of(he, struct kvm_event, he);
+	perf_kvm = event->perf_kvm;
+	if (!event_is_valid(event, perf_kvm->trace_vcpu))
+		he->filtered = 1;
+	else
+		he->filtered = 0;
+	return 0;
 }
 
-/* returns left most element of result, and erase it */
-static struct kvm_event *pop_from_result(struct rb_root *result)
+static void sort_result(struct perf_kvm_stat *kvm)
 {
-	struct rb_node *node = rb_first(result);
-
-	if (!node)
-		return NULL;
+	const char *output_columns = "ev_name,sample,time,max_t,min_t,mean_t";
 
-	rb_erase(node, result);
-	return container_of(node, struct kvm_event, rb);
+	kvm_hists__reinit(output_columns, kvm->sort_key);
+	hists__collapse_resort(&kvm_hists.hists, NULL);
+	hists__output_resort_cb(&kvm_hists.hists, NULL, filter_cb);
 }
 
 static void print_vcpu_info(struct perf_kvm_stat *kvm)
@@ -847,6 +840,7 @@ static void print_result(struct perf_kvm_stat *kvm)
 	char decode[KVM_EVENT_NAME_LEN];
 	struct kvm_event *event;
 	int vcpu = kvm->trace_vcpu;
+	struct rb_node *nd;
 
 	if (kvm->live) {
 		puts(CONSOLE_CLEAR);
@@ -865,9 +859,15 @@ static void print_result(struct perf_kvm_stat *kvm)
 	pr_info("%16s ", "Avg time");
 	pr_info("\n\n");
 
-	while ((event = pop_from_result(&kvm->result))) {
+	for (nd = rb_first_cached(&kvm_hists.hists.entries); nd; nd = rb_next(nd)) {
+		struct hist_entry *he;
 		u64 ecount, etime, max, min;
 
+		he = rb_entry(nd, struct hist_entry, rb_node);
+		if (he->filtered)
+			continue;
+
+		event = container_of(he, struct kvm_event, he);
 		ecount = get_event_count(event, vcpu);
 		etime = get_event_time(event, vcpu);
 		max = get_event_max(event, vcpu);
@@ -1144,8 +1144,11 @@ static int perf_kvm__handle_timerfd(struct perf_kvm_stat *kvm)
 	sort_result(kvm);
 	print_result(kvm);
 
+	/* Reset sort list to "ev_name" */
+	kvm_hists__reinit(NULL, "ev_name");
+
 	/* reset counts */
-	clear_events_cache_stats(kvm->kvm_events_cache);
+	clear_events_cache_stats();
 	kvm->total_count = 0;
 	kvm->total_time = 0;
 	kvm->lost_events = 0;
@@ -1201,7 +1204,6 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 	}
 
 	set_term_quiet_input(&save);
-	init_kvm_event_record(kvm);
 
 	kvm_hists__init();
 
@@ -1397,7 +1399,6 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 	if (!register_kvm_events_ops(kvm))
 		goto exit;
 
-	init_kvm_event_record(kvm);
 	setup_pager();
 
 	kvm_hists__init();
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index fc30a72dfac1..3f0cbecb862c 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -36,7 +36,6 @@ struct perf_kvm_stat;
 
 struct kvm_event {
 	struct list_head hash_entry;
-	struct rb_node rb;
 
 	struct perf_kvm_stat *perf_kvm;
 	struct event_key key;
@@ -81,9 +80,6 @@ struct exit_reasons_table {
 	const char *reason;
 };
 
-#define EVENTS_BITS		12
-#define EVENTS_CACHE_SIZE	(1UL << EVENTS_BITS)
-
 struct perf_kvm_stat {
 	struct perf_tool    tool;
 	struct record_opts  opts;
@@ -103,7 +99,6 @@ struct perf_kvm_stat {
 
 	struct kvm_events_ops *events_ops;
 	key_cmp_fun compare;
-	struct list_head kvm_events_cache[EVENTS_CACHE_SIZE];
 
 	u64 total_time;
 	u64 total_count;
@@ -112,8 +107,6 @@ struct perf_kvm_stat {
 
 	struct intlist *pid_list;
 
-	struct rb_root result;
-
 	int timerfd;
 	unsigned int display_time;
 	bool live;
-- 
2.34.1

Re: [PATCH v5 11/16] perf kvm: Use histograms list to replace cached list

[Namhyung Kim]

On Wed, Mar 15, 2023 at 7:52 AM Leo Yan <leo.yan@linaro.org> wrote:
>
> perf kvm tool defines its own cached list which is managed with RB tree,
> histograms also provide RB tree to manage data entries.  Since now we
> have introduced histograms in the tool, it's not necessary to use the
> self defined list and we can directly use histograms list to manage
> KVM events.
>
> This patch changes to use histograms list to track KVM events, and it
> invokes the common function hists__output_resort_cb() to sort result,
> this also give us flexibility to extend more sorting key words easily.
>
> After histograms list supported, the cached list is redundant so remove
> the relevant code for it.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/builtin-kvm.c   | 189 +++++++++++++++++++------------------
>  tools/perf/util/kvm-stat.h |   7 --
>  2 files changed, 95 insertions(+), 101 deletions(-)
>
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 3f601ccb7aab..ba3134613bcb 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -323,6 +323,12 @@ static int kvm_hists__init(void)
>         return kvm_hpp_list__parse(&kvm_hists.list, NULL, "ev_name");
>  }
>
> +static int kvm_hists__reinit(const char *output, const char *sort)
> +{
> +       perf_hpp__reset_output_field(&kvm_hists.list);
> +       return kvm_hpp_list__parse(&kvm_hists.list, output, sort);
> +}
> +
>  static const char *get_filename_for_perf_kvm(void)
>  {
>         const char *filename;
> @@ -420,44 +426,37 @@ struct vcpu_event_record {
>         struct kvm_event *last_event;
>  };
>
> -
> -static void init_kvm_event_record(struct perf_kvm_stat *kvm)
> -{
> -       unsigned int i;
> -
> -       for (i = 0; i < EVENTS_CACHE_SIZE; i++)
> -               INIT_LIST_HEAD(&kvm->kvm_events_cache[i]);
> -}
> -
>  #ifdef HAVE_TIMERFD_SUPPORT
> -static void clear_events_cache_stats(struct list_head *kvm_events_cache)
> +static void clear_events_cache_stats(void)
>  {
> -       struct list_head *head;
> +       struct rb_root_cached *root;
> +       struct rb_node *nd;
>         struct kvm_event *event;
> -       unsigned int i;
> -       int j;
> -
> -       for (i = 0; i < EVENTS_CACHE_SIZE; i++) {
> -               head = &kvm_events_cache[i];
> -               list_for_each_entry(event, head, hash_entry) {
> -                       /* reset stats for event */
> -                       event->total.time = 0;
> -                       init_stats(&event->total.stats);
> -
> -                       for (j = 0; j < event->max_vcpu; ++j) {
> -                               event->vcpu[j].time = 0;
> -                               init_stats(&event->vcpu[j].stats);
> -                       }
> +       int i;
> +
> +       if (hists__has(&kvm_hists.hists, need_collapse))
> +               root = &kvm_hists.hists.entries_collapsed;
> +       else
> +               root = kvm_hists.hists.entries_in;
> +
> +       for (nd = rb_first_cached(root); nd; nd = rb_next(nd)) {
> +               struct hist_entry *he;
> +
> +               he = rb_entry(nd, struct hist_entry, rb_node_in);
> +               event = container_of(he, struct kvm_event, he);
> +
> +               /* reset stats for event */
> +               event->total.time = 0;
> +               init_stats(&event->total.stats);
> +
> +               for (i = 0; i < event->max_vcpu; ++i) {
> +                       event->vcpu[i].time = 0;
> +                       init_stats(&event->vcpu[i].stats);
>                 }
>         }
>  }
>  #endif
>
> -static int kvm_events_hash_fn(u64 key)
> -{
> -       return key & (EVENTS_CACHE_SIZE - 1);
> -}
> -
>  static bool kvm_event_expand(struct kvm_event *event, int vcpu_id)
>  {
>         int old_max_vcpu = event->max_vcpu;
> @@ -483,44 +482,64 @@ static bool kvm_event_expand(struct kvm_event *event, int vcpu_id)
>         return true;
>  }
>
> -static struct kvm_event *kvm_alloc_init_event(struct perf_kvm_stat *kvm,
> -                                             struct event_key *key,
> -                                             struct perf_sample *sample __maybe_unused)
> +static void *kvm_he_zalloc(size_t size)
>  {
> -       struct kvm_event *event;
> +       struct kvm_event *kvm_ev;
>
> -       event = zalloc(sizeof(*event));
> -       if (!event) {
> -               pr_err("Not enough memory\n");
> +       kvm_ev = zalloc(size + sizeof(*kvm_ev));
> +       if (!kvm_ev)
>                 return NULL;
> -       }
>
> -       event->perf_kvm = kvm;
> -       event->key = *key;
> -       init_stats(&event->total.stats);
> -       return event;
> +       init_stats(&kvm_ev->total.stats);
> +       hists__inc_nr_samples(&kvm_hists.hists, 0);
> +       return &kvm_ev->he;
> +}
> +
> +static void kvm_he_free(void *he)
> +{
> +       struct kvm_event *kvm_ev;
> +
> +       free(((struct hist_entry *)he)->kvm_info);
> +       kvm_ev = container_of(he, struct kvm_event, he);
> +       free(kvm_ev);
>  }
>
> +static struct hist_entry_ops kvm_ev_entry_ops = {
> +       .new    = kvm_he_zalloc,
> +       .free   = kvm_he_free,
> +};
> +
>  static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
>                                                struct event_key *key,
>                                                struct perf_sample *sample)
>  {
>         struct kvm_event *event;
> -       struct list_head *head;
> +       struct hist_entry *he;
> +       struct kvm_info *ki;
>
>         BUG_ON(key->key == INVALID_KEY);
>
> -       head = &kvm->kvm_events_cache[kvm_events_hash_fn(key->key)];
> -       list_for_each_entry(event, head, hash_entry) {
> -               if (event->key.key == key->key && event->key.info == key->info)
> -                       return event;
> +       ki = zalloc(sizeof(*ki));
> +       if (!ki) {
> +               pr_err("Failed to allocate kvm info\n");
> +               return NULL;
>         }
>
> -       event = kvm_alloc_init_event(kvm, key, sample);
> -       if (!event)
> +       kvm->events_ops->decode_key(kvm, key, ki->name);
> +       he = hists__add_entry_ops(&kvm_hists.hists, &kvm_ev_entry_ops,
> +                                 &kvm->al, NULL, NULL, NULL, ki, sample, true);

The hists__add_entry{,_ops} can return either a new entry
or an existing one.  I think it'd leak the 'ki' when it returns
the existing one.  You may deep-copy it in hist_entry__init()
and always free the 'ki' here.

Another thought on this.  Lots of fields in the hist_entry are
not used for kvm.  We might split the hist_entry somehow
so that we can use unnecessary parts only.  But that could
be a future project. :)

Thanks,
Namhyung


> +       if (he == NULL) {
> +               pr_err("Failed to allocate hist entry\n");
> +               free(ki);
>                 return NULL;
> +       }
> +
> +       event = container_of(he, struct kvm_event, he);
> +       if (!event->perf_kvm) {
> +               event->perf_kvm = kvm;
> +               event->key = *key;
> +       }
>
> -       list_add(&event->hash_entry, head);
>         return event;
>  }
>
> @@ -753,58 +772,32 @@ static bool select_key(struct perf_kvm_stat *kvm)
>         return false;
>  }
>
> -static void insert_to_result(struct rb_root *result, struct kvm_event *event,
> -                            key_cmp_fun bigger, int vcpu)
> -{
> -       struct rb_node **rb = &result->rb_node;
> -       struct rb_node *parent = NULL;
> -       struct kvm_event *p;
> -
> -       while (*rb) {
> -               p = container_of(*rb, struct kvm_event, rb);
> -               parent = *rb;
> -
> -               if (bigger(event, p, vcpu) > 0)
> -                       rb = &(*rb)->rb_left;
> -               else
> -                       rb = &(*rb)->rb_right;
> -       }
> -
> -       rb_link_node(&event->rb, parent, rb);
> -       rb_insert_color(&event->rb, result);
> -}
> -
>  static bool event_is_valid(struct kvm_event *event, int vcpu)
>  {
>         return !!get_event_count(event, vcpu);
>  }
>
> -static void sort_result(struct perf_kvm_stat *kvm)
> +static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
>  {
> -       unsigned int i;
> -       int vcpu = kvm->trace_vcpu;
>         struct kvm_event *event;
> +       struct perf_kvm_stat *perf_kvm;
>
> -       for (i = 0; i < EVENTS_CACHE_SIZE; i++) {
> -               list_for_each_entry(event, &kvm->kvm_events_cache[i], hash_entry) {
> -                       if (event_is_valid(event, vcpu)) {
> -                               insert_to_result(&kvm->result, event,
> -                                                kvm->compare, vcpu);
> -                       }
> -               }
> -       }
> +       event = container_of(he, struct kvm_event, he);
> +       perf_kvm = event->perf_kvm;
> +       if (!event_is_valid(event, perf_kvm->trace_vcpu))
> +               he->filtered = 1;
> +       else
> +               he->filtered = 0;
> +       return 0;
>  }
>
> -/* returns left most element of result, and erase it */
> -static struct kvm_event *pop_from_result(struct rb_root *result)
> +static void sort_result(struct perf_kvm_stat *kvm)
>  {
> -       struct rb_node *node = rb_first(result);
> -
> -       if (!node)
> -               return NULL;
> +       const char *output_columns = "ev_name,sample,time,max_t,min_t,mean_t";
>
> -       rb_erase(node, result);
> -       return container_of(node, struct kvm_event, rb);
> +       kvm_hists__reinit(output_columns, kvm->sort_key);
> +       hists__collapse_resort(&kvm_hists.hists, NULL);
> +       hists__output_resort_cb(&kvm_hists.hists, NULL, filter_cb);
>  }
>
>  static void print_vcpu_info(struct perf_kvm_stat *kvm)
> @@ -847,6 +840,7 @@ static void print_result(struct perf_kvm_stat *kvm)
>         char decode[KVM_EVENT_NAME_LEN];
>         struct kvm_event *event;
>         int vcpu = kvm->trace_vcpu;
> +       struct rb_node *nd;
>
>         if (kvm->live) {
>                 puts(CONSOLE_CLEAR);
> @@ -865,9 +859,15 @@ static void print_result(struct perf_kvm_stat *kvm)
>         pr_info("%16s ", "Avg time");
>         pr_info("\n\n");
>
> -       while ((event = pop_from_result(&kvm->result))) {
> +       for (nd = rb_first_cached(&kvm_hists.hists.entries); nd; nd = rb_next(nd)) {
> +               struct hist_entry *he;
>                 u64 ecount, etime, max, min;
>
> +               he = rb_entry(nd, struct hist_entry, rb_node);
> +               if (he->filtered)
> +                       continue;
> +
> +               event = container_of(he, struct kvm_event, he);
>                 ecount = get_event_count(event, vcpu);
>                 etime = get_event_time(event, vcpu);
>                 max = get_event_max(event, vcpu);
> @@ -1144,8 +1144,11 @@ static int perf_kvm__handle_timerfd(struct perf_kvm_stat *kvm)
>         sort_result(kvm);
>         print_result(kvm);
>
> +       /* Reset sort list to "ev_name" */
> +       kvm_hists__reinit(NULL, "ev_name");
> +
>         /* reset counts */
> -       clear_events_cache_stats(kvm->kvm_events_cache);
> +       clear_events_cache_stats();
>         kvm->total_count = 0;
>         kvm->total_time = 0;
>         kvm->lost_events = 0;
> @@ -1201,7 +1204,6 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
>         }
>
>         set_term_quiet_input(&save);
> -       init_kvm_event_record(kvm);
>
>         kvm_hists__init();
>
> @@ -1397,7 +1399,6 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
>         if (!register_kvm_events_ops(kvm))
>                 goto exit;
>
> -       init_kvm_event_record(kvm);
>         setup_pager();
>
>         kvm_hists__init();
> diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
> index fc30a72dfac1..3f0cbecb862c 100644
> --- a/tools/perf/util/kvm-stat.h
> +++ b/tools/perf/util/kvm-stat.h
> @@ -36,7 +36,6 @@ struct perf_kvm_stat;
>
>  struct kvm_event {
>         struct list_head hash_entry;
> -       struct rb_node rb;
>
>         struct perf_kvm_stat *perf_kvm;
>         struct event_key key;
> @@ -81,9 +80,6 @@ struct exit_reasons_table {
>         const char *reason;
>  };
>
> -#define EVENTS_BITS            12
> -#define EVENTS_CACHE_SIZE      (1UL << EVENTS_BITS)
> -
>  struct perf_kvm_stat {
>         struct perf_tool    tool;
>         struct record_opts  opts;
> @@ -103,7 +99,6 @@ struct perf_kvm_stat {
>
>         struct kvm_events_ops *events_ops;
>         key_cmp_fun compare;
> -       struct list_head kvm_events_cache[EVENTS_CACHE_SIZE];
>
>         u64 total_time;
>         u64 total_count;
> @@ -112,8 +107,6 @@ struct perf_kvm_stat {
>
>         struct intlist *pid_list;
>
> -       struct rb_root result;
> -
>         int timerfd;
>         unsigned int display_time;
>         bool live;
> --
> 2.34.1
>

Re: [PATCH v5 11/16] perf kvm: Use histograms list to replace cached list

[Leo Yan]

On Thu, Mar 16, 2023 at 12:42:53AM -0700, Namhyung Kim wrote:

[...]

> >  static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
> >                                                struct event_key *key,
> >                                                struct perf_sample *sample)
> >  {
> >         struct kvm_event *event;
> > -       struct list_head *head;
> > +       struct hist_entry *he;
> > +       struct kvm_info *ki;
> >
> >         BUG_ON(key->key == INVALID_KEY);
> >
> > -       head = &kvm->kvm_events_cache[kvm_events_hash_fn(key->key)];
> > -       list_for_each_entry(event, head, hash_entry) {
> > -               if (event->key.key == key->key && event->key.info == key->info)
> > -                       return event;
> > +       ki = zalloc(sizeof(*ki));
> > +       if (!ki) {
> > +               pr_err("Failed to allocate kvm info\n");
> > +               return NULL;
> >         }
> >
> > -       event = kvm_alloc_init_event(kvm, key, sample);
> > -       if (!event)
> > +       kvm->events_ops->decode_key(kvm, key, ki->name);
> > +       he = hists__add_entry_ops(&kvm_hists.hists, &kvm_ev_entry_ops,
> > +                                 &kvm->al, NULL, NULL, NULL, ki, sample, true);
> 
> The hists__add_entry{,_ops} can return either a new entry
> or an existing one.  I think it'd leak the 'ki' when it returns
> the existing one.  You may deep-copy it in hist_entry__init()
> and always free the 'ki' here.

Thanks for pointing out this, Namhyung.  I will fix it.

@Arnaldo, do you want me to send an appending patch, or will you drop
this patch series from your branch so I send a new patch set?

> Another thought on this.  Lots of fields in the hist_entry are
> not used for kvm.  We might split the hist_entry somehow
> so that we can use unnecessary parts only.  But that could
> be a future project. :)

Yeah, I found now hist_entry contains many fields
(branch_info/mem_info/kvm_info/block_info); we can consider to
refactor the struct hist_entry to use an abstract pointer to refer
tool's specific data, this could be easily extend hist_entry to
support more tools.

Thanks,
Leo

Re: [PATCH v5 11/16] perf kvm: Use histograms list to replace cached list

[Arnaldo Carvalho de Melo]

Em Thu, Mar 16, 2023 at 05:04:18PM +0800, Leo Yan escreveu:
> On Thu, Mar 16, 2023 at 12:42:53AM -0700, Namhyung Kim wrote:
> 
> [...]
> 
> > >  static struct kvm_event *find_create_kvm_event(struct perf_kvm_stat *kvm,
> > >                                                struct event_key *key,
> > >                                                struct perf_sample *sample)
> > >  {
> > >         struct kvm_event *event;
> > > -       struct list_head *head;
> > > +       struct hist_entry *he;
> > > +       struct kvm_info *ki;
> > >
> > >         BUG_ON(key->key == INVALID_KEY);
> > >
> > > -       head = &kvm->kvm_events_cache[kvm_events_hash_fn(key->key)];
> > > -       list_for_each_entry(event, head, hash_entry) {
> > > -               if (event->key.key == key->key && event->key.info == key->info)
> > > -                       return event;
> > > +       ki = zalloc(sizeof(*ki));
> > > +       if (!ki) {
> > > +               pr_err("Failed to allocate kvm info\n");
> > > +               return NULL;
> > >         }
> > >
> > > -       event = kvm_alloc_init_event(kvm, key, sample);
> > > -       if (!event)
> > > +       kvm->events_ops->decode_key(kvm, key, ki->name);
> > > +       he = hists__add_entry_ops(&kvm_hists.hists, &kvm_ev_entry_ops,
> > > +                                 &kvm->al, NULL, NULL, NULL, ki, sample, true);
> > 
> > The hists__add_entry{,_ops} can return either a new entry
> > or an existing one.  I think it'd leak the 'ki' when it returns
> > the existing one.  You may deep-copy it in hist_entry__init()
> > and always free the 'ki' here.
> 
> Thanks for pointing out this, Namhyung.  I will fix it.
> 
> @Arnaldo, do you want me to send an appending patch, or will you drop
> this patch series from your branch so I send a new patch set?
> 
> > Another thought on this.  Lots of fields in the hist_entry are
> > not used for kvm.  We might split the hist_entry somehow
> > so that we can use unnecessary parts only.  But that could
> > be a future project. :)
> 
> Yeah, I found now hist_entry contains many fields
> (branch_info/mem_info/kvm_info/block_info); we can consider to
> refactor the struct hist_entry to use an abstract pointer to refer
> tool's specific data, this could be easily extend hist_entry to
> support more tools.

Since I build tested this already and had the other fixes, I'm pushing
this out to perf-tools-next (and perf/core for a while, for people not
knowing about the new nbranch names) and you can continue from there,
ok?

- Arnaldo

Re: [PATCH v5 11/16] perf kvm: Use histograms list to replace cached list

[Leo Yan]

On Thu, Mar 16, 2023 at 07:13:02AM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> Since I build tested this already and had the other fixes, I'm pushing
> this out to perf-tools-next (and perf/core for a while, for people not
> knowing about the new nbranch names) and you can continue from there,
> ok?

Sure, I will send a fixing patch.

Thanks,
Leo

[PATCH v5 12/16] perf kvm: Polish sorting key

[Leo Yan]

Since histograms supports sorting, the tool doesn't need to maintain the
mapping between the sorting keys and the corresponding comparison
callbacks, therefore, this patch removes structure kvm_event_key.

But we still need to validate the sorting key, this patch uses an array
for sorting keys and renames function select_key() to is_valid_key()
to validate the sorting key passed by user.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-kvm.c   | 29 ++++++++++-------------------
 tools/perf/util/kvm-stat.h |  8 --------
 2 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index ba3134613bcb..1e9338855239 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -76,15 +76,6 @@ COMPARE_EVENT_KEY(min, stats.min);
 COMPARE_EVENT_KEY(count, stats.n);
 COMPARE_EVENT_KEY(mean, stats.mean);
 
-#define DEF_SORT_NAME_KEY(name, compare_key)				\
-	{ #name, cmp_event_ ## compare_key }
-
-static struct kvm_event_key keys[] = {
-	DEF_SORT_NAME_KEY(sample, count),
-	DEF_SORT_NAME_KEY(time, mean),
-	{ NULL, NULL }
-};
-
 struct kvm_hists {
 	struct hists		hists;
 	struct perf_hpp_list	list;
@@ -757,18 +748,18 @@ static bool handle_kvm_event(struct perf_kvm_stat *kvm,
 	return true;
 }
 
-static bool select_key(struct perf_kvm_stat *kvm)
+static bool is_valid_key(struct perf_kvm_stat *kvm)
 {
-	int i;
+	static const char *key_array[] = {
+		"ev_name", "sample", "time", "max_t", "min_t", "mean_t",
+	};
+	unsigned int i;
 
-	for (i = 0; keys[i].name; i++) {
-		if (!strcmp(keys[i].name, kvm->sort_key)) {
-			kvm->compare = keys[i].key;
+	for (i = 0; i < ARRAY_SIZE(key_array); i++)
+		if (!strcmp(key_array[i], kvm->sort_key))
 			return true;
-		}
-	}
 
-	pr_err("Unknown compare key:%s\n", kvm->sort_key);
+	pr_err("Unsupported sort key: %s\n", kvm->sort_key);
 	return false;
 }
 
@@ -1198,7 +1189,7 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 		return ret;
 
 	if (!verify_vcpu(kvm->trace_vcpu) ||
-	    !select_key(kvm) ||
+	    !is_valid_key(kvm) ||
 	    !register_kvm_events_ops(kvm)) {
 		goto out;
 	}
@@ -1393,7 +1384,7 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 	if (!verify_vcpu(vcpu))
 		goto exit;
 
-	if (!select_key(kvm))
+	if (!is_valid_key(kvm))
 		goto exit;
 
 	if (!register_kvm_events_ops(kvm))
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 3f0cbecb862c..35d03894fac3 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -49,13 +49,6 @@ struct kvm_event {
 	struct hist_entry he;
 };
 
-typedef int64_t (*key_cmp_fun)(struct kvm_event*, struct kvm_event*, int);
-
-struct kvm_event_key {
-	const char *name;
-	key_cmp_fun key;
-};
-
 struct child_event_ops {
 	void (*get_key)(struct evsel *evsel,
 			struct perf_sample *sample,
@@ -98,7 +91,6 @@ struct perf_kvm_stat {
 	const char *exit_reasons_isa;
 
 	struct kvm_events_ops *events_ops;
-	key_cmp_fun compare;
 
 	u64 total_time;
 	u64 total_count;
-- 
2.34.1

[PATCH v5 13/16] perf kvm: Support printing attributions for dimensions

[Leo Yan]

This patch adds header, entry callback and width for every dimension,
thus in TUI mode the tool can print items with the defined attributions.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c | 97 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 1e9338855239..0d857df826cf 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -83,8 +83,12 @@ struct kvm_hists {
 
 struct kvm_dimension {
 	const char *name;
+	const char *header;
+	int width;
 	int64_t (*cmp)(struct perf_hpp_fmt *fmt, struct hist_entry *left,
 		       struct hist_entry *right);
+	int (*entry)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		     struct hist_entry *he);
 };
 
 struct kvm_fmt {
@@ -102,9 +106,24 @@ static int64_t ev_name_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
 	return -strcmp(left->kvm_info->name, right->kvm_info->name);
 }
 
+static int fmt_width(struct perf_hpp_fmt *fmt,
+		     struct perf_hpp *hpp __maybe_unused,
+		     struct hists *hists __maybe_unused);
+
+static int ev_name_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+			 struct hist_entry *he)
+{
+	int width = fmt_width(fmt, hpp, he->hists);
+
+	return scnprintf(hpp->buf, hpp->size, "%*s", width, he->kvm_info->name);
+}
+
 static struct kvm_dimension dim_event = {
+	.header		= "Event name",
 	.name		= "ev_name",
 	.cmp		= ev_name_cmp,
+	.entry		= ev_name_entry,
+	.width		= 40,
 };
 
 #define EV_METRIC_CMP(metric)						\
@@ -130,29 +149,77 @@ EV_METRIC_CMP(max)
 EV_METRIC_CMP(min)
 EV_METRIC_CMP(mean)
 
+#define EV_METRIC_ENTRY(metric)						\
+static int ev_entry_##metric(struct perf_hpp_fmt *fmt,			\
+			     struct perf_hpp *hpp,			\
+			     struct hist_entry *he)			\
+{									\
+	struct kvm_event *event;					\
+	int width = fmt_width(fmt, hpp, he->hists);			\
+	struct perf_kvm_stat *perf_kvm;					\
+									\
+	event = container_of(he, struct kvm_event, he);			\
+	perf_kvm = event->perf_kvm;					\
+	return scnprintf(hpp->buf, hpp->size, "%*lu", width,		\
+		get_event_##metric(event, perf_kvm->trace_vcpu));	\
+}
+
+EV_METRIC_ENTRY(time)
+EV_METRIC_ENTRY(count)
+EV_METRIC_ENTRY(max)
+EV_METRIC_ENTRY(min)
+
 static struct kvm_dimension dim_time = {
+	.header		= "Time (ns)",
 	.name		= "time",
 	.cmp		= ev_cmp_time,
+	.entry		= ev_entry_time,
+	.width		= 12,
 };
 
 static struct kvm_dimension dim_count = {
+	.header		= "Samples",
 	.name		= "sample",
 	.cmp		= ev_cmp_count,
+	.entry		= ev_entry_count,
+	.width		= 12,
 };
 
 static struct kvm_dimension dim_max_time = {
+	.header		= "Max Time (ns)",
 	.name		= "max_t",
 	.cmp		= ev_cmp_max,
+	.entry		= ev_entry_max,
+	.width		= 14,
 };
 
 static struct kvm_dimension dim_min_time = {
+	.header		= "Min Time (ns)",
 	.name		= "min_t",
 	.cmp		= ev_cmp_min,
+	.entry		= ev_entry_min,
+	.width		= 14,
 };
 
+static int ev_entry_mean(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+			 struct hist_entry *he)
+{
+	struct kvm_event *event;
+	int width = fmt_width(fmt, hpp, he->hists);
+	struct perf_kvm_stat *perf_kvm;
+
+	event = container_of(he, struct kvm_event, he);
+	perf_kvm = event->perf_kvm;
+	return scnprintf(hpp->buf, hpp->size, "%*lu", width,
+			 get_event_mean(event, perf_kvm->trace_vcpu));
+}
+
 static struct kvm_dimension dim_mean_time = {
+	.header		= "Mean Time (ns)",
 	.name		= "mean_t",
 	.cmp		= ev_cmp_mean,
+	.entry		= ev_entry_mean,
+	.width		= 14,
 };
 
 static struct kvm_dimension *dimensions[] = {
@@ -165,6 +232,30 @@ static struct kvm_dimension *dimensions[] = {
 	NULL,
 };
 
+static int fmt_width(struct perf_hpp_fmt *fmt,
+		     struct perf_hpp *hpp __maybe_unused,
+		     struct hists *hists __maybe_unused)
+{
+	struct kvm_fmt *kvm_fmt;
+
+	kvm_fmt = container_of(fmt, struct kvm_fmt, fmt);
+	return kvm_fmt->dim->width;
+}
+
+static int fmt_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		      struct hists *hists, int line __maybe_unused,
+		      int *span __maybe_unused)
+{
+	struct kvm_fmt *kvm_fmt;
+	struct kvm_dimension *dim;
+	int width = fmt_width(fmt, hpp, hists);
+
+	kvm_fmt = container_of(fmt, struct kvm_fmt, fmt);
+	dim = kvm_fmt->dim;
+
+	return scnprintf(hpp->buf, hpp->size, "%*s", width, dim->header);
+}
+
 static bool fmt_equal(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
 {
 	struct kvm_fmt *kvm_fmt_a = container_of(a, struct kvm_fmt, fmt);
@@ -214,9 +305,9 @@ static struct kvm_fmt *get_format(const char *name)
 	fmt->cmp	= dim->cmp;
 	fmt->sort	= dim->cmp;
 	fmt->color	= NULL;
-	fmt->entry	= NULL;
-	fmt->header	= NULL;
-	fmt->width	= NULL;
+	fmt->entry	= dim->entry;
+	fmt->header	= fmt_header;
+	fmt->width	= fmt_width;
 	fmt->collapse	= dim->cmp;
 	fmt->equal	= fmt_equal;
 	fmt->free	= fmt_free;
-- 
2.34.1

[PATCH v5 14/16] perf kvm: Add dimensions for percentages

[Leo Yan]

Add dimensions for count and time percentages, it would be useful for
user to review percentage statistics.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/builtin-kvm.c | 98 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 0d857df826cf..ab1cd6b5e528 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -222,10 +222,105 @@ static struct kvm_dimension dim_mean_time = {
 	.width		= 14,
 };
 
+#define PERC_STR(__s, __v)				\
+({							\
+	scnprintf(__s, sizeof(__s), "%.2F%%", __v);	\
+	__s;						\
+})
+
+static double percent(u64 st, u64 tot)
+{
+	return tot ? 100. * (double) st / (double) tot : 0;
+}
+
+#define EV_METRIC_PERCENT(metric)					\
+static int ev_percent_##metric(struct hist_entry *he)			\
+{									\
+	struct kvm_event *event;					\
+	struct perf_kvm_stat *perf_kvm;					\
+									\
+	event = container_of(he, struct kvm_event, he);			\
+	perf_kvm = event->perf_kvm;					\
+									\
+	return percent(get_event_##metric(event, perf_kvm->trace_vcpu),	\
+		       perf_kvm->total_##metric);			\
+}
+
+EV_METRIC_PERCENT(time)
+EV_METRIC_PERCENT(count)
+
+static int ev_entry_time_precent(struct perf_hpp_fmt *fmt,
+				 struct perf_hpp *hpp,
+				 struct hist_entry *he)
+{
+	int width = fmt_width(fmt, hpp, he->hists);
+	double per;
+	char buf[10];
+
+	per = ev_percent_time(he);
+	return scnprintf(hpp->buf, hpp->size, "%*s", width, PERC_STR(buf, per));
+}
+
+static int64_t
+ev_cmp_time_precent(struct perf_hpp_fmt *fmt __maybe_unused,
+		    struct hist_entry *left, struct hist_entry *right)
+{
+	double per_left;
+	double per_right;
+
+	per_left  = ev_percent_time(left);
+	per_right = ev_percent_time(right);
+
+	return per_left - per_right;
+}
+
+static struct kvm_dimension dim_time_percent = {
+	.header		= "Time%",
+	.name		= "percent_time",
+	.cmp		= ev_cmp_time_precent,
+	.entry		= ev_entry_time_precent,
+	.width		= 12,
+};
+
+static int ev_entry_count_precent(struct perf_hpp_fmt *fmt,
+				  struct perf_hpp *hpp,
+				  struct hist_entry *he)
+{
+	int width = fmt_width(fmt, hpp, he->hists);
+	double per;
+	char buf[10];
+
+	per = ev_percent_count(he);
+	return scnprintf(hpp->buf, hpp->size, "%*s", width, PERC_STR(buf, per));
+}
+
+static int64_t
+ev_cmp_count_precent(struct perf_hpp_fmt *fmt __maybe_unused,
+		     struct hist_entry *left, struct hist_entry *right)
+{
+	double per_left;
+	double per_right;
+
+	per_left  = ev_percent_count(left);
+	per_right = ev_percent_count(right);
+
+	return per_left - per_right;
+}
+
+static struct kvm_dimension dim_count_percent = {
+	.header		= "Sample%",
+	.name		= "percent_sample",
+	.cmp		= ev_cmp_count_precent,
+	.entry		= ev_entry_count_precent,
+	.width		= 12,
+};
+
 static struct kvm_dimension *dimensions[] = {
 	&dim_event,
 	&dim_time,
+	&dim_time_percent,
 	&dim_count,
+	&dim_count_percent,
 	&dim_max_time,
 	&dim_min_time,
 	&dim_mean_time,
@@ -875,7 +970,8 @@ static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
 
 static void sort_result(struct perf_kvm_stat *kvm)
 {
-	const char *output_columns = "ev_name,sample,time,max_t,min_t,mean_t";
+	const char *output_columns = "ev_name,sample,percent_sample,"
+				     "time,percent_time,max_t,min_t,mean_t";
 
 	kvm_hists__reinit(output_columns, kvm->sort_key);
 	hists__collapse_resort(&kvm_hists.hists, NULL);
-- 
2.34.1

[PATCH v5 15/16] perf kvm: Add TUI mode for stat report

[Leo Yan]

Since we have supported histograms list and prepared the dimensions in
the tool, this patch adds TUI mode for stat report.  It also adds UI
progress for sorting for better user experience.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-kvm.c   | 109 ++++++++++++++++++++++++++++++++++++-
 tools/perf/util/kvm-stat.h |   1 +
 2 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index ab1cd6b5e528..3dd44763d1fc 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -23,6 +23,8 @@
 #include "util/data.h"
 #include "util/ordered-events.h"
 #include "util/kvm-stat.h"
+#include "ui/browsers/hists.h"
+#include "ui/progress.h"
 #include "ui/ui.h"
 #include "util/string2.h"
 
@@ -495,6 +497,7 @@ static int kvm_hpp_list__parse(struct perf_hpp_list *hpp_list,
 
 static int kvm_hists__init(void)
 {
+	kvm_hists.list.nr_header_lines = 1;
 	__hists__init(&kvm_hists.hists, &kvm_hists.list);
 	perf_hpp_list__init(&kvm_hists.list);
 	return kvm_hpp_list__parse(&kvm_hists.list, NULL, "ev_name");
@@ -506,6 +509,93 @@ static int kvm_hists__reinit(const char *output, const char *sort)
 	return kvm_hpp_list__parse(&kvm_hists.list, output, sort);
 }
 
+static void print_result(struct perf_kvm_stat *kvm);
+
+#ifdef HAVE_SLANG_SUPPORT
+static void kvm_browser__update_nr_entries(struct hist_browser *hb)
+{
+	struct rb_node *nd = rb_first_cached(&hb->hists->entries);
+	u64 nr_entries = 0;
+
+	for (; nd; nd = rb_next(nd)) {
+		struct hist_entry *he = rb_entry(nd, struct hist_entry,
+						 rb_node);
+
+		if (!he->filtered)
+			nr_entries++;
+	}
+
+	hb->nr_non_filtered_entries = nr_entries;
+}
+
+static int kvm_browser__title(struct hist_browser *browser,
+			      char *buf, size_t size)
+{
+	scnprintf(buf, size, "KVM event statistics (%lu entries)",
+		  browser->nr_non_filtered_entries);
+	return 0;
+}
+
+static struct hist_browser*
+perf_kvm_browser__new(struct hists *hists)
+{
+	struct hist_browser *browser = hist_browser__new(hists);
+
+	if (browser)
+		browser->title = kvm_browser__title;
+
+	return browser;
+}
+
+static int kvm__hists_browse(struct hists *hists)
+{
+	struct hist_browser *browser;
+	int key = -1;
+
+	browser = perf_kvm_browser__new(hists);
+	if (browser == NULL)
+		return -1;
+
+	/* reset abort key so that it can get Ctrl-C as a key */
+	SLang_reset_tty();
+	SLang_init_tty(0, 0, 0);
+
+	kvm_browser__update_nr_entries(browser);
+
+	while (1) {
+		key = hist_browser__run(browser, "? - help", true, 0);
+
+		switch (key) {
+		case 'q':
+			goto out;
+		default:
+			break;
+		}
+	}
+
+out:
+	hist_browser__delete(browser);
+	return 0;
+}
+
+static void kvm_display(struct perf_kvm_stat *kvm)
+{
+	if (!use_browser)
+		print_result(kvm);
+	else
+		kvm__hists_browse(&kvm_hists.hists);
+}
+
+#else
+
+static void kvm_display(struct perf_kvm_stat *kvm)
+{
+	use_browser = 0;
+	print_result(kvm);
+}
+
+#endif /* HAVE_SLANG_SUPPORT */
+
 static const char *get_filename_for_perf_kvm(void)
 {
 	const char *filename;
@@ -970,12 +1060,15 @@ static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
 
 static void sort_result(struct perf_kvm_stat *kvm)
 {
+	struct ui_progress prog;
 	const char *output_columns = "ev_name,sample,percent_sample,"
 				     "time,percent_time,max_t,min_t,mean_t";
 
 	kvm_hists__reinit(output_columns, kvm->sort_key);
+	ui_progress__init(&prog, kvm_hists.hists.nr_entries, "Sorting...");
 	hists__collapse_resort(&kvm_hists.hists, NULL);
 	hists__output_resort_cb(&kvm_hists.hists, NULL, filter_cb);
+	ui_progress__finish();
 }
 
 static void print_vcpu_info(struct perf_kvm_stat *kvm)
@@ -1577,7 +1670,14 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 	if (!register_kvm_events_ops(kvm))
 		goto exit;
 
-	setup_pager();
+	if (kvm->use_stdio) {
+		use_browser = 0;
+		setup_pager();
+	} else {
+		use_browser = 1;
+	}
+
+	setup_browser(false);
 
 	kvm_hists__init();
 
@@ -1586,7 +1686,7 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 		goto exit;
 
 	sort_result(kvm);
-	print_result(kvm);
+	kvm_display(kvm);
 
 exit:
 	return ret;
@@ -1693,6 +1793,7 @@ kvm_events_report(struct perf_kvm_stat *kvm, int argc, const char **argv)
 		OPT_STRING('p', "pid", &kvm->opts.target.pid, "pid",
 			   "analyze events only for given process id(s)"),
 		OPT_BOOLEAN('f', "force", &kvm->force, "don't complain, do it"),
+		OPT_BOOLEAN(0, "stdio", &kvm->use_stdio, "use the stdio interface"),
 		OPT_END()
 	};
 
@@ -1710,6 +1811,10 @@ kvm_events_report(struct perf_kvm_stat *kvm, int argc, const char **argv)
 					   kvm_events_report_options);
 	}
 
+#ifndef HAVE_SLANG_SUPPORT
+	kvm->use_stdio = true;
+#endif
+
 	if (!kvm->opts.target.pid)
 		kvm->opts.target.system_wide = true;
 
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 35d03894fac3..bc6c8e38ef50 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -103,6 +103,7 @@ struct perf_kvm_stat {
 	unsigned int display_time;
 	bool live;
 	bool force;
+	bool use_stdio;
 };
 
 struct kvm_reg_events_ops {
-- 
2.34.1

[PATCH v5 16/16] perf kvm: Update documentation to reflect new changes

[Leo Yan]

Update documentation for new sorting and option '--stdio'.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/Documentation/perf-kvm.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-kvm.txt b/tools/perf/Documentation/perf-kvm.txt
index 2ad3f5d9f72b..b66be66fe836 100644
--- a/tools/perf/Documentation/perf-kvm.txt
+++ b/tools/perf/Documentation/perf-kvm.txt
@@ -58,7 +58,7 @@ There are a couple of variants of perf kvm:
   events.
 
   'perf kvm stat report' reports statistical data which includes events
-  handled time, samples, and so on.
+  handled sample, percent_sample, time, percent_time, max_t, min_t, mean_t.
 
   'perf kvm stat live' reports statistical data in a live mode (similar to
   record + report but with statistical data updated live at a given display
@@ -82,6 +82,8 @@ OPTIONS
 :GMEXAMPLESUBCMD: top
 include::guest-files.txt[]
 
+--stdio:: Use the stdio interface.
+
 -v::
 --verbose::
 	Be more verbose (show counter open errors, etc).
@@ -97,7 +99,10 @@ STAT REPORT OPTIONS
 -k::
 --key=<value>::
        Sorting key. Possible values: sample (default, sort by samples
-       number), time (sort by average time).
+       number), percent_sample (sort by sample percentage), time
+       (sort by average time), precent_time (sort by time percentage),
+       max_t (sort by maximum time), min_t (sort by minimum time), mean_t
+       (sort by mean time).
 -p::
 --pid=::
     Analyze events only for given process ID(s) (comma separated list).
-- 
2.34.1

Re: [PATCH v5 00/16] perf kvm: Support histograms and TUI mode

[Arnaldo Carvalho de Melo]

Em Wed, Mar 15, 2023 at 10:50:56PM +0800, Leo Yan escreveu:
> This patch set is to enable histograms and (partially) TUI mode in perf
> kvm tool; patch set v1 [1] gives brief introduction for the change.
> 
> We can use below commands for testing this series:
> 
> In a terminal, you could launch a virtual machine with qemu command; in
> below case, I downloaded a Ubuntu (or Debian) iso file and used it as
> the file system image:
> 
>   $ qemu-system-x86_64 -M pc -enable-kvm -cpu host -m 4096 -hda ubuntu-22.04-desktop-amd64.iso
> 
> Then in another terminal, I can use below command to capture KVM trace
> data and report the result:
> 
>   # cd linux/tools/perf
>   # ./perf kvm stat record
>   # ./perf kvm stat report          => Output in TUI mode
>   # ./perf kvm stat report --stdio  => Output in stdio mode
> 
> Changes from v4:
> * Fixed building failure in patch 04; compiled successfully for every
>   patch to ensure bisection.

Trying to fix this:

⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
cbd137d776f05b70 (HEAD -> perf-tools-next) perf kvm: Update documentation to reflect new changes
make: Entering directory '/var/home/acme/git/perf-tools-next/tools/perf'
- tarpkg: ./tests/perf-targz-src-pkg .
                 make_static: cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 NO_LIBTRACEEVENT=1 NO_LIBELF=1 -j32  DESTDIR=/tmp/tmp.AJidGMSQnF
cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 NO_LIBTRACEEVENT=1 NO_LIBELF=1 -j32 DESTDIR=/tmp/tmp.AJidGMSQnF
  BUILD:   Doing 'make -j32' parallel build
  HOSTCC  fixdep.o
  HOSTLD  fixdep-in.o
  LINK    fixdep
Makefile.config:678: Disabling post unwind, no support found.
Makefile.config:746: No libcrypto.h found, disables jitted code injection, please install openssl-devel or libssl-dev
Makefile.config:758: slang not found, disables TUI support. Please install slang-devel, libslang-dev or libslang2-dev
Makefile.config:805: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev
Makefile.config:845: No 'Python.h' was found: disables Python support - please install python-devel/python-dev
Makefile.config:950: No liblzma found, disables xz kernel module decompression, please install xz-devel/liblzma-dev
Makefile.config:963: No libzstd found, disables trace compression, please install libzstd-dev[el] and/or set LIBZSTD_DIR
Makefile.config:974: No libcap found, disables capability support, please install libcap-devel/libcap-dev
Makefile.config:987: No numa.h found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev
Makefile.config:1046: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev
Makefile.config:1137: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev

Auto-detecting system features:
...                                   dwarf: [ OFF ]
...                      dwarf_getlocations: [ OFF ]
...                                   glibc: [ on  ]
...                                  libbfd: [ OFF ]
...                          libbfd-buildid: [ OFF ]
...                                  libcap: [ OFF ]
...                                  libelf: [ OFF ]
...                                 libnuma: [ OFF ]
...                  numa_num_possible_cpus: [ OFF ]
...                                 libperl: [ OFF ]
...                               libpython: [ OFF ]
...                               libcrypto: [ OFF ]
...                               libunwind: [ OFF ]
...                      libdw-dwarf-unwind: [ OFF ]
...                                    zlib: [ OFF ]
...                                    lzma: [ OFF ]
...                               get_cpuid: [ on  ]
...                                     bpf: [ on  ]
...                                  libaio: [ on  ]
...                                 libzstd: [ OFF ]

  GEN     common-cmds.h
  CC      dlfilters/dlfilter-test-api-v0.o
  CC      dlfilters/dlfilter-show-cycles.o
  LINK    dlfilters/dlfilter-show-cycles.so
  LINK    dlfilters/dlfilter-test-api-v0.so
  INSTALL /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/include/subcmd/exec-cmd.h
  INSTALL /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/include/subcmd/help.h
  INSTALL /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/include/subcmd/pager.h
  INSTALL /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/include/subcmd/parse-options.h
  INSTALL /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/include/subcmd/run-command.h
  CC      /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/exec-cmd.o
  CC      /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/help.o
  CC      /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/pager.o
  <SNIP>
  CC      /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/parse-options.o
  CC      /var/home/acme/git/perf-tools-next/tools/perf/libsubcmd/run-command.o
  CC      util/sample-raw.o
  CC      util/s390-sample-raw.o
  CC      util/amd-sample-raw.o
  CC      util/ordered-events.o
builtin-kvm.c:512:13: error: ‘print_result’ used but never defined [-Werror]
  512 | static void print_result(struct perf_kvm_stat *kvm);
      |             ^~~~~~~~~~~~
builtin-kvm.c:591:13: error: ‘kvm_display’ defined but not used [-Werror=unused-function]
  591 | static void kvm_display(struct perf_kvm_stat *kvm)
      |             ^~~~~~~~~~~
builtin-kvm.c:506:12: error: ‘kvm_hists__reinit’ defined but not used [-Werror=unused-function]
  506 | static int kvm_hists__reinit(const char *output, const char *sort)
      |            ^~~~~~~~~~~~~~~~~
builtin-kvm.c:498:12: error: ‘kvm_hists__init’ defined but not used [-Werror=unused-function]
  498 | static int kvm_hists__init(void)
      |            ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[5]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: builtin-kvm.o] Error 1
make[5]: *** Waiting for unfinished jobs....
  CC      tests/perf-time-to-tsc.o
  CC      util/namespaces.o

Re: [PATCH v5 00/16] perf kvm: Support histograms and TUI mode

[Leo Yan]

On Wed, Mar 15, 2023 at 04:35:04PM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> Trying to fix this:
> 
> ⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test

I tried to run the build-test after I fetched the branch
acme/tmp.perf-tools-next and found errors; seems the errors are not
related with perf kvm but it's good to check a bit.

/usr/bin/ld: /usr/lib/llvm-15/lib/libclangSema.a(SemaRISCVVectorLookup.cpp.o): in function `clang::CreateRISCVIntrinsicManager(clang::Sema&)':
(.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x1b8): undefined reference to `clang::RISCV::RVVIntrinsic::computeBuiltinTypes(llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>, bool, bool, bool, unsigned int)'
/usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x1f9): undefined reference to `clang::RISCV::RVVIntrinsic::computeBuiltinTypes(llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>, bool, bool, bool, unsigned int)'
/usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x2cc): undefined reference to `clang::RISCV::RVVType::computeTypes(clang::RISCV::BasicType, int, unsigned int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
/usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x2ec): undefined reference to `clang::RISCV::RVVIntrinsic::getSuffixStr[abi:cxx11](clang::RISCV::BasicType, int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
/usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x307): undefined reference to `clang::RISCV::RVVIntrinsic::getSuffixStr[abi:cxx11](clang::RISCV::BasicType, int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
/usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x36c): undefined reference to `clang::RISCV::RVVType::computeTypes(clang::RISCV::BasicType, int, unsigned int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
collect2: error: ld returned 1 exit status

The complete building log is in the link:
https://termbin.com/jujt.

Later I think it would be a good habit to run build-test on my
patches locally :)

Thanks,
Leo

Re: [PATCH v5 00/16] perf kvm: Support histograms and TUI mode

[Arnaldo Carvalho de Melo]

Em Thu, Mar 16, 2023 at 11:10:06AM +0800, Leo Yan escreveu:
> On Wed, Mar 15, 2023 at 04:35:04PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> [...]
> 
> > Trying to fix this:
> > 
> > ⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
> 
> I tried to run the build-test after I fetched the branch
> acme/tmp.perf-tools-next and found errors; seems the errors are not
> related with perf kvm but it's good to check a bit.
> 
> /usr/bin/ld: /usr/lib/llvm-15/lib/libclangSema.a(SemaRISCVVectorLookup.cpp.o): in function `clang::CreateRISCVIntrinsicManager(clang::Sema&)':
> (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x1b8): undefined reference to `clang::RISCV::RVVIntrinsic::computeBuiltinTypes(llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>, bool, bool, bool, unsigned int)'
> /usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x1f9): undefined reference to `clang::RISCV::RVVIntrinsic::computeBuiltinTypes(llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>, bool, bool, bool, unsigned int)'
> /usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x2cc): undefined reference to `clang::RISCV::RVVType::computeTypes(clang::RISCV::BasicType, int, unsigned int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
> /usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x2ec): undefined reference to `clang::RISCV::RVVIntrinsic::getSuffixStr[abi:cxx11](clang::RISCV::BasicType, int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
> /usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x307): undefined reference to `clang::RISCV::RVVIntrinsic::getSuffixStr[abi:cxx11](clang::RISCV::BasicType, int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
> /usr/bin/ld: (.text._ZN5clang27CreateRISCVIntrinsicManagerERNS_4SemaE+0x36c): undefined reference to `clang::RISCV::RVVType::computeTypes(clang::RISCV::BasicType, int, unsigned int, llvm::ArrayRef<clang::RISCV::PrototypeDescriptor>)'
> collect2: error: ld returned 1 exit status

Nevermind this one, it is related to linking with clang libs, something
we'll deprecated.
 
> The complete building log is in the link:
> https://termbin.com/jujt.
> 
> Later I think it would be a good habit to run build-test on my
> patches locally :)

great!
 
> Thanks,
> Leo

-- 

- Arnaldo