[libvirt] [PATCH 07/17] Add qemuProcessSetHypervisorAffinites and set hypervisor threads affinities

[Eric Blake]

On 08/03/2012 12:36 AM, Hu Tao wrote:
> From: Tang Chen <tangchen at cn.fujitsu.com>
> 
> Hypervisor threads should also be pinned by sched_setaffinity(), just
> the same as vcpu threads.

Indeed, this fallback makes sense when cpuset cgroup is not present.

Same question as earlier in the series - is affinity necessary when
cpuset is in effect, or does cpuset already guarantee everything that
affinity would already provide?


> +static int
> +qemuProcessSetHypervisorAffinites(virConnectPtr conn,
> +                                  virDomainObjPtr vm)
> +{
> +    virDomainDefPtr def = vm->def;
> +    pid_t pid = vm->pid;
> +    unsigned char *cpumask = NULL;
> +    unsigned char *cpumap = NULL;
> +    virNodeInfo nodeinfo;
> +    int cpumaplen, hostcpus, maxcpu, i;
> +    int ret = -1;
> +
> +    if (virNodeGetInfo(conn, &nodeinfo) != 0)
> +        return -1;
> +
> +    if (!def->cputune.hypervisorpin)
> +        return 0;
> +
> +    hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
> +    cpumaplen = VIR_CPU_MAPLEN(hostcpus);
> +    maxcpu = cpumaplen * 8;

That's a magic number; I prefer CHAR_BIT from <limits.h>.

Not your fault, but I really hate the amount of gross code duplication
we have for dealing with conversions between strings, cpu maps, and
bitmaps.  It would be really nice to scrub the code to make nice helper
functions that can provide easier conversion interfaces rather than open
coding the conversion at each client.

> +
> +    if (virProcessInfoSetAffinity(pid,
> +                                  cpumap,
> +                                  cpumaplen,
> +                                  maxcpu) < 0) {

Does this work, or does it slam the affinity of all the vcpu children
threads at the same time, even when we have used <vcpupin> to request
different pinning?

> @@ -3765,6 +3815,10 @@ int qemuProcessStart(virConnectPtr conn,
>      if (qemuProcessSetVcpuAffinites(conn, vm) < 0)
>          goto cleanup;
>  
> +    VIR_DEBUG("Setting hypervisor threads affinities");

Reads awkwardly; maybe 'setting affinity of hypervisor threads'

But the basic idea still looks sound, and if we save the conversion
cleanup for later patches, then we're down to nits on this patch.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120806/d73964c6/attachment-0001.sig>