[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 5/6] monitor: adding info tb and tbs to monit
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v3 5/6] monitor: adding info tb and tbs to monitor |
Date: |
Tue, 9 Jul 2019 10:57:16 +0100 |
User-agent: |
Mutt/1.12.0 (2019-05-25) |
* vandersonmr (address@hidden) wrote:
> adding options to list tbs by some metric and
> investigate their code.
>
> Signed-off-by: Vanderson M. do Rosario <address@hidden>
As Markus said you need a short justification that it's for debug etc
to justify HMP only; it doesn't need to be huge, but we should have it.
> ---
> hmp-commands-info.hx | 22 ++++++++++++++
> monitor/misc.c | 69 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 91 insertions(+)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index c59444c461..0b8c0de95d 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -288,6 +288,28 @@ ETEXI
> .params = "",
> .help = "show dynamic compiler info",
> .cmd = hmp_info_jit,
> + {
> + .name = "tbs",
> + .args_type = "number:i?,sortedby:s?",
> + .params = "[number sortedby]",
> + .help = "show a [number] translated blocks sorted by
> [sortedby]"
> + "sortedby opts: hotness hg",
If this is showing 'number' blocks then I think it should be called
'count'
> + .cmd = hmp_info_tbs,
> + },
> + {
> + .name = "tb",
> + .args_type = "id:i,flags:s?",
> + .params = "id [log1[,...] flags]",
> + .help = "show information about one translated block by id",
> + .cmd = hmp_info_tb,
That doesn't say what those flags are for; qemu has lots of flags for
different things; I think you're saying it's some log flag?
> + },
> + {
> + .name = "coverset",
> + .args_type = "number:i?",
> + .params = "[number]",
> + .help = "show hottest translated blocks neccesary to cover"
> + "[number]% of the execution count",
> + .cmd = hmp_info_coverset,
> },
That 'number' should be something like 'percent' or 'coverage'
> #endif
>
> diff --git a/monitor/misc.c b/monitor/misc.c
> index bf9faceb86..1fb4d75871 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -469,6 +469,75 @@ static void hmp_info_jit(Monitor *mon, const QDict
> *qdict)
> dump_drift_info();
> }
>
> +static void hmp_info_tbs(Monitor *mon, const QDict *qdict)
> +{
> + int n;
> + const char *s = NULL;
> + if (!tcg_enabled()) {
> + error_report("TB information is only available with accel=tcg");
> + return;
> + }
> + if (!tb_ctx.tb_stats.map) {
> + error_report("no TB information recorded");
> + return;
> + }
> +
> + n = qdict_get_try_int(qdict, "number", 10);
> + s = qdict_get_try_str(qdict, "sortedby");
No need to use single characters; sortedby_str is fine for
example.
> + int sortedby = 0;
> + if (s == NULL || strcmp(s, "hotness") == 0) {
> + sortedby = SORT_BY_HOTNESS;
> + } else if (strcmp(s, "hg") == 0) {
> + sortedby = SORT_BY_HG;
> + }
You should error if there's another word in 's'
> + dump_tbs_info(n, sortedby, true);
> +}
> +
> +static void hmp_info_tb(Monitor *mon, const QDict *qdict)
> +{
> + const int id = qdict_get_int(qdict, "id");
> + const char *flags = qdict_get_try_str(qdict, "flags");
> + int mask;
> +
> + if (!tcg_enabled()) {
> + error_report("TB information is only available with accel=tcg");
> + return;
> + }
> +
> + mask = flags ? qemu_str_to_log_mask(flags) : CPU_LOG_TB_IN_ASM;
> +
> + if (!mask) {
> + help_cmd(mon, "log");
That's not obvious - I'd perfer you said something like
Unable to parse log flags, see 'help log'
> + return;
> + }
> +
> + dump_tb_info(id, mask, true);
> +}
> +
> +static void hmp_info_coverset(Monitor *mon, const QDict *qdict)
> +{
> + int n;
> + if (!tcg_enabled()) {
> + error_report("TB information is only available with accel=tcg");
> + return;
> + }
> + if (!qemu_loglevel_mask(CPU_LOG_HOT_TBS)) {
> + error_report("TB information not being recorded");
> + return;
> + }
> +
> + n = qdict_get_try_int(qdict, "number", 90);
> +
> + if (n < 0 || n > 100) {
> + error_report("Coverset percentage should be between 0 and 100");
> + return;
> + }
> +
> + dump_coverset_info(n, true);
> +}
> +
> static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
> {
> dump_opcount_info();
> --
> 2.22.0
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v3 4/6] util/log: introduce dump of tbstats and -d hot_tbs:limit, (continued)
- Re: [Qemu-devel] [PATCH v3 5/6] monitor: adding info tb and tbs to monitor,
Dr. David Alan Gilbert <=
Re: [Qemu-devel] [PATCH v3 1/6] accel/tcg: adding structure to store TB statistics, Alex Bennée, 2019/07/04
Re: [Qemu-devel] [PATCH v3 1/6] accel/tcg: adding structure to store TB statistics, Alex Bennée, 2019/07/04