According to our recent changes (clarifications), we should be pinning qemu's emulator processes using the<vcpu> 'cpuset' attribute in case there is no<emulatorpin> specified. This however doesn't work entirely as expected and this patch should resolve all the remaining issues. --- src/qemu/qemu_cgroup.c | 25 ++++++++++++++++--------- src/qemu/qemu_cgroup.h | 4 ++-- src/qemu/qemu_driver.c | 3 ++- src/qemu/qemu_process.c | 16 ++++++++-------- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 166f9b9..6b94686 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -501,7 +501,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, for (i = 0; i< nvcpupin; i++) { if (vcpuid == vcpupin[i]->vcpuid) { - return qemuSetupCgroupEmulatorPin(cgroup, vcpupin[i]); + return qemuSetupCgroupEmulatorPin(cgroup, vcpupin[i]->cpumask); } } @@ -509,12 +509,12 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, } int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, - virDomainVcpuPinDefPtr vcpupin) + virBitmapPtr cpumask) { int rc = 0; char *new_cpus = NULL; - new_cpus = virBitmapFormat(vcpupin->cpumask); + new_cpus = virBitmapFormat(cpumask); if (!new_cpus) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to convert cpu mask")); @@ -643,6 +643,7 @@ cleanup: int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virDomainObjPtr vm) { + virBitmapPtr cpumask = NULL; virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_emulator = NULL; virDomainDefPtr def = vm->def; @@ -690,12 +691,18 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, } } - if (def->cputune.emulatorpin&& - qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { - rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, - def->cputune.emulatorpin); - if (rc< 0) - goto cleanup; + if (def->cputune.emulatorpin) + cpumask = def->cputune.emulatorpin->cpumask; + else if (def->cpumask) + cpumask = def->cpumask; + + if (cpumask) { + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, cpumask); + if (rc< 0) + goto cleanup; + } + cpumask = NULL; /* sanity */ } if (period || quota) { diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index b7b0211..362080a 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -1,7 +1,7 @@ /* * qemu_cgroup.h: QEMU cgroup management * - * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -57,7 +57,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, int nvcpupin, int vcpuid); -int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr vcpupin); +int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virBitmapPtr cpumask); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fa37bfd..adfbfa6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4245,7 +4245,8 @@ qemudDomainPinEmulator(virDomainPtr dom, if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup_dom, 0) == 0) { if (virCgroupForEmulator(cgroup_dom,&cgroup_emulator, 0) == 0) { - if (qemuSetupCgroupEmulatorPin(cgroup_emulator, newVcpuPin[0])< 0) { + if (qemuSetupCgroupEmulatorPin(cgroup_emulator, + newVcpuPin[0]->cpumask)< 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("failed to set cpuset.cpus in cgroup" " for emulator threads")); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a94e9c4..e08ec67 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2024,11 +2024,12 @@ cleanup: return ret; } -/* Set CPU affinities for emulator threads if emulatorpin xml provided. */ +/* Set CPU affinities for emulator threads. */ static int qemuProcessSetEmulatorAffinites(virConnectPtr conn, virDomainObjPtr vm) { + virBitmapPtr cpumask; virDomainDefPtr def = vm->def; virNodeInfo nodeinfo; int ret = -1; @@ -2036,15 +2037,14 @@ qemuProcessSetEmulatorAffinites(virConnectPtr conn, if (virNodeGetInfo(conn,&nodeinfo) != 0) return -1; - if (!def->cputune.emulatorpin) - return 0; - - if (virProcessInfoSetAffinity(vm->pid, - def->cputune.emulatorpin->cpumask)< 0) { + if (def->cputune.emulatorpin) + cpumask = def->cputune.emulatorpin->cpumask; + else if (def->cpumask) + cpumask = def->cpumask; + else goto cleanup; - } - ret = 0; + ret = virProcessInfoSetAffinity(vm->pid, cpumask); cleanup: return ret; }

On 10/19/2012 05:45 AM, Osier Yang wrote: > On 2012年10月17日 23:38, Martin Kletzander wrote: >> According to our recent changes (clarifications), we should be pinning >> qemu's emulator processes using the<vcpu> 'cpuset' attribute in case >> there is no<emulatorpin> specified. This however doesn't work >> entirely as expected and this patch should resolve all the remaining >> issues. >> --- >> src/qemu/qemu_cgroup.c | 25 ++++++++++++++++--------- >> src/qemu/qemu_cgroup.h | 4 ++-- >> src/qemu/qemu_driver.c | 3 ++- >> src/qemu/qemu_process.c | 16 ++++++++-------- >> 4 files changed, 28 insertions(+), 20 deletions(-) >> [...] >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index a94e9c4..e08ec67 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -2024,11 +2024,12 @@ cleanup: >> return ret; >> } >> >> -/* Set CPU affinities for emulator threads if emulatorpin xml >> provided. */ >> +/* Set CPU affinities for emulator threads. */ >> static int >> qemuProcessSetEmulatorAffinites(virConnectPtr conn, >> virDomainObjPtr vm) >> { >> + virBitmapPtr cpumask; >> virDomainDefPtr def = vm->def; >> virNodeInfo nodeinfo; >> int ret = -1; >> @@ -2036,15 +2037,14 @@ qemuProcessSetEmulatorAffinites(virConnectPtr >> conn, >> if (virNodeGetInfo(conn,&nodeinfo) != 0) >> return -1; >> >> - if (!def->cputune.emulatorpin) >> - return 0; >> - >> - if (virProcessInfoSetAffinity(vm->pid, >> - >> def->cputune.emulatorpin->cpumask)< 0) { >> + if (def->cputune.emulatorpin) >> + cpumask = def->cputune.emulatorpin->cpumask; >> + else if (def->cpumask) >> + cpumask = def->cpumask; >> + else >> goto cleanup; >> - } >> >> - ret = 0; >> + ret = virProcessInfoSetAffinity(vm->pid, cpumask); >> cleanup: >> return ret; >> } > > This patch actually duplicates the affinity setting on domain > process with sched_setaffinity. > > Assume "cpuset" of<vcpu> is specified, and no "<emulatorpin>". > > qemuProcessInitCpuAffinity is excuted between the fork > and exec, with this patch, qemuProcessSetEmulatorAffinites > will set the affinity again using sched_setaffinity. > > Cgroup setting is fine though, as there is no previous setting > with cgroup. > > I'm wondering why the domain process is not pinned to > pCPUs specified "cpuset" of "<vcpu>" by qemuProcessInitCpuAffinity, > when<emulatorpin> is not specified. > > Regards, > Osier I'm glad you pointed it out. To be honest, it took me quite a while to see where this should be set to fix the problem with as few lines as possible. As Peter says, there is a mess with the pinning, cgroups and affinities.