Re: [libvirt] [PATCH 2/3] qemu: Keep the affinity when creating cgroup for emulator thread

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);