4:10 p.m.
If there's a domain configured as:
<currentMemory unit='MiB'>4096</currentMemory>
<numatune>
<memory mode='strict' nodeset='1'/>
</numatune>
but there is not enough memory on NUMA node 1 the domain will start
successfully because we allow it to allocate memory from other nodes.
This is a result of some previous fix (v1.2.7-rc1~91). However, I've
tested my fix successfully on a NUMA machine with F29 and recent kernel.
So the kernel bug I'm mentioning in 4/4 is probably fixed then and we
can drop the workaround.
Michal Prívozník (4):
qemuSetupCpusetMems: Use VIR_AUTOFREE()
qemuSetupCpusetMems: Create EMULATOR thread upfront
qemu_cgroup: Make qemuSetupCpusetMems static
qemuSetupCpusetCgroup: Set up cpuset.mems before execing qemu
src/qemu/qemu_cgroup.c | 12 +++++++-----
src/qemu/qemu_cgroup.h | 1 -
2 files changed, 7 insertions(+), 6 deletions(-)
--
2.21.0
4:10 p.m.
New subject: [libvirt] [PATCH 1/4] qemuSetupCpusetMems: Use VIR_AUTOFREE()
There is one string that can be VIR_AUTOFREE used on.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index c23f0af2aa..689e0839cd 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -830,7 +830,7 @@ qemuSetupCpusetMems(virDomainObjPtr vm)
virCgroupPtr cgroup_temp = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainNumatuneMemMode mode;
- char *mem_mask = NULL;
+ VIR_AUTOFREE(char *) mem_mask = NULL;
int ret = -1;
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
@@ -843,7 +843,7 @@ qemuSetupCpusetMems(virDomainObjPtr vm)
if (virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
priv->autoNodeset,
&mem_mask, -1) < 0)
- goto cleanup;
+ return -1;
if (mem_mask)
if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
@@ -853,7 +853,6 @@ qemuSetupCpusetMems(virDomainObjPtr vm)
ret = 0;
cleanup:
- VIR_FREE(mem_mask);
virCgroupFree(&cgroup_temp);
return ret;
}
--
2.21.0
4:10 p.m.
New subject: [libvirt] [PATCH 2/4] qemuSetupCpusetMems: Create EMULATOR thread upfront
This function is going to be called after fork() and before
exec() of qemu to ensure that 'strict' mode of numatune is
honored. This point is far before qemuProcessSetupEmulator() is
called and thus CGroup for EMULATOR thread does not exist yet.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 689e0839cd..2c663aca0a 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -847,7 +847,7 @@ qemuSetupCpusetMems(virDomainObjPtr vm)
if (mem_mask)
if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
- false, &cgroup_temp) < 0 ||
+ true, &cgroup_temp) < 0 ||
virCgroupSetCpusetMems(cgroup_temp, mem_mask) < 0)
goto cleanup;
--
2.21.0
4:10 p.m.
New subject: [libvirt] [PATCH 3/4] qemu_cgroup: Make qemuSetupCpusetMems static
This function is not used (nor going to be used) outside of
qemu_cgroup.c file. Mark it as static.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_cgroup.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2c663aca0a..b2d300b419 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -824,7 +824,7 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
}
-int
+static int
qemuSetupCpusetMems(virDomainObjPtr vm)
{
virCgroupPtr cgroup_temp = NULL;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index dc6d173fce..88ad4a38b6 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -61,7 +61,6 @@ int qemuConnectCgroup(virDomainObjPtr vm);
int qemuSetupCgroup(virDomainObjPtr vm,
size_t nnicindexes,
int *nicindexes);
-int qemuSetupCpusetMems(virDomainObjPtr vm);
int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota);
--
2.21.0
4:10 p.m.
New subject: [libvirt] [PATCH 4/4] qemuSetupCpusetCgroup: Set up cpuset.mems before execing qemu
It's funny how this went unnoticed for such a long time. Long
story short, if a domain is configured with
VIR_DOMAIN_NUMATUNE_MEM_STRICT libvirt doesn't really honour
that. This is because of 7e72ac787848 after which libvirt allowed
qemu to allocate memory just anywhere and only after that it used
some magic involving cpuset.memory_migrate and cpuset.mems to
move the memory to desired NUMA nodes. This was done in order to
work around some KVM bug where KVM would fail if there wasn't a
DMA zone available on the NUMA node. Well, while the work around
might stopped libvirt tickling the KVM bug it also caused a bug
on libvirt side: if there is not enough memory on configured NUMA
node(s) then any attempt to start a domain must fail. Because of
the way we play with guest memory domains can start just happily.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index b2d300b419..d6c982aeed 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -869,6 +869,9 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm)
if (virCgroupSetCpusetMemoryMigrate(priv->cgroup, true) < 0)
return -1;
+ if (qemuSetupCpusetMems(vm) < 0)
+ return -1;
+
return 0;
}
--
2.21.0
12:35 a.m.
Hi,
On 4/9/19 11:10 AM, Michal Privoznik wrote:
If there's a domain configured as:
<currentMemory unit='MiB'>4096</currentMemory>
<numatune>
<memory mode='strict' nodeset='1'/>
</numatune>
but there is not enough memory on NUMA node 1 the domain will start
successfully because we allow it to allocate memory from other nodes.
This is a result of some previous fix (v1.2.7-rc1~91). However, I've
tested my fix successfully on a NUMA machine with F29 and recent kernel.
So the kernel bug I'm mentioning in 4/4 is probably fixed then and we
can drop the workaround.
I've tested out of curiosity your patch set in a Power8 system to see
if I could spot a difference, but in my case it didn't change the behavior.
I've tried a guest with the following numatune:
<memory unit='KiB'>67108864</memory>
<currentMemory unit='KiB'>67108864</currentMemory>
<vcpu placement='static' current='4'>16</vcpu>
<numatune>
<memory mode='strict' nodeset='0'/>
</numatune>
This is the numa setup of the host:
$ numactl -H
available: 4 nodes (0-1,16-17)
node 0 cpus: 0 8 16 24 32 40
node 0 size: 32606 MB
node 0 free: 24125 MB
node 1 cpus: 48 56 64 72 80 88
node 1 size: 32704 MB
node 1 free: 27657 MB
node 16 cpus: 96 104 112 120 128 136
node 16 size: 32704 MB
node 16 free: 25455 MB
node 17 cpus: 144 152 160 168 176 184
node 17 size: 32565 MB
node 17 free: 30030 MB
node distances:
node 0 1 16 17
0: 10 20 40 40
1: 20 10 40 40
16: 40 40 10 20
17: 40 40 20 10
If I understood it right, the patches removed the capability to allocate
memory from different numa nodes with the 'strict' setting, making the
guest failing to launch if the numa node does not have enough memory.
Unless I am getting something wrong, this guest shouldn't launch after
applying this patch (node 0 does not have 64Gb available). But the guest
is launching as if nothing changed.
I'll dig it further if I have the chance. I'm just curious if this is
something
that works differently with pseries guests.
Thanks,
DHB
Michal Prívozník (4):
qemuSetupCpusetMems: Use VIR_AUTOFREE()
qemuSetupCpusetMems: Create EMULATOR thread upfront
qemu_cgroup: Make qemuSetupCpusetMems static
qemuSetupCpusetCgroup: Set up cpuset.mems before execing qemu
src/qemu/qemu_cgroup.c | 12 +++++++-----
src/qemu/qemu_cgroup.h | 1 -
2 files changed, 7 insertions(+), 6 deletions(-)
3:14 p.m.
On 4/10/19 12:35 AM, Daniel Henrique Barboza wrote:
Hi,
On 4/9/19 11:10 AM, Michal Privoznik wrote:
> If there's a domain configured as:
>
> <currentMemory unit='MiB'>4096</currentMemory>
> <numatune>
> <memory mode='strict' nodeset='1'/>
> </numatune>
>
> but there is not enough memory on NUMA node 1 the domain will start
> successfully because we allow it to allocate memory from other nodes.
> This is a result of some previous fix (v1.2.7-rc1~91). However, I've
> tested my fix successfully on a NUMA machine with F29 and recent kernel.
> So the kernel bug I'm mentioning in 4/4 is probably fixed then and we
> can drop the workaround.
I've tested out of curiosity your patch set in a Power8 system to see
if I could spot a difference, but in my case it didn't change the behavior.
I've tried a guest with the following numatune:
<memory unit='KiB'>67108864</memory>
<currentMemory unit='KiB'>67108864</currentMemory>
<vcpu placement='static' current='4'>16</vcpu>
<numatune>
<memory mode='strict' nodeset='0'/>
</numatune>
This is the numa setup of the host:
$ numactl -H
available: 4 nodes (0-1,16-17)
node 0 cpus: 0 8 16 24 32 40
node 0 size: 32606 MB
node 0 free: 24125 MB
node 1 cpus: 48 56 64 72 80 88
node 1 size: 32704 MB
node 1 free: 27657 MB
node 16 cpus: 96 104 112 120 128 136
node 16 size: 32704 MB
node 16 free: 25455 MB
node 17 cpus: 144 152 160 168 176 184
node 17 size: 32565 MB
node 17 free: 30030 MB
node distances:
node 0 1 16 17
0: 10 20 40 40
1: 20 10 40 40
16: 40 40 10 20
17: 40 40 20 10
If I understood it right, the patches removed the capability to allocate
memory from different numa nodes with the 'strict' setting, making the
guest failing to launch if the numa node does not have enough memory.
Unless I am getting something wrong, this guest shouldn't launch after
applying this patch (node 0 does not have 64Gb available). But the guest
is launching as if nothing changed.
I'll dig it further if I have the chance. I'm just curious if this is
something
that works differently with pseries guests.
Hey,
firstly thanks for testing this! And yes, it shows flaw in my patches.
Thing is, my patches set up emulator/cpuset.mems but at the time qemu is
doing its allocation it is still living under top level CGroup (which is
left untouched). Will post v2! Thanks.
Michal
1989
days inactive
1990
days old
6 comments
2 participants
participants (2)
-
Daniel Henrique Barboza -
Michal Privoznik