All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
@ 2014-10-17  8:50 zhanghailiang
  2014-10-17  9:02 ` Gonglei
  0 siblings, 1 reply; 12+ messages in thread
From: zhanghailiang @ 2014-10-17  8:50 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>
---
 v6:
- remove unnecessary 'di' variable (GongLei)
 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                  | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 44 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..ae1c7f4 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,43 @@ 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:
+                node_mem[value->dimm->node] += value->dimm->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] 12+ messages in thread

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

On 2014/10/17 16:50, 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>
> ---
>  v6:
> - remove unnecessary 'di' variable (GongLei)
>  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                  | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

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

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

Hi,

Ping...
This patch has been reviewed, please pick up, Thanks.

Best Regargs,
zhanghailiang

On 2014/10/17 17:02, Gonglei wrote:
> On 2014/10/17 16:50, 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>
>> ---
>>   v6:
>> - remove unnecessary 'di' variable (GongLei)
>>   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                  | 38 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>>
>
> Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>
> Best regards,
> -Gonglei
>
>
> .
>

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

* Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-10-23  0:32   ` zhanghailiang
@ 2014-10-27  9:40     ` zhanghailiang
  2014-10-27 12:46       ` Luiz Capitulino
  0 siblings, 1 reply; 12+ messages in thread
From: zhanghailiang @ 2014-10-27  9:40 UTC (permalink / raw)
  To: lcapitulino
  Cc: luonengjun, qemu-devel, peter.huangpeng, Gonglei, imammedo, pbonzini

Hi Luiz,

Can you apply this to your qmp branch?
It has been reviewd;)

Thanks,
zhanghailiang

On 2014/10/23 8:32, zhanghailiang wrote:
> Hi,
>
> Ping...
> This patch has been reviewed, please pick up, Thanks.
>
> Best Regargs,
> zhanghailiang
>
> On 2014/10/17 17:02, Gonglei wrote:
>> On 2014/10/17 16:50, 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>
>>> ---
>>>   v6:
>>> - remove unnecessary 'di' variable (GongLei)
>>>   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                  | 38 ++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 44 insertions(+), 1 deletion(-)
>>>
>>
>> Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>>
>> Best regards,
>> -Gonglei
>>
>>
>> .
>>
>
>
>
>
>

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

* Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-10-27  9:40     ` zhanghailiang
@ 2014-10-27 12:46       ` Luiz Capitulino
  2014-10-28  0:38         ` zhanghailiang
  2014-10-30  8:22         ` zhanghailiang
  0 siblings, 2 replies; 12+ messages in thread
From: Luiz Capitulino @ 2014-10-27 12:46 UTC (permalink / raw)
  To: zhanghailiang
  Cc: luonengjun, qemu-devel, peter.huangpeng, Gonglei, imammedo, pbonzini

On Mon, 27 Oct 2014 17:40:11 +0800
zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:

> Hi Luiz,
> 
> Can you apply this to your qmp branch?
> It has been reviewd;)

I can, but I don't have bandwidth for another pull request for v2.2 unless
it's a fix for a blocker. This means you have two options, wait for v2.3
or find another tree to merge this.

> 
> Thanks,
> zhanghailiang
> 
> On 2014/10/23 8:32, zhanghailiang wrote:
> > Hi,
> >
> > Ping...
> > This patch has been reviewed, please pick up, Thanks.
> >
> > Best Regargs,
> > zhanghailiang
> >
> > On 2014/10/17 17:02, Gonglei wrote:
> >> On 2014/10/17 16:50, 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>
> >>> ---
> >>>   v6:
> >>> - remove unnecessary 'di' variable (GongLei)
> >>>   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                  | 38 ++++++++++++++++++++++++++++++++++++++
> >>>   3 files changed, 44 insertions(+), 1 deletion(-)
> >>>
> >>
> >> Reviewed-by: Gonglei <arei.gonglei@huawei.com>
> >>
> >> Best regards,
> >> -Gonglei
> >>
> >>
> >> .
> >>
> >
> >
> >
> >
> >
> 
> 

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

* Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-10-27 12:46       ` Luiz Capitulino
@ 2014-10-28  0:38         ` zhanghailiang
  2014-10-30  8:22         ` zhanghailiang
  1 sibling, 0 replies; 12+ messages in thread
From: zhanghailiang @ 2014-10-28  0:38 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: luonengjun, qemu-devel, peter.huangpeng, Gonglei, imammedo, pbonzini

On 2014/10/27 20:46, Luiz Capitulino wrote:
> On Mon, 27 Oct 2014 17:40:11 +0800
> zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
>
>> Hi Luiz,
>>
>> Can you apply this to your qmp branch?
>> It has been reviewd;)
>
> I can, but I don't have bandwidth for another pull request for v2.2 unless
> it's a fix for a blocker. This means you have two options, wait for v2.3
> or find another tree to merge this.
>

OK, i will wait for v2.3, thanks;)

>>
>> Thanks,
>> zhanghailiang
>>
>> On 2014/10/23 8:32, zhanghailiang wrote:
>>> Hi,
>>>
>>> Ping...
>>> This patch has been reviewed, please pick up, Thanks.
>>>
>>> Best Regargs,
>>> zhanghailiang
>>>
>>> On 2014/10/17 17:02, Gonglei wrote:
>>>> On 2014/10/17 16:50, 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>
>>>>> ---
>>>>>    v6:
>>>>> - remove unnecessary 'di' variable (GongLei)
>>>>>    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                  | 38 ++++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 44 insertions(+), 1 deletion(-)
>>>>>
>>>>
>>>> Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>>>>
>>>> Best regards,
>>>> -Gonglei
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
> .
>

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

* Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-10-27 12:46       ` Luiz Capitulino
  2014-10-28  0:38         ` zhanghailiang
@ 2014-10-30  8:22         ` zhanghailiang
  2014-11-02  7:03           ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
  1 sibling, 1 reply; 12+ messages in thread
From: zhanghailiang @ 2014-10-30  8:22 UTC (permalink / raw)
  To: mjt
  Cc: qemu-trivial, luonengjun, qemu-devel, peter.huangpeng, Gonglei,
	imammedo, pbonzini, Luiz Capitulino

Hi Michael,

Can you help applying this patch to -trivial branch?
It has been reviewed, and it mainly fix bug for hmp command of 'info numa'.
Which i don't know if it should go qemu-stable, for this is not a blocker.

Maybe go trivial branch is a better choice.

Thanks,
zhanghailiang

On 2014/10/27 20:46, Luiz Capitulino wrote:
> On Mon, 27 Oct 2014 17:40:11 +0800
> zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
>
>> Hi Luiz,
>>
>> Can you apply this to your qmp branch?
>> It has been reviewd;)
>
> I can, but I don't have bandwidth for another pull request for v2.2 unless
> it's a fix for a blocker. This means you have two options, wait for v2.3
> or find another tree to merge this.
>
>>
>> Thanks,
>> zhanghailiang
>>
>> On 2014/10/23 8:32, zhanghailiang wrote:
>>> Hi,
>>>
>>> Ping...
>>> This patch has been reviewed, please pick up, Thanks.
>>>
>>> Best Regargs,
>>> zhanghailiang
>>>
>>> On 2014/10/17 17:02, Gonglei wrote:
>>>> On 2014/10/17 16:50, 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>
>>>>> ---
>>>>>    v6:
>>>>> - remove unnecessary 'di' variable (GongLei)
>>>>>    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                  | 38 ++++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 44 insertions(+), 1 deletion(-)
>>>>>
>>>>
>>>> Reviewed-by: Gonglei <arei.gonglei@huawei.com>
>>>>
>>>> Best regards,
>>>> -Gonglei
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
> .
>

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

* Re: [Qemu-devel] [Qemu-trivial] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-10-30  8:22         ` zhanghailiang
@ 2014-11-02  7:03           ` Michael Tokarev
  2014-11-03  7:56             ` Markus Armbruster
  2014-11-03 11:54             ` [Qemu-devel] [Qemu-trivial] " zhanghailiang
  0 siblings, 2 replies; 12+ messages in thread
From: Michael Tokarev @ 2014-11-02  7:03 UTC (permalink / raw)
  To: zhanghailiang
  Cc: qemu-trivial, luonengjun, qemu-devel, peter.huangpeng, Gonglei,
	pbonzini, imammedo, Luiz Capitulino

30.10.2014 11:22, zhanghailiang wrote:
> Hi Michael,
> 
> Can you help applying this patch to -trivial branch?
> It has been reviewed, and it mainly fix bug for hmp command of 'info numa'.
> Which i don't know if it should go qemu-stable, for this is not a blocker.
> 
> Maybe go trivial branch is a better choice.

And the original patch description is:

>>>>>> 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".

What does the "for now" means in this context?  Is the patch
incpmplete somehow and we should expect more code in this
area/theme?

In the patch we have:

+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;
+    }
+}

Please note that while the node_mem is a pointer, it is used as
an array.  In C, pointers and arrays in this context is the same
thing, but I think it is better to make the fact that it is an
array explicit in the function prototype, to be like this:

+void query_numa_node_mem(uint64_t node_mem[])

(But I don't know how various tools like coverity et al will react
to this.  Gcc and any other C compiler should be fine).

The same stands for other function prototype.

I'm not sure this qualifies as -trivial really.  Yes the change
does not affect anything but the `info' command, and is rather
simple, but... I'm not sure.

Thanks.

/mjt

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

* Re: [Qemu-devel] [Qemu-trivial] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-11-02  7:03           ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
@ 2014-11-03  7:56             ` Markus Armbruster
  2014-11-03  8:06               ` [Qemu-devel] " Michael Tokarev
  2014-11-03 11:54             ` [Qemu-devel] [Qemu-trivial] " zhanghailiang
  1 sibling, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2014-11-03  7:56 UTC (permalink / raw)
  To: Michael Tokarev
  Cc: zhanghailiang, qemu-trivial, luonengjun, peter.huangpeng,
	qemu-devel, Gonglei, imammedo, pbonzini, Luiz Capitulino

Michael Tokarev <mjt@tls.msk.ru> writes:

> 30.10.2014 11:22, zhanghailiang wrote:
>> Hi Michael,
>> 
>> Can you help applying this patch to -trivial branch?
>> It has been reviewed, and it mainly fix bug for hmp command of 'info numa'.
>> Which i don't know if it should go qemu-stable, for this is not a blocker.
>> 
>> Maybe go trivial branch is a better choice.
>
> And the original patch description is:
>
>>>>>>> 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".
>
> What does the "for now" means in this context?  Is the patch
> incpmplete somehow and we should expect more code in this
> area/theme?
>
> In the patch we have:
>
> +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;
> +    }
> +}
>
> Please note that while the node_mem is a pointer, it is used as
> an array.  In C, pointers and arrays in this context is the same
> thing, but I think it is better to make the fact that it is an
> array explicit in the function prototype, to be like this:
>
> +void query_numa_node_mem(uint64_t node_mem[])
>
> (But I don't know how various tools like coverity et al will react
> to this.  Gcc and any other C compiler should be fine).
>
> The same stands for other function prototype.
>
> I'm not sure this qualifies as -trivial really.  Yes the change
> does not affect anything but the `info' command, and is rather
> simple, but... I'm not sure.

Fortunately, monitor.c got a maintainer.  Luiz, would you be willing to
shepherd this patch?

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

* Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-11-03  7:56             ` Markus Armbruster
@ 2014-11-03  8:06               ` Michael Tokarev
  2014-11-03 14:39                 ` Markus Armbruster
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Tokarev @ 2014-11-03  8:06 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: zhanghailiang, qemu-trivial, luonengjun, qemu-devel,
	peter.huangpeng, Gonglei, pbonzini, imammedo, Luiz Capitulino

03.11.2014 10:56, Markus Armbruster wrote:
> Michael Tokarev <mjt@tls.msk.ru> writes:
[]
>> I'm not sure this qualifies as -trivial really.  Yes the change
>> does not affect anything but the `info' command, and is rather
>> simple, but... I'm not sure.
> 
> Fortunately, monitor.c got a maintainer.  Luiz, would you be willing to
> shepherd this patch?

He already replied to that, that's why the whole thing has been Cc'd
to -trivial:

"""I can, but I don't have bandwidth for another pull request for v2.2 unless
it's a fix for a blocker. This means you have two options, wait for v2.3
or find another tree to merge this."""

(http://patchwork.ozlabs.org/patch/400457/)

Thanks,

/mjt

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

* Re: [Qemu-devel] [Qemu-trivial] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-11-02  7:03           ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
  2014-11-03  7:56             ` Markus Armbruster
@ 2014-11-03 11:54             ` zhanghailiang
  1 sibling, 0 replies; 12+ messages in thread
From: zhanghailiang @ 2014-11-03 11:54 UTC (permalink / raw)
  To: Michael Tokarev
  Cc: qemu-trivial, luonengjun, qemu-devel, peter.huangpeng, Gonglei,
	pbonzini, imammedo, Luiz Capitulino

On 2014/11/2 15:03, Michael Tokarev wrote:
> 30.10.2014 11:22, zhanghailiang wrote:
>> Hi Michael,
>>
>> Can you help applying this patch to -trivial branch?
>> It has been reviewed, and it mainly fix bug for hmp command of 'info numa'.
>> Which i don't know if it should go qemu-stable, for this is not a blocker.
>>
>> Maybe go trivial branch is a better choice.
>
> And the original patch description is:
>
>>>>>>> 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".
>
> What does the "for now" means in this context?  Is the patch
> incpmplete somehow and we should expect more code in this
> area/theme?
>

Hmm, yes, it not exact, i should remove the confused context.

> In the patch we have:
>
> +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;
> +    }
> +}
>
> Please note that while the node_mem is a pointer, it is used as
> an array.  In C, pointers and arrays in this context is the same
> thing, but I think it is better to make the fact that it is an
> array explicit in the function prototype, to be like this:
>
> +void query_numa_node_mem(uint64_t node_mem[])
>

Good idea;), it is more clear.

> (But I don't know how various tools like coverity et al will react
> to this.  Gcc and any other C compiler should be fine).
>

I will look into this.

> The same stands for other function prototype.
>
> I'm not sure this qualifies as -trivial really.  Yes the change
> does not affect anything but the `info' command, and is rather
> simple, but... I'm not sure.
>

Hmm, this a tangled thing.:(

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

* Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory
  2014-11-03  8:06               ` [Qemu-devel] " Michael Tokarev
@ 2014-11-03 14:39                 ` Markus Armbruster
  0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2014-11-03 14:39 UTC (permalink / raw)
  To: Michael Tokarev
  Cc: zhanghailiang, qemu-trivial, luonengjun, peter.huangpeng,
	qemu-devel, Gonglei, imammedo, pbonzini, Luiz Capitulino

Michael Tokarev <mjt@tls.msk.ru> writes:

> 03.11.2014 10:56, Markus Armbruster wrote:
>> Michael Tokarev <mjt@tls.msk.ru> writes:
> []
>>> I'm not sure this qualifies as -trivial really.  Yes the change
>>> does not affect anything but the `info' command, and is rather
>>> simple, but... I'm not sure.
>> 
>> Fortunately, monitor.c got a maintainer.  Luiz, would you be willing to
>> shepherd this patch?
>
> He already replied to that, that's why the whole thing has been Cc'd
> to -trivial:
>
> """I can, but I don't have bandwidth for another pull request for v2.2 unless
> it's a fix for a blocker. This means you have two options, wait for v2.3
> or find another tree to merge this."""
>
> (http://patchwork.ozlabs.org/patch/400457/)

Right, I had already forgotten.

It's a bug fix for "info numa".  Whether the fix is important enough to
justify another pull for 2.2 is for Luiz to decide.  Whether it's
trivial enough for you to take it via -trivial is for you to decide.  If
you both say no, it goes into 2.3 via Luiz.  Not exactly terrible.

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

end of thread, other threads:[~2014-11-03 14:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-17  8:50 [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory zhanghailiang
2014-10-17  9:02 ` Gonglei
2014-10-23  0:32   ` zhanghailiang
2014-10-27  9:40     ` zhanghailiang
2014-10-27 12:46       ` Luiz Capitulino
2014-10-28  0:38         ` zhanghailiang
2014-10-30  8:22         ` zhanghailiang
2014-11-02  7:03           ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2014-11-03  7:56             ` Markus Armbruster
2014-11-03  8:06               ` [Qemu-devel] " Michael Tokarev
2014-11-03 14:39                 ` Markus Armbruster
2014-11-03 11:54             ` [Qemu-devel] [Qemu-trivial] " zhanghailiang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.