Skip to contentSkip to conversation

numa: report all DIMM/NVDIMMs as plugged memory

PATCH Superseded David Hildenbrand 2 participants 5 messages Jun 22, 2018 – Jun 28, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

David Hildenbrand

author
#0 lore raw

PATCH 1/1 numa: report all DIMM/NVDIMMs as plugged memory

Right now, there is some inconsistency between hotplugged and coldplugged memory. DIMMs added via "-device" result in different stats than DIMMs added using "device_add".

E.g.

    [...]
    -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
    -m 4G,maxmem=20G,slots=2 \
    -object memory-backend-ram,id=mem0,size=8G \
    -device pc-dimm,id=dimm0,memdev=mem0 \
    -object memory-backend-ram,id=mem1,size=8G \
    -device nvdimm,id=dimm1,memdev=mem1,node=1

Results in NUMA info
(qemu) info numa
info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 10240 MB
node 0 plugged: 0 MB
node 1 cpus: 2 3
node 1 size: 10240 MB
node 1 plugged: 0 MB

But in memory size summary:

    (qemu) info memory_size_summary
    info memory_size_summary
    base memory: 4294967296
    plugged memory: 17179869184

Make this consistent by reporting all hot and coldplugged memory a.k.a. DIMM and NVDIMM as "plugged".

Fixes 31959e82fb0 ("hmp: extend "info numa" with hotplugged memory information")
Signed-off-by David Hildenbrand <[email protected]>
---
 numa.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
numa.c +2 -4
index 94f758c757..5f6367b989 100644
@@ -566,10 +566,8 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])
 
             if (pcdimm_info) {
                 node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
-                if (pcdimm_info->hotpluggable && pcdimm_info->hotplugged) {
-                    node_mem[pcdimm_info->node].node_plugged_mem +=
-                        pcdimm_info->size;
-                }
+                node_mem[pcdimm_info->node].node_plugged_mem +=
+                    pcdimm_info->size;
             }
         }
     }
Signature · 1 line
2.17.1
#1 lore raw
Quoted 23 lines · On Fri, 22 Jun 2018 16:40:45 +0200 David Hildenbrand <[email protected]> wrote:

Right now, there is some inconsistency between hotplugged and coldplugged memory. DIMMs added via "-device" result in different stats than DIMMs added using "device_add".

E.g.

    [...]
    -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
    -m 4G,maxmem=20G,slots=2 \
    -object memory-backend-ram,id=mem0,size=8G \
    -device pc-dimm,id=dimm0,memdev=mem0 \
    -object memory-backend-ram,id=mem1,size=8G \
    -device nvdimm,id=dimm1,memdev=mem1,node=1

Results in NUMA info
(qemu) info numa
info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 10240 MB
node 0 plugged: 0 MB
node 1 cpus: 2 3
node 1 size: 10240 MB
node 1 plugged: 0 MB

...

Quoted 5 lines

But in memory size summary:

    (qemu) info memory_size_summary
    info memory_size_summary
    base memory: 4294967296
    plugged memory: 17179869184

this looks like right amount wrt used CLI, is this supposed to be correct output or broken one? ('but' implies incorrect one)

Quoted 27 lines

Make this consistent by reporting all hot and coldplugged memory a.k.a. DIMM and NVDIMM as "plugged".

Fixes 31959e82fb0 ("hmp: extend "info numa" with hotplugged memory information")
Signed-off-by David Hildenbrand <[email protected]>
---
 numa.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
numa.c +2 -4
index 94f758c757..5f6367b989 100644
@@ -566,10 +566,8 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])
 
             if (pcdimm_info) {
                 node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
-                if (pcdimm_info->hotpluggable && pcdimm_info->hotplugged) {
-                    node_mem[pcdimm_info->node].node_plugged_mem +=
-                        pcdimm_info->size;
-                }
+                node_mem[pcdimm_info->node].node_plugged_mem +=
+                    pcdimm_info->size;
             }
         }
     }
author
#2 lore raw
Quoted 27 lines · Igor Mammedov
Quoted 23 lines · On Fri, 22 Jun 2018 16:40:45 +0200 David Hildenbrand <[email protected]> wrote:

Right now, there is some inconsistency between hotplugged and coldplugged memory. DIMMs added via "-device" result in different stats than DIMMs added using "device_add".

E.g.

    [...]
    -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
    -m 4G,maxmem=20G,slots=2 \
    -object memory-backend-ram,id=mem0,size=8G \
    -device pc-dimm,id=dimm0,memdev=mem0 \
    -object memory-backend-ram,id=mem1,size=8G \
    -device nvdimm,id=dimm1,memdev=mem1,node=1

Results in NUMA info
(qemu) info numa
info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 10240 MB
node 0 plugged: 0 MB
node 1 cpus: 2 3
node 1 size: 10240 MB
node 1 plugged: 0 MB

...

Quoted 8 lines
Quoted 5 lines

But in memory size summary:

    (qemu) info memory_size_summary
    info memory_size_summary
    base memory: 4294967296
    plugged memory: 17179869184

this looks like right amount wrt used CLI, is this supposed to be correct output or broken one? ('but' implies incorrect one)

'but' just highlights the inconsistency here. As explained above, all dimms should be reported as plugged (hot and coldplugged).

So in my opinion "memory size summary" does the right thing right now.

Quoted 29 lines
Quoted 28 lines

Make this consistent by reporting all hot and coldplugged memory a.k.a. DIMM and NVDIMM as "plugged".

Fixes 31959e82fb0 ("hmp: extend "info numa" with hotplugged memory information")
Signed-off-by David Hildenbrand <[email protected]>
---
 numa.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
numa.c +2 -4
index 94f758c757..5f6367b989 100644
@@ -566,10 +566,8 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])
 
             if (pcdimm_info) {
                 node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
-                if (pcdimm_info->hotpluggable && pcdimm_info->hotplugged) {
-                    node_mem[pcdimm_info->node].node_plugged_mem +=
-                        pcdimm_info->size;
-                }
+                node_mem[pcdimm_info->node].node_plugged_mem +=
+                    pcdimm_info->size;
             }
         }
     }
Signature · 3 lines
Thanks,

David / dhildenb
#3 lore raw
Quoted 33 lines · On Wed, 27 Jun 2018 15:38:23 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 32 lines · Igor Mammedov
Quoted 23 lines · On Fri, 22 Jun 2018 16:40:45 +0200 David Hildenbrand <[email protected]> wrote:

Right now, there is some inconsistency between hotplugged and coldplugged memory. DIMMs added via "-device" result in different stats than DIMMs added using "device_add".

E.g.

    [...]
    -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
    -m 4G,maxmem=20G,slots=2 \
    -object memory-backend-ram,id=mem0,size=8G \
    -device pc-dimm,id=dimm0,memdev=mem0 \
    -object memory-backend-ram,id=mem1,size=8G \
    -device nvdimm,id=dimm1,memdev=mem1,node=1

Results in NUMA info
(qemu) info numa
info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 10240 MB
node 0 plugged: 0 MB
node 1 cpus: 2 3
node 1 size: 10240 MB
node 1 plugged: 0 MB

...

Quoted 5 lines

But in memory size summary:

    (qemu) info memory_size_summary
    info memory_size_summary
    base memory: 4294967296
    plugged memory: 17179869184
Quoted 8 lines
Quoted 3 lines

this looks like right amount wrt used CLI, is this supposed to be correct output or broken one? ('but' implies incorrect one)

'but' just highlights the inconsistency here. As explained above, all dimms should be reported as plugged (hot and coldplugged).

So in my opinion "memory size summary" does the right thing right now.

maybe add to commit message that says what exactly is being fixed, otherwise it's not clear what is expected/wrong beside of output being inconsistent.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Make this consistent by reporting all hot and coldplugged memory a.k.a. DIMM and NVDIMM as "plugged".

maybe mention 'info numa' and maybe QMP equivalent if it exists,
otherwise above text is a bit vague for bystander (could be /me|you in a day-month-year)

Quoted 27 lines
Quoted 25 lines
Quoted 24 lines
Fixes 31959e82fb0 ("hmp: extend "info numa" with hotplugged memory information")
Signed-off-by David Hildenbrand <[email protected]>
---
 numa.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
numa.c +2 -4
index 94f758c757..5f6367b989 100644
@@ -566,10 +566,8 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])
 
             if (pcdimm_info) {
                 node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
-                if (pcdimm_info->hotpluggable && pcdimm_info->hotplugged) {
-                    node_mem[pcdimm_info->node].node_plugged_mem +=
-                        pcdimm_info->size;
-                }
+                node_mem[pcdimm_info->node].node_plugged_mem +=
+                    pcdimm_info->size;
             }
         }
     }
author
#4 lore raw
Quoted 40 lines · Igor Mammedov
Quoted 37 lines · On Wed, 27 Jun 2018 15:38:23 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 35 lines · Igor Mammedov
Quoted 23 lines · On Fri, 22 Jun 2018 16:40:45 +0200 David Hildenbrand <[email protected]> wrote:

Right now, there is some inconsistency between hotplugged and coldplugged memory. DIMMs added via "-device" result in different stats than DIMMs added using "device_add".

E.g.

    [...]
    -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
    -m 4G,maxmem=20G,slots=2 \
    -object memory-backend-ram,id=mem0,size=8G \
    -device pc-dimm,id=dimm0,memdev=mem0 \
    -object memory-backend-ram,id=mem1,size=8G \
    -device nvdimm,id=dimm1,memdev=mem1,node=1

Results in NUMA info
(qemu) info numa
info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 10240 MB
node 0 plugged: 0 MB
node 1 cpus: 2 3
node 1 size: 10240 MB
node 1 plugged: 0 MB

...

Quoted 5 lines

But in memory size summary:

    (qemu) info memory_size_summary
    info memory_size_summary
    base memory: 4294967296
    plugged memory: 17179869184

this looks like right amount wrt used CLI, is this supposed to be correct output or broken one? ('but' implies incorrect one)

Quoted 6 lines
Quoted 4 lines

'but' just highlights the inconsistency here. As explained above, all dimms should be reported as plugged (hot and coldplugged).

So in my opinion "memory size summary" does the right thing right now.

maybe add to commit message that says what exactly is being fixed, otherwise it's not clear what is expected/wrong beside of output being inconsistent.

I will add some more details.

Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

Make this consistent by reporting all hot and coldplugged memory a.k.a. DIMM and NVDIMM as "plugged".

maybe mention 'info numa' and maybe QMP equivalent if it exists,
otherwise above text is a bit vague for bystander (could be /me|you in a day-month-year)

Yes, will do, thanks!

Signature · 3 lines
Thanks,

David / dhildenb