[v1] monitor: Support specified vCPU registers

[PATCH] monitor: Support specified vCPU registers

Posted by zhenwei pi 3 years, 6 months ago

Originally we have to get all the vCPU registers and parse the
specified one. To improve the performance of this usage, allow user
specified vCPU id to query registers.

Run a VM with 16 vCPU, use bcc tool to track the latency of
'hmp_info_registers':
'info registers -a' uses about 3ms;
'info register 12' uses about 150us.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 hmp-commands-info.hx |  6 +++---
 monitor/misc.c       | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 3ffa24bd67..6023e2b5c5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -100,9 +100,9 @@ ERST
 
     {
         .name       = "registers",
-        .args_type  = "cpustate_all:-a",
-        .params     = "[-a]",
-        .help       = "show the cpu registers (-a: all - show register info for all cpus)",
+        .args_type  = "cpustate_all:-a,vcpu:i?",
+        .params     = "[-a] [vcpu]",
+        .help       = "show the cpu registers (-a: all - show register info for all cpus; vcpu: vCPU to query)",
         .cmd        = hmp_info_registers,
     },
 
diff --git a/monitor/misc.c b/monitor/misc.c
index 3d2312ba8d..b12309faad 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -307,6 +307,7 @@ int monitor_get_cpu_index(Monitor *mon)
 static void hmp_info_registers(Monitor *mon, const QDict *qdict)
 {
     bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
+    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
     CPUState *cs;
 
     if (all_cpus) {
@@ -314,6 +315,24 @@ static void hmp_info_registers(Monitor *mon, const QDict *qdict)
             monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
             cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
         }
+    } else if (vcpu >= 0) {
+        CPUState *target_cs = NULL;
+
+        CPU_FOREACH(cs) {
+            if (cs->cpu_index == vcpu) {
+                target_cs = cs;
+                break;
+            }
+        }
+
+        if (!target_cs) {
+            monitor_printf(mon, "CPU#%d not available\n", vcpu);
+            return;
+        }
+
+        monitor_printf(mon, "\nCPU#%d\n", target_cs->cpu_index);
+        cpu_dump_state(target_cs, NULL, CPU_DUMP_FPU);
+        return;
     } else {
         cs = mon_get_cpu(mon);
 
-- 
2.20.1

Re: [PATCH] monitor: Support specified vCPU registers

Posted by Darren Kenny 3 years, 6 months ago

On Tuesday, 2022-07-19 at 15:55:44 +08, zhenwei pi wrote:
> Originally we have to get all the vCPU registers and parse the
> specified one. To improve the performance of this usage, allow user
> specified vCPU id to query registers.
>
> Run a VM with 16 vCPU, use bcc tool to track the latency of
> 'hmp_info_registers':
> 'info registers -a' uses about 3ms;
> 'info register 12' uses about 150us.

TYPO: I'm assuming that should be 'info registers 12'?

>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  hmp-commands-info.hx |  6 +++---
>  monitor/misc.c       | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 3ffa24bd67..6023e2b5c5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -100,9 +100,9 @@ ERST
>  
>      {
>          .name       = "registers",
> -        .args_type  = "cpustate_all:-a",
> -        .params     = "[-a]",
> -        .help       = "show the cpu registers (-a: all - show register info for all cpus)",
> +        .args_type  = "cpustate_all:-a,vcpu:i?",
> +        .params     = "[-a] [vcpu]",

From what I can see in the code, only one of these may be specified at a
time - or at least, '-a' will take precedence.

Maybe it would read more correctly as '[-a|vcpu]' ?

> +        .help       = "show the cpu registers (-a: all - show register info for all cpus; vcpu: vCPU to query)",

Possibly also it would be worth saying "...; vcpu: specific vCPU to query"?

>          .cmd        = hmp_info_registers,
>      },
>  
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 3d2312ba8d..b12309faad 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -307,6 +307,7 @@ int monitor_get_cpu_index(Monitor *mon)
>  static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>  {
>      bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
> +    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
>      CPUState *cs;
>  
>      if (all_cpus) {
> @@ -314,6 +315,24 @@ static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>              monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
>              cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
>          }
> +    } else if (vcpu >= 0) {
> +        CPUState *target_cs = NULL;
> +
> +        CPU_FOREACH(cs) {
> +            if (cs->cpu_index == vcpu) {
> +                target_cs = cs;
> +                break;
> +            }
> +        }
> +
> +        if (!target_cs) {
> +            monitor_printf(mon, "CPU#%d not available\n", vcpu);
> +            return;
> +        }
> +
> +        monitor_printf(mon, "\nCPU#%d\n", target_cs->cpu_index);
> +        cpu_dump_state(target_cs, NULL, CPU_DUMP_FPU);
> +        return;

This return call seems unnecessary here.

Thanks,

Darren.

PING: [PATCH] monitor: Support specified vCPU registers

Posted by zhenwei pi 3 years, 6 months ago

PING!

On 7/19/22 15:55, zhenwei pi wrote:
> Originally we have to get all the vCPU registers and parse the
> specified one. To improve the performance of this usage, allow user
> specified vCPU id to query registers.
> 
> Run a VM with 16 vCPU, use bcc tool to track the latency of
> 'hmp_info_registers':
> 'info registers -a' uses about 3ms;
> 'info register 12' uses about 150us.
> 
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>   hmp-commands-info.hx |  6 +++---
>   monitor/misc.c       | 19 +++++++++++++++++++
>   2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 3ffa24bd67..6023e2b5c5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -100,9 +100,9 @@ ERST
>   
>       {
>           .name       = "registers",
> -        .args_type  = "cpustate_all:-a",
> -        .params     = "[-a]",
> -        .help       = "show the cpu registers (-a: all - show register info for all cpus)",
> +        .args_type  = "cpustate_all:-a,vcpu:i?",
> +        .params     = "[-a] [vcpu]",
> +        .help       = "show the cpu registers (-a: all - show register info for all cpus; vcpu: vCPU to query)",
>           .cmd        = hmp_info_registers,
>       },
>   
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 3d2312ba8d..b12309faad 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -307,6 +307,7 @@ int monitor_get_cpu_index(Monitor *mon)
>   static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>   {
>       bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
> +    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
>       CPUState *cs;
>   
>       if (all_cpus) {
> @@ -314,6 +315,24 @@ static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>               monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
>               cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
>           }
> +    } else if (vcpu >= 0) {
> +        CPUState *target_cs = NULL;
> +
> +        CPU_FOREACH(cs) {
> +            if (cs->cpu_index == vcpu) {
> +                target_cs = cs;
> +                break;
> +            }
> +        }
> +
> +        if (!target_cs) {
> +            monitor_printf(mon, "CPU#%d not available\n", vcpu);
> +            return;
> +        }
> +
> +        monitor_printf(mon, "\nCPU#%d\n", target_cs->cpu_index);
> +        cpu_dump_state(target_cs, NULL, CPU_DUMP_FPU);
> +        return;
>       } else {
>           cs = mon_get_cpu(mon);
>   

-- 
zhenwei pi