On 2012年10月24日 21:36, Martin Kletzander wrote:
On 10/24/2012 03:29 PM, Osier Yang wrote:
On 2012年10月24日 21:17, Martin Kletzander wrote:
On 10/24/2012 12:00 PM, Osier Yang wrote:
When the cpu placement model is "auto", it sets the affinity for
domain process with the advisory nodeset from numad, however,
creating cgroup for the domain process (called emulator thread
in some contexts) later overrides that with pinning it to all
available pCPUs.
How to reproduce:
* Configure the domain with "auto" placement for<vcpu>, e.g.
<vcpu placement='auto'>4</vcpu>
* % virsh start dom
* % cat /proc/$dompid/status
Though the emulator cgroup cause conflicts, but we can't simply
prohibit creating it, as other tunables are still useful, such
as "emulator_period", which is used by API
virDomainSetSchedulerParameter. So this patch doesn't prohibit
creating the emulator cgroup, but inherit the nodeset from numad,
and reset the affinity for domain process.
* src/qemu/qemu_cgroup.h: Modify definition of
qemuSetupCgroupForEmulator
to accept the passed nodenet
* src/qemu/qemu_cgroup.c: Set the affinity with the passed nodeset
---
src/qemu/qemu_cgroup.c | 17 ++++++++++++++---
src/qemu/qemu_cgroup.h | 3 ++-
src/qemu/qemu_process.c | 2 +-
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index db371a0..8e99c01 100644
@@ -698,6 +706,9 @@ int qemuSetupCgroupForEmulator(struct
qemud_driver *driver,
if (rc< 0)
goto cleanup;
In case you go to the cleanup here, cpumask is not free()'d.
Ah, okay.
I will squash the following in:
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1754d6f..8f8ef08 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -706,9 +706,6 @@ int qemuSetupCgroupForEmulator(struct qemud_driver
*driver,
if (rc< 0)
goto cleanup;
}
-
- if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
- virBitmapFree(cpumask);
cpumask = NULL; /* sanity */
}
@@ -725,6 +722,8 @@ int qemuSetupCgroupForEmulator(struct qemud_driver
*driver,
return 0;
cleanup:
+ virBitmapFree(cpumap);
+
if (cgroup_emulator) {
virCgroupRemove(cgroup_emulator);
virCgroupFree(&cgroup_emulator);
Not quite the right thing to do, cleanup is not done if it is
successful, you have to _add_ it to cleanup or reorganize it :)
Sigh, though rushing is always not good, but I will rush again.
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1754d6f..5ce748a 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -706,9 +706,6 @@ int qemuSetupCgroupForEmulator(struct qemud_driver
*driver,
if (rc < 0)
goto cleanup;
}
-
- if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
- virBitmapFree(cpumask);
cpumask = NULL; /* sanity */
}
@@ -722,9 +719,12 @@ int qemuSetupCgroupForEmulator(struct qemud_driver
*driver,
virCgroupFree(&cgroup_emulator);
virCgroupFree(&cgroup);
+ virBitmapFree(cpumap);
return 0;
cleanup:
+ virBitmapFree(cpumap);
+
if (cgroup_emulator) {
virCgroupRemove(cgroup_emulator);
virCgroupFree(&cgroup_emulator);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list