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 :)
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list