Re: [Qemu-devel] [PATCH v4 5/7] log: adding -d tb_stats to control tbstats

["Alex Bennée" <alex.bennee@linaro.org>]

vandersonmr <vandersonmr2@gmail.com> writes:

> Adding -d tb_stats:[limit:[all|jit|exec]] to control TBStatistics
> collection. "limit" is used to limit the number of TBStats in the
> linux-user dump. [all|jit|exec] control the profilling level used
> by the TBStats: all, only jit stats or only execution count stats.
>
> Signed-off-by: Vanderson M. do Rosario <vandersonmr2@gmail.com>
> ---
>  include/qemu/log.h |  1 +
>  util/log.c         | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index 8cdfc268ca..93642603e5 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -45,6 +45,7 @@ static inline bool qemu_log_separate(void)
>  /* LOG_TRACE (1 << 15) is defined in log-for-trace.h */
>  #define CPU_LOG_TB_OP_IND  (1 << 16)
>  #define CPU_LOG_TB_FPU     (1 << 17)
> +#define CPU_LOG_TB_STATS   (1 << 18)
>
>  /* Lock output for a series of related logs.  Since this is not needed
>   * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
> diff --git a/util/log.c b/util/log.c
> index f81653d712..8109d19afb 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -30,6 +30,7 @@ FILE *qemu_logfile;
>  int qemu_loglevel;
>  static int log_append = 0;
>  static GArray *debug_regions;
> +int32_t max_num_hot_tbs_to_dump;
>
>  /* Return the number of characters emitted.  */
>  int qemu_log(const char *fmt, ...)
> @@ -273,6 +274,8 @@ const QEMULogItem qemu_log_items[] = {
>      { CPU_LOG_TB_NOCHAIN, "nochain",
>        "do not chain compiled TBs so that \"exec\" and \"cpu\" show\n"
>        "complete traces" },
> +    { CPU_LOG_TB_STATS, "tb_stats[:limit:(all,jit,exec)]",
> +      "show TBs statistics (until given a limit) ordered by their hotness.\n" },
>      { 0, NULL, NULL },
>  };
>
> @@ -294,6 +297,37 @@ int qemu_str_to_log_mask(const char *str)
>              trace_enable_events((*tmp) + 6);
>              mask |= LOG_TRACE;
>  #endif
> +        } else if (g_str_has_prefix(*tmp, "tb_stats")) {
> +            if (g_str_has_prefix(*tmp, "tb_stats:") && (*tmp)[9] != '\0') {
> +
> +                if (!g_ascii_isdigit(*((*tmp) + 9))) {
> +                    fprintf(stderr,
> +                            "should be a number follow by [all|jit|exec], as tb_stats:10:all\n");
> +                    exit(1);
> +                }
> +                /* get limit */
> +                max_num_hot_tbs_to_dump = atoi((*tmp) + 9);

We probably want to handle -d tb_stats:[all|jit|exec] as well because we
might be doing interactive exploration for softmmu. I'd suggest
splitting the number processing off to a new patch and merging the exit
processing part of the later patches with it. As you don't have a HMP
for linux-user you can make the number required for linux-user or only
optional for softmmu.

> +
> +                /* get profilling level */
> +                char *s = (*tmp) + 9;
> +                while (*s != '\0') {
> +                    if (*s++ == ':') {
> +                        break;
> +                    }
> +                }
> +                if (g_str_equal(s, "jit") == 0) {
> +                    set_default_tbstats_flag(TB_JIT_STATS);
> +                } else if (g_str_equal(s, "exec") == 0) {
> +                    set_default_tbstats_flag(TB_EXEC_STATS);
> +                } else {
> +                    set_default_tbstats_flag(TB_JIT_STATS | TB_EXEC_STATS);
> +                }

If additional stats types get added we shall want to be additive:

  -d host_tbs:jit,exec

> +            } else {
> +                set_default_tbstats_flag(TB_JIT_STATS | TB_EXEC_STATS);
> +            }
> +
> +            mask |= CPU_LOG_TB_STATS;
> +            enable_collect_tb_stats();
>          } else {
>              for (item = qemu_log_items; item->mask != 0; item++) {
>                  if (g_str_equal(*tmp, item->name)) {


--
Alex Bennée