diff mbox

[PATCHv5,5/5] hmp: change hmp_info_cpus to use query-cpus-fast

Message ID 1518797321-28356-6-git-send-email-mihajlov@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Viktor Mihajlovski Feb. 16, 2018, 4:08 p.m. UTC
Changing the implementation of hmp_info_cpus() to call
qmp_query_cpus_fast() instead of qmp_query_cpus. This has the
following consequences:

  o No further code change required for qmp_query_cpus deprecation

  o HMP profits from the less disruptive cpu information retrieval

  o HMP 'info cpus' won't display architecture specific data anymore,
    which should be tolerable in the light of the deprecation of
    query-cpus.

In order to allow 'info cpus' to be executed completely on the
fast path, monitor_get_cpu_index() has been adapted to not synchronize
the cpu state.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
---
 hmp.c     | 41 +++++++----------------------------------
 monitor.c | 13 ++++++++++---
 2 files changed, 17 insertions(+), 37 deletions(-)

Comments

Cornelia Huck Feb. 19, 2018, 2:59 p.m. UTC | #1
On Fri, 16 Feb 2018 17:08:41 +0100
Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> wrote:

> Changing the implementation of hmp_info_cpus() to call

s/Changing/Change/

> qmp_query_cpus_fast() instead of qmp_query_cpus. This has the
> following consequences:
> 
>   o No further code change required for qmp_query_cpus deprecation
> 
>   o HMP profits from the less disruptive cpu information retrieval
> 
>   o HMP 'info cpus' won't display architecture specific data anymore,
>     which should be tolerable in the light of the deprecation of
>     query-cpus.
> 
> In order to allow 'info cpus' to be executed completely on the
> fast path, monitor_get_cpu_index() has been adapted to not synchronize
> the cpu state.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> ---
>  hmp.c     | 41 +++++++----------------------------------
>  monitor.c | 13 ++++++++++---
>  2 files changed, 17 insertions(+), 37 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Eric Blake Feb. 19, 2018, 4:57 p.m. UTC | #2
On 02/16/2018 10:08 AM, Viktor Mihajlovski wrote:
> Changing the implementation of hmp_info_cpus() to call
> qmp_query_cpus_fast() instead of qmp_query_cpus. This has the
> following consequences:
> 
>    o No further code change required for qmp_query_cpus deprecation
> 
>    o HMP profits from the less disruptive cpu information retrieval
> 
>    o HMP 'info cpus' won't display architecture specific data anymore,
>      which should be tolerable in the light of the deprecation of
>      query-cpus.
> 
> In order to allow 'info cpus' to be executed completely on the
> fast path, monitor_get_cpu_index() has been adapted to not synchronize
> the cpu state.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> ---
>   hmp.c     | 41 +++++++----------------------------------
>   monitor.c | 13 ++++++++++---
>   2 files changed, 17 insertions(+), 37 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 7870d6a..ae86bfb 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -360,50 +360,23 @@ void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
>   
>   void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>   {
> -    CpuInfoList *cpu_list, *cpu;
> +    CpuInfoFastList *cpu_list, *cpu;
>   
> -    cpu_list = qmp_query_cpus(NULL);
> +    cpu_list = qmp_query_cpus_fast(NULL);

Pre-existing, but a bit unusual to call this with errors completely 
ignored.  Can be a followup patch, if desired.

Reviewed-by: Eric Blake <eblake@redhat.com>
Dr. David Alan Gilbert Feb. 27, 2018, 7:40 p.m. UTC | #3
* Viktor Mihajlovski (mihajlov@linux.vnet.ibm.com) wrote:
> Changing the implementation of hmp_info_cpus() to call
> qmp_query_cpus_fast() instead of qmp_query_cpus. This has the
> following consequences:
> 
>   o No further code change required for qmp_query_cpus deprecation
> 
>   o HMP profits from the less disruptive cpu information retrieval
> 
>   o HMP 'info cpus' won't display architecture specific data anymore,
>     which should be tolerable in the light of the deprecation of
>     query-cpus.
> 
> In order to allow 'info cpus' to be executed completely on the
> fast path, monitor_get_cpu_index() has been adapted to not synchronize
> the cpu state.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

Yep, that's fine we let HMP change over time.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  hmp.c     | 41 +++++++----------------------------------
>  monitor.c | 13 ++++++++++---
>  2 files changed, 17 insertions(+), 37 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 7870d6a..ae86bfb 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -360,50 +360,23 @@ void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
>  
>  void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>  {
> -    CpuInfoList *cpu_list, *cpu;
> +    CpuInfoFastList *cpu_list, *cpu;
>  
> -    cpu_list = qmp_query_cpus(NULL);
> +    cpu_list = qmp_query_cpus_fast(NULL);
>  
>      for (cpu = cpu_list; cpu; cpu = cpu->next) {
>          int active = ' ';
>  
> -        if (cpu->value->CPU == monitor_get_cpu_index()) {
> +        if (cpu->value->cpu_index == monitor_get_cpu_index()) {
>              active = '*';
>          }
>  
> -        monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU);
> -
> -        switch (cpu->value->arch) {
> -        case CPU_INFO_ARCH_X86:
> -            monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->u.x86.pc);
> -            break;
> -        case CPU_INFO_ARCH_PPC:
> -            monitor_printf(mon, " nip=0x%016" PRIx64, cpu->value->u.ppc.nip);
> -            break;
> -        case CPU_INFO_ARCH_SPARC:
> -            monitor_printf(mon, " pc=0x%016" PRIx64,
> -                           cpu->value->u.q_sparc.pc);
> -            monitor_printf(mon, " npc=0x%016" PRIx64,
> -                           cpu->value->u.q_sparc.npc);
> -            break;
> -        case CPU_INFO_ARCH_MIPS:
> -            monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.q_mips.PC);
> -            break;
> -        case CPU_INFO_ARCH_TRICORE:
> -            monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.tricore.PC);
> -            break;
> -        default:
> -            break;
> -        }
> -
> -        if (cpu->value->halted) {
> -            monitor_printf(mon, " (halted)");
> -        }
> -
> -        monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> +        monitor_printf(mon, "%c CPU #%" PRId64 ":", active,
> +                       cpu->value->cpu_index);
> +        monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
>      }
>  
> -    qapi_free_CpuInfoList(cpu_list);
> +    qapi_free_CpuInfoFastList(cpu_list);
>  }
>  
>  static void print_block_info(Monitor *mon, BlockInfo *info,
> diff --git a/monitor.c b/monitor.c
> index f499250..db57241 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1055,7 +1055,7 @@ int monitor_set_cpu(int cpu_index)
>      return 0;
>  }
>  
> -CPUState *mon_get_cpu(void)
> +static CPUState *mon_get_cpu_sync(bool synchronize)
>  {
>      CPUState *cpu;
>  
> @@ -1074,10 +1074,17 @@ CPUState *mon_get_cpu(void)
>          monitor_set_cpu(first_cpu->cpu_index);
>          cpu = first_cpu;
>      }
> -    cpu_synchronize_state(cpu);
> +    if (synchronize) {
> +        cpu_synchronize_state(cpu);
> +    }
>      return cpu;
>  }
>  
> +CPUState *mon_get_cpu(void)
> +{
> +    return mon_get_cpu_sync(true);
> +}
> +
>  CPUArchState *mon_get_cpu_env(void)
>  {
>      CPUState *cs = mon_get_cpu();
> @@ -1087,7 +1094,7 @@ CPUArchState *mon_get_cpu_env(void)
>  
>  int monitor_get_cpu_index(void)
>  {
> -    CPUState *cs = mon_get_cpu();
> +    CPUState *cs = mon_get_cpu_sync(false);
>  
>      return cs ? cs->cpu_index : UNASSIGNED_CPU_INDEX;
>  }
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

diff --git a/hmp.c b/hmp.c
index 7870d6a..ae86bfb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -360,50 +360,23 @@  void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
 
 void hmp_info_cpus(Monitor *mon, const QDict *qdict)
 {
-    CpuInfoList *cpu_list, *cpu;
+    CpuInfoFastList *cpu_list, *cpu;
 
-    cpu_list = qmp_query_cpus(NULL);
+    cpu_list = qmp_query_cpus_fast(NULL);
 
     for (cpu = cpu_list; cpu; cpu = cpu->next) {
         int active = ' ';
 
-        if (cpu->value->CPU == monitor_get_cpu_index()) {
+        if (cpu->value->cpu_index == monitor_get_cpu_index()) {
             active = '*';
         }
 
-        monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU);
-
-        switch (cpu->value->arch) {
-        case CPU_INFO_ARCH_X86:
-            monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->u.x86.pc);
-            break;
-        case CPU_INFO_ARCH_PPC:
-            monitor_printf(mon, " nip=0x%016" PRIx64, cpu->value->u.ppc.nip);
-            break;
-        case CPU_INFO_ARCH_SPARC:
-            monitor_printf(mon, " pc=0x%016" PRIx64,
-                           cpu->value->u.q_sparc.pc);
-            monitor_printf(mon, " npc=0x%016" PRIx64,
-                           cpu->value->u.q_sparc.npc);
-            break;
-        case CPU_INFO_ARCH_MIPS:
-            monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.q_mips.PC);
-            break;
-        case CPU_INFO_ARCH_TRICORE:
-            monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.tricore.PC);
-            break;
-        default:
-            break;
-        }
-
-        if (cpu->value->halted) {
-            monitor_printf(mon, " (halted)");
-        }
-
-        monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
+        monitor_printf(mon, "%c CPU #%" PRId64 ":", active,
+                       cpu->value->cpu_index);
+        monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
     }
 
-    qapi_free_CpuInfoList(cpu_list);
+    qapi_free_CpuInfoFastList(cpu_list);
 }
 
 static void print_block_info(Monitor *mon, BlockInfo *info,
diff --git a/monitor.c b/monitor.c
index f499250..db57241 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1055,7 +1055,7 @@  int monitor_set_cpu(int cpu_index)
     return 0;
 }
 
-CPUState *mon_get_cpu(void)
+static CPUState *mon_get_cpu_sync(bool synchronize)
 {
     CPUState *cpu;
 
@@ -1074,10 +1074,17 @@  CPUState *mon_get_cpu(void)
         monitor_set_cpu(first_cpu->cpu_index);
         cpu = first_cpu;
     }
-    cpu_synchronize_state(cpu);
+    if (synchronize) {
+        cpu_synchronize_state(cpu);
+    }
     return cpu;
 }
 
+CPUState *mon_get_cpu(void)
+{
+    return mon_get_cpu_sync(true);
+}
+
 CPUArchState *mon_get_cpu_env(void)
 {
     CPUState *cs = mon_get_cpu();
@@ -1087,7 +1094,7 @@  CPUArchState *mon_get_cpu_env(void)
 
 int monitor_get_cpu_index(void)
 {
-    CPUState *cs = mon_get_cpu();
+    CPUState *cs = mon_get_cpu_sync(false);
 
     return cs ? cs->cpu_index : UNASSIGNED_CPU_INDEX;
 }