On 5/30/23 01:35, Fei Wu wrote:
From: "Vanderson M. do Rosario" <vanderson...@gmail.com>
Adding tb_stats [start|pause|stop|filter] command to hmp.
This allows controlling the collection of statistics.
It is also possible to set the level of collection:
all, jit, or exec.
tb_stats filter allow to only collect statistics for the TB
in the last_search list.
The goal of this command is to allow the dynamic exploration
of the TCG behavior and quality. Therefore, for now, a
corresponding QMP command is not worthwhile.
Acked-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com>
Message-Id: <20190829173437.5926-8-vanderson...@gmail.com>
Message-Id: <20190829173437.5926-9-vanderson...@gmail.com>
[AJB: fix authorship]
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
Signed-off-by: Fei Wu <fei2...@intel.com>
---
I still don't see what pause does.
diff --git a/accel/tcg/tb-stats.c b/accel/tcg/tb-stats.c
index 68ac7d3f73..805e1fc74d 100644
--- a/accel/tcg/tb-stats.c
+++ b/accel/tcg/tb-stats.c
@@ -16,18 +16,20 @@
#include "qemu/timer.h"
#include "exec/tb-stats.h"
+#include "exec/tb-flush.h"
#include "tb-context.h"
/* TBStatistic collection controls */
enum TBStatsStatus {
TB_STATS_DISABLED = 0,
TB_STATS_RUNNING,
- TB_STATS_PAUSED,
- TB_STATS_STOPPED
+ TB_STATS_PAUSED
};
Why?
static enum TBStatsStatus tcg_collect_tb_stats;
static uint32_t default_tbstats_flag;
+/* only accessed in safe work */
+static GList *last_search;
uint64_t dev_time;
@@ -170,6 +172,101 @@ void dump_jit_profile_info(TCGProfile *s, GString *buf)
g_free(jpi);
}
+static void free_tbstats(void *p, uint32_t hash, void *userp)
+{
+ g_free(p);
+}
+
+static void clean_tbstats(void)
+{
+ /* remove all tb_stats */
+ qht_iter(&tb_ctx.tb_stats, free_tbstats, NULL);
+ qht_destroy(&tb_ctx.tb_stats);
+}
+
+void do_hmp_tbstats_safe(CPUState *cpu, run_on_cpu_data icmd)
+{
+ struct TbstatsCommand *cmdinfo = icmd.host_ptr;
+ int cmd = cmdinfo->cmd;
+ uint32_t level = cmdinfo->level;
+
+ switch (cmd) {
+ case START:
+ if (tb_stats_collection_enabled()) {
+ qemu_printf("TB information already being recorded");
+ return;
+ } else if (tb_stats_collection_paused()) {
+ set_tbstats_flags(level);
+ } else {
+ qht_init(&tb_ctx.tb_stats, tb_stats_cmp, CODE_GEN_HTABLE_SIZE,
+ QHT_MODE_AUTO_RESIZE);
+ }
+
+ set_default_tbstats_flag(level);
+ enable_collect_tb_stats();
+ tb_flush(cpu);
+ break;
+ case PAUSE:
+ if (!tb_stats_collection_enabled()) {
+ qemu_printf("TB information not being recorded");
+ return;
+ }
+
+ /*
+ * Continue to create TBStatistic structures but stop collecting
+ * statistics
+ */
+ pause_collect_tb_stats();
+ set_default_tbstats_flag(TB_NOTHING);
+ set_tbstats_flags(TB_PAUSED);
+ tb_flush(cpu);
+ break;
+ case STOP:
+ if (tb_stats_collection_disabled()) {
+ qemu_printf("TB information not being recorded");
+ return;
+ }
+
+ /* Dissalloc all TBStatistics structures and stop creating new ones */
+ disable_collect_tb_stats();
+ clean_tbstats();
+ tb_flush(cpu);
+ break;
+ case FILTER:
+ if (!tb_stats_collection_enabled()) {
+ qemu_printf("TB information not being recorded");
+ return;
+ }
+ if (!last_search) {
+ qemu_printf(
+ "no search on record! execute info tbs before filtering!");
+ return;
+ }
+
+ set_default_tbstats_flag(TB_NOTHING);
+
+ /*
+ * Set all tbstats as paused, then return only the ones from
last_search
+ */
+ pause_collect_tb_stats();
+ set_tbstats_flags(TB_PAUSED);
+
+ for (GList *iter = last_search; iter; iter = g_list_next(iter)) {
+ TBStatistics *tbs = iter->data;
+ tbs->stats_enabled = level;
+ }
+
+ tb_flush(cpu);
+
+ break;
+ default: /* INVALID */
+ g_assert_not_reached();
+ break;
+ }
+
+ g_free(cmdinfo);
+}
Why isn't all of this in monitor.c?
It's not used or usable from user-only mode.
+void set_tbstats_flags(uint32_t flag)
+{
+ /* iterate over tbstats setting their flag as TB_NOTHING */
+ qht_iter(&tb_ctx.tb_stats, reset_tbstats_flag, &flag);
+}
Again, I question why a global variable is not more appropriate.
r~
