On 4/21/23 14:24, Fei Wu wrote:
From: "Vanderson M. do Rosario" <vanderson...@gmail.com>

To store statistics for each TB, we created a TBStatistics structure
which is linked with the TBs. TBStatistics can stay alive after
tb_flush and be relinked to a regenerated TB. So the statistics can
be accumulated even through flushes.

The goal is to have all present and future qemu/tcg statistics and
meta-data stored in this new structure.

Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com>
Message-Id: <20190829173437.5926-2-vanderson...@gmail.com>
[AJB: fix git author, review comments]
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
---
  accel/tcg/meson.build     |  1 +
  accel/tcg/tb-context.h    |  1 +
  accel/tcg/tb-hash.h       |  7 +++++
  accel/tcg/tb-maint.c      | 19 ++++++++++++
  accel/tcg/tb-stats.c      | 58 +++++++++++++++++++++++++++++++++++++
  accel/tcg/translate-all.c | 43 +++++++++++++++++++++++++++
  include/exec/exec-all.h   |  3 ++
  include/exec/tb-stats.h   | 61 +++++++++++++++++++++++++++++++++++++++
  8 files changed, 193 insertions(+)
  create mode 100644 accel/tcg/tb-stats.c
  create mode 100644 include/exec/tb-stats.h

diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index aeb20a6ef0..9263bdde11 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -4,6 +4,7 @@ tcg_ss.add(files(
    'cpu-exec-common.c',
    'cpu-exec.c',
    'tb-maint.c',
+  'tb-stats.c',
    'tcg-runtime-gvec.c',
    'tcg-runtime.c',
    'translate-all.c',
diff --git a/accel/tcg/tb-context.h b/accel/tcg/tb-context.h
index cac62d9749..d7910d586b 100644
--- a/accel/tcg/tb-context.h
+++ b/accel/tcg/tb-context.h
@@ -35,6 +35,7 @@ struct TBContext {
      /* statistics */
      unsigned tb_flush_count;
      unsigned tb_phys_invalidate_count;
+    struct qht tb_stats;
  };
extern TBContext tb_ctx;
diff --git a/accel/tcg/tb-hash.h b/accel/tcg/tb-hash.h
index 83dc610e4c..87d657a1c6 100644
--- a/accel/tcg/tb-hash.h
+++ b/accel/tcg/tb-hash.h
@@ -67,4 +67,11 @@ uint32_t tb_hash_func(tb_page_addr_t phys_pc, target_ulong 
pc, uint32_t flags,
      return qemu_xxhash7(phys_pc, pc, flags, cf_mask, trace_vcpu_dstate);
  }
+static inline
+uint32_t tb_stats_hash_func(tb_page_addr_t phys_pc, target_ulong pc,
+                            uint32_t flags)
+{
+    return qemu_xxhash5(phys_pc, pc, flags);
+}
+

Why are you avoiding the hash of cs_base?
It certainly changes the comparison for a number of guests.

+/*
+ * This is the more or less the same compare as tb_cmp(), but the
+ * data persists over tb_flush. We also aggregate the various
+ * variations of cflags under one record and ignore the details of
+ * page overlap (although we can count it).
+ */
+bool tb_stats_cmp(const void *ap, const void *bp)
+{
+    const TBStatistics *a = ap;
+    const TBStatistics *b = bp;
+
+    return a->phys_pc == b->phys_pc &&
+        a->pc == b->pc &&
+        a->cs_base == b->cs_base &&
+        a->flags == b->flags;
+}

So, comparing cs_base, but not hashing it?

+void disable_collect_tb_stats(void)
+{
+    tcg_collect_tb_stats = TB_STATS_PAUSED;
+}
+
+void pause_collect_tb_stats(void)
+{
+    tcg_collect_tb_stats = TB_STATS_STOPPED;
+}

These two seem swapped.


r~

Reply via email to