qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory
@ 2014-10-16 12:02 zhanghailiang
  2014-10-16 12:50 ` Gonglei
  0 siblings, 1 reply; 5+ messages in thread
From: zhanghailiang @ 2014-10-16 12:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: zhanghailiang, hutao, luonengjun, peter.huangpeng, pbonzini,
	imammedo

When do memory hotplug, if there is numa node, we should add
the memory size to the corresponding node memory size.

For now, it mainly affects the result of hmp command "info numa".

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 v5:
- reword the subject (Igor Mammedov)
- turn query_numa_node_mem to void (Igor Mammedov)
 v4:
- s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
- rewrite numa_stat_memory_devices and this will also fix compile error for 
  targets that don't support memory hotplug
 v3:
- cold-plugged memory should not be excluded (Igor Mammedov)
 v2:
- Don't modify the numa_info.node_mem directly when treating hotplug memory,
  fix the "info numa" instead (Igor Mammedov)

Thanks for review!;)
---
 include/sysemu/sysemu.h |  1 +
 monitor.c               |  6 +++++-
 numa.c                  | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 0037a69..ef5eaf4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -161,6 +161,7 @@ typedef struct node_info {
 extern NodeInfo numa_info[MAX_NODES];
 void set_numa_nodes(void);
 void set_numa_modes(void);
+void query_numa_node_mem(uint64_t *node_mem);
 extern QemuOptsList qemu_numa_opts;
 int numa_init_func(QemuOpts *opts, void *opaque);
 
diff --git a/monitor.c b/monitor.c
index 2d14f39..d45b0a3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1949,7 +1949,10 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
 {
     int i;
     CPUState *cpu;
+    uint64_t *node_mem;
 
+    node_mem = g_new0(uint64_t, nb_numa_nodes);
+    query_numa_node_mem(node_mem);
     monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
     for (i = 0; i < nb_numa_nodes; i++) {
         monitor_printf(mon, "node %d cpus:", i);
@@ -1960,8 +1963,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
         }
         monitor_printf(mon, "\n");
         monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
-            numa_info[i].node_mem >> 20);
+                       node_mem[i] >> 20);
     }
+    g_free(node_mem);
 }
 
 #ifdef CONFIG_PROFILER
diff --git a/numa.c b/numa.c
index 3b98135..5b84ffb 100644
--- a/numa.c
+++ b/numa.c
@@ -35,6 +35,7 @@
 #include "hw/boards.h"
 #include "sysemu/hostmem.h"
 #include "qmp-commands.h"
+#include "hw/mem/pc-dimm.h"
 
 QemuOptsList qemu_numa_opts = {
     .name = "numa",
@@ -315,6 +316,46 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
     }
 }
 
+static void numa_stat_memory_devices(uint64_t *node_mem)
+{
+    MemoryDeviceInfoList *info_list = NULL;
+    MemoryDeviceInfoList **prev = &info_list;
+    MemoryDeviceInfoList *info;
+
+    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
+    for (info = info_list; info; info = info->next) {
+        MemoryDeviceInfo *value = info->value;
+
+        if (value) {
+            switch (value->kind) {
+            case MEMORY_DEVICE_INFO_KIND_DIMM: {
+                PCDIMMDeviceInfo *di = value->dimm;
+
+                node_mem[di->node] += di->size;
+                break;
+            }
+            default:
+                break;
+            }
+        }
+    }
+    qapi_free_MemoryDeviceInfoList(info_list);
+}
+
+void query_numa_node_mem(uint64_t *node_mem)
+{
+    int i;
+
+    if (nb_numa_nodes <= 0) {
+        return;
+    }
+
+    numa_stat_memory_devices(node_mem);
+    for (i = 0; i < nb_numa_nodes; i++) {
+        node_mem[i] += numa_info[i].node_mem;
+    }
+}
+
 static int query_memdev(Object *obj, void *opaque)
 {
     MemdevList **list = opaque;
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory
  2014-10-16 12:02 [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory zhanghailiang
@ 2014-10-16 12:50 ` Gonglei
  2014-10-17  0:45   ` zhanghailiang
  0 siblings, 1 reply; 5+ messages in thread
From: Gonglei @ 2014-10-16 12:50 UTC (permalink / raw)
  To: zhanghailiang
  Cc: hutao, luonengjun, qemu-devel, peter.huangpeng, pbonzini,
	imammedo

On 2014/10/16 20:02, zhanghailiang wrote:

> When do memory hotplug, if there is numa node, we should add
> the memory size to the corresponding node memory size.
> 
> For now, it mainly affects the result of hmp command "info numa".
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  v5:
> - reword the subject (Igor Mammedov)
> - turn query_numa_node_mem to void (Igor Mammedov)
>  v4:
> - s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
> - rewrite numa_stat_memory_devices and this will also fix compile error for 
>   targets that don't support memory hotplug
>  v3:
> - cold-plugged memory should not be excluded (Igor Mammedov)
>  v2:
> - Don't modify the numa_info.node_mem directly when treating hotplug memory,
>   fix the "info numa" instead (Igor Mammedov)
> 
> Thanks for review!;)
> ---
>  include/sysemu/sysemu.h |  1 +
>  monitor.c               |  6 +++++-
>  numa.c                  | 41 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 0037a69..ef5eaf4 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -161,6 +161,7 @@ typedef struct node_info {
>  extern NodeInfo numa_info[MAX_NODES];
>  void set_numa_nodes(void);
>  void set_numa_modes(void);
> +void query_numa_node_mem(uint64_t *node_mem);
>  extern QemuOptsList qemu_numa_opts;
>  int numa_init_func(QemuOpts *opts, void *opaque);
>  
> diff --git a/monitor.c b/monitor.c
> index 2d14f39..d45b0a3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1949,7 +1949,10 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>  {
>      int i;
>      CPUState *cpu;
> +    uint64_t *node_mem;
>  
> +    node_mem = g_new0(uint64_t, nb_numa_nodes);
> +    query_numa_node_mem(node_mem);
>      monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
>      for (i = 0; i < nb_numa_nodes; i++) {
>          monitor_printf(mon, "node %d cpus:", i);
> @@ -1960,8 +1963,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>          }
>          monitor_printf(mon, "\n");
>          monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
> -            numa_info[i].node_mem >> 20);
> +                       node_mem[i] >> 20);
>      }
> +    g_free(node_mem);
>  }
>  
>  #ifdef CONFIG_PROFILER
> diff --git a/numa.c b/numa.c
> index 3b98135..5b84ffb 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -35,6 +35,7 @@
>  #include "hw/boards.h"
>  #include "sysemu/hostmem.h"
>  #include "qmp-commands.h"
> +#include "hw/mem/pc-dimm.h"
>  
>  QemuOptsList qemu_numa_opts = {
>      .name = "numa",
> @@ -315,6 +316,46 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>      }
>  }
>  
> +static void numa_stat_memory_devices(uint64_t *node_mem)
> +{
> +    MemoryDeviceInfoList *info_list = NULL;
> +    MemoryDeviceInfoList **prev = &info_list;
> +    MemoryDeviceInfoList *info;
> +
> +    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
> +    for (info = info_list; info; info = info->next) {
> +        MemoryDeviceInfo *value = info->value;
> +
> +        if (value) {
> +            switch (value->kind) {
> +            case MEMORY_DEVICE_INFO_KIND_DIMM: {

I think this '{}' symbol is superfluous.

Best regards,
-Gonglei

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory
  2014-10-16 12:50 ` Gonglei
@ 2014-10-17  0:45   ` zhanghailiang
  2014-10-17  1:06     ` Gonglei
  0 siblings, 1 reply; 5+ messages in thread
From: zhanghailiang @ 2014-10-17  0:45 UTC (permalink / raw)
  To: Gonglei; +Cc: hutao, luonengjun, qemu-devel, peter.huangpeng, pbonzini,
	imammedo

On 2014/10/16 20:50, Gonglei wrote:
> On 2014/10/16 20:02, zhanghailiang wrote:
>
>> When do memory hotplug, if there is numa node, we should add
>> the memory size to the corresponding node memory size.
>>
>> For now, it mainly affects the result of hmp command "info numa".
>>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   v5:
>> - reword the subject (Igor Mammedov)
>> - turn query_numa_node_mem to void (Igor Mammedov)
>>   v4:
>> - s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
>> - rewrite numa_stat_memory_devices and this will also fix compile error for
>>    targets that don't support memory hotplug
>>   v3:
>> - cold-plugged memory should not be excluded (Igor Mammedov)
>>   v2:
>> - Don't modify the numa_info.node_mem directly when treating hotplug memory,
>>    fix the "info numa" instead (Igor Mammedov)
>>
>> Thanks for review!;)
>> ---
>>   include/sysemu/sysemu.h |  1 +
>>   monitor.c               |  6 +++++-
>>   numa.c                  | 41 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 0037a69..ef5eaf4 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -161,6 +161,7 @@ typedef struct node_info {
>>   extern NodeInfo numa_info[MAX_NODES];
>>   void set_numa_nodes(void);
>>   void set_numa_modes(void);
>> +void query_numa_node_mem(uint64_t *node_mem);
>>   extern QemuOptsList qemu_numa_opts;
>>   int numa_init_func(QemuOpts *opts, void *opaque);
>>
>> diff --git a/monitor.c b/monitor.c
>> index 2d14f39..d45b0a3 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1949,7 +1949,10 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>>   {
>>       int i;
>>       CPUState *cpu;
>> +    uint64_t *node_mem;
>>
>> +    node_mem = g_new0(uint64_t, nb_numa_nodes);
>> +    query_numa_node_mem(node_mem);
>>       monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
>>       for (i = 0; i < nb_numa_nodes; i++) {
>>           monitor_printf(mon, "node %d cpus:", i);
>> @@ -1960,8 +1963,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>>           }
>>           monitor_printf(mon, "\n");
>>           monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
>> -            numa_info[i].node_mem >> 20);
>> +                       node_mem[i] >> 20);
>>       }
>> +    g_free(node_mem);
>>   }
>>
>>   #ifdef CONFIG_PROFILER
>> diff --git a/numa.c b/numa.c
>> index 3b98135..5b84ffb 100644
>> --- a/numa.c
>> +++ b/numa.c
>> @@ -35,6 +35,7 @@
>>   #include "hw/boards.h"
>>   #include "sysemu/hostmem.h"
>>   #include "qmp-commands.h"
>> +#include "hw/mem/pc-dimm.h"
>>
>>   QemuOptsList qemu_numa_opts = {
>>       .name = "numa",
>> @@ -315,6 +316,46 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>>       }
>>   }
>>
>> +static void numa_stat_memory_devices(uint64_t *node_mem)
>> +{
>> +    MemoryDeviceInfoList *info_list = NULL;
>> +    MemoryDeviceInfoList **prev = &info_list;
>> +    MemoryDeviceInfoList *info;
>> +
>> +    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
>> +    for (info = info_list; info; info = info->next) {
>> +        MemoryDeviceInfo *value = info->value;
>> +
>> +        if (value) {
>> +            switch (value->kind) {
>> +            case MEMORY_DEVICE_INFO_KIND_DIMM: {
>
> I think this '{}' symbol is superfluous.
>

No, without it, compiler will complain 'error: a label can only be part of
statement and a declaration is not a statement'.

So it is necessary for defining a variable inside switch.
Actually, you will see lots of such statements in kernel source;)

> Best regards,
> -Gonglei
>
>
>
> .
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory
  2014-10-17  0:45   ` zhanghailiang
@ 2014-10-17  1:06     ` Gonglei
  2014-10-17  2:12       ` zhanghailiang
  0 siblings, 1 reply; 5+ messages in thread
From: Gonglei @ 2014-10-17  1:06 UTC (permalink / raw)
  To: Zhanghailiang
  Cc: hutao@cn.fujitsu.com, Luonengjun, qemu-devel@nongnu.org,
	Huangpeng (Peter), pbonzini@redhat.com, imammedo@redhat.com

On 2014/10/17 8:45, Zhanghailiang wrote:

> On 2014/10/16 20:50, Gonglei wrote:
>> On 2014/10/16 20:02, zhanghailiang wrote:
>>
>>> When do memory hotplug, if there is numa node, we should add
>>> the memory size to the corresponding node memory size.
>>>
>>> For now, it mainly affects the result of hmp command "info numa".
>>>
>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>> ---
>>>   v5:
>>> - reword the subject (Igor Mammedov)
>>> - turn query_numa_node_mem to void (Igor Mammedov)
>>>   v4:
>>> - s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
>>> - rewrite numa_stat_memory_devices and this will also fix compile error for
>>>    targets that don't support memory hotplug
>>>   v3:
>>> - cold-plugged memory should not be excluded (Igor Mammedov)
>>>   v2:
>>> - Don't modify the numa_info.node_mem directly when treating hotplug memory,
>>>    fix the "info numa" instead (Igor Mammedov)
>>>
>>> Thanks for review!;)
>>> ---
>>>   include/sysemu/sysemu.h |  1 +
>>>   monitor.c               |  6 +++++-
>>>   numa.c                  | 41 +++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 47 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>>> index 0037a69..ef5eaf4 100644
>>> --- a/include/sysemu/sysemu.h
>>> +++ b/include/sysemu/sysemu.h
>>> @@ -161,6 +161,7 @@ typedef struct node_info {
>>>   extern NodeInfo numa_info[MAX_NODES];
>>>   void set_numa_nodes(void);
>>>   void set_numa_modes(void);
>>> +void query_numa_node_mem(uint64_t *node_mem);
>>>   extern QemuOptsList qemu_numa_opts;
>>>   int numa_init_func(QemuOpts *opts, void *opaque);
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index 2d14f39..d45b0a3 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -1949,7 +1949,10 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>>>   {
>>>       int i;
>>>       CPUState *cpu;
>>> +    uint64_t *node_mem;
>>>
>>> +    node_mem = g_new0(uint64_t, nb_numa_nodes);
>>> +    query_numa_node_mem(node_mem);
>>>       monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
>>>       for (i = 0; i < nb_numa_nodes; i++) {
>>>           monitor_printf(mon, "node %d cpus:", i);
>>> @@ -1960,8 +1963,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>>>           }
>>>           monitor_printf(mon, "\n");
>>>           monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
>>> -            numa_info[i].node_mem >> 20);
>>> +                       node_mem[i] >> 20);
>>>       }
>>> +    g_free(node_mem);
>>>   }
>>>
>>>   #ifdef CONFIG_PROFILER
>>> diff --git a/numa.c b/numa.c
>>> index 3b98135..5b84ffb 100644
>>> --- a/numa.c
>>> +++ b/numa.c
>>> @@ -35,6 +35,7 @@
>>>   #include "hw/boards.h"
>>>   #include "sysemu/hostmem.h"
>>>   #include "qmp-commands.h"
>>> +#include "hw/mem/pc-dimm.h"
>>>
>>>   QemuOptsList qemu_numa_opts = {
>>>       .name = "numa",
>>> @@ -315,6 +316,46 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>>>       }
>>>   }
>>>
>>> +static void numa_stat_memory_devices(uint64_t *node_mem)
>>> +{
>>> +    MemoryDeviceInfoList *info_list = NULL;
>>> +    MemoryDeviceInfoList **prev = &info_list;
>>> +    MemoryDeviceInfoList *info;
>>> +
>>> +    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
>>> +    for (info = info_list; info; info = info->next) {
>>> +        MemoryDeviceInfo *value = info->value;
>>> +
>>> +        if (value) {
>>> +            switch (value->kind) {
>>> +            case MEMORY_DEVICE_INFO_KIND_DIMM: {
>>
>> I think this '{}' symbol is superfluous.
>>
> 
> No, without it, compiler will complain 'error: a label can only be part of
> statement and a declaration is not a statement'.
> 

I see. You just add it for the declaration of a local variable. Actually, the 'di'
is not necessary,  How about like below?

node_mem[value->dimm->node] += value->dimm->size;

Anyway, your patch is ok on function. :)

Best regards,
-Gonglei

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory
  2014-10-17  1:06     ` Gonglei
@ 2014-10-17  2:12       ` zhanghailiang
  0 siblings, 0 replies; 5+ messages in thread
From: zhanghailiang @ 2014-10-17  2:12 UTC (permalink / raw)
  To: Gonglei
  Cc: hutao@cn.fujitsu.com, Luonengjun, qemu-devel@nongnu.org,
	Huangpeng (Peter), pbonzini@redhat.com, imammedo@redhat.com

On 2014/10/17 9:06, Gonglei wrote:
> On 2014/10/17 8:45, Zhanghailiang wrote:
>
>> On 2014/10/16 20:50, Gonglei wrote:
>>> On 2014/10/16 20:02, zhanghailiang wrote:
>>>
>>>> When do memory hotplug, if there is numa node, we should add
>>>> the memory size to the corresponding node memory size.
>>>>
>>>> For now, it mainly affects the result of hmp command "info numa".
>>>>
>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>> ---
>>>>    v5:
>>>> - reword the subject (Igor Mammedov)
>>>> - turn query_numa_node_mem to void (Igor Mammedov)
>>>>    v4:
>>>> - s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
>>>> - rewrite numa_stat_memory_devices and this will also fix compile error for
>>>>     targets that don't support memory hotplug
>>>>    v3:
>>>> - cold-plugged memory should not be excluded (Igor Mammedov)
>>>>    v2:
>>>> - Don't modify the numa_info.node_mem directly when treating hotplug memory,
>>>>     fix the "info numa" instead (Igor Mammedov)
>>>>
>>>> Thanks for review!;)
>>>> ---
>>>>    include/sysemu/sysemu.h |  1 +
>>>>    monitor.c               |  6 +++++-
>>>>    numa.c                  | 41 +++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 47 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>>>> index 0037a69..ef5eaf4 100644
>>>> --- a/include/sysemu/sysemu.h
>>>> +++ b/include/sysemu/sysemu.h
>>>> @@ -161,6 +161,7 @@ typedef struct node_info {
>>>>    extern NodeInfo numa_info[MAX_NODES];
>>>>    void set_numa_nodes(void);
>>>>    void set_numa_modes(void);
>>>> +void query_numa_node_mem(uint64_t *node_mem);
>>>>    extern QemuOptsList qemu_numa_opts;
>>>>    int numa_init_func(QemuOpts *opts, void *opaque);
>>>>
>>>> diff --git a/monitor.c b/monitor.c
>>>> index 2d14f39..d45b0a3 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -1949,7 +1949,10 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>>>>    {
>>>>        int i;
>>>>        CPUState *cpu;
>>>> +    uint64_t *node_mem;
>>>>
>>>> +    node_mem = g_new0(uint64_t, nb_numa_nodes);
>>>> +    query_numa_node_mem(node_mem);
>>>>        monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
>>>>        for (i = 0; i < nb_numa_nodes; i++) {
>>>>            monitor_printf(mon, "node %d cpus:", i);
>>>> @@ -1960,8 +1963,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
>>>>            }
>>>>            monitor_printf(mon, "\n");
>>>>            monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
>>>> -            numa_info[i].node_mem >> 20);
>>>> +                       node_mem[i] >> 20);
>>>>        }
>>>> +    g_free(node_mem);
>>>>    }
>>>>
>>>>    #ifdef CONFIG_PROFILER
>>>> diff --git a/numa.c b/numa.c
>>>> index 3b98135..5b84ffb 100644
>>>> --- a/numa.c
>>>> +++ b/numa.c
>>>> @@ -35,6 +35,7 @@
>>>>    #include "hw/boards.h"
>>>>    #include "sysemu/hostmem.h"
>>>>    #include "qmp-commands.h"
>>>> +#include "hw/mem/pc-dimm.h"
>>>>
>>>>    QemuOptsList qemu_numa_opts = {
>>>>        .name = "numa",
>>>> @@ -315,6 +316,46 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>>>>        }
>>>>    }
>>>>
>>>> +static void numa_stat_memory_devices(uint64_t *node_mem)
>>>> +{
>>>> +    MemoryDeviceInfoList *info_list = NULL;
>>>> +    MemoryDeviceInfoList **prev = &info_list;
>>>> +    MemoryDeviceInfoList *info;
>>>> +
>>>> +    qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
>>>> +    for (info = info_list; info; info = info->next) {
>>>> +        MemoryDeviceInfo *value = info->value;
>>>> +
>>>> +        if (value) {
>>>> +            switch (value->kind) {
>>>> +            case MEMORY_DEVICE_INFO_KIND_DIMM: {
>>>
>>> I think this '{}' symbol is superfluous.
>>>
>>
>> No, without it, compiler will complain 'error: a label can only be part of
>> statement and a declaration is not a statement'.
>>
>
> I see. You just add it for the declaration of a local variable. Actually, the 'di'
> is not necessary,  How about like below?
>
> node_mem[value->dimm->node] += value->dimm->size;
>

Good idea, it will be more clean in this way, will fix in V6 Thanks.

> Anyway, your patch is ok on function. :)
>
> Best regards,
> -Gonglei
>
>
>
>
> .
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-10-17  2:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-16 12:02 [Qemu-devel] [PATCH v5] numa: make 'info numa' take into account hotplugged memory zhanghailiang
2014-10-16 12:50 ` Gonglei
2014-10-17  0:45   ` zhanghailiang
2014-10-17  1:06     ` Gonglei
2014-10-17  2:12       ` zhanghailiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).