Re: [PATCH] qemu: Pin the emulator when only cpuset is specified

[Osier Yang <jyang@xxxxxxxxxx>]

On 2012年10月19日 11:45, 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_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;
> > }
>
> 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

Just realized this patch actually introduced regression,

Assumes XML like:

<vcpu placement="auto">10</vcpu>

Which expects the domain process is pinned to the nodeset
returned from numad, however, the qemuProcessSetEmulatorAffinites
will override the previous pinning with all available pCPUs.

Because:

>> + if (def->cputune.emulatorpin)
>> + cpumask = def->cputune.emulatorpin->cpumask;
>> + else if (def->cpumask)
>> + cpumask = def->cpumask;

Regards,
Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list