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

On 08/03/2012 12:36 AM, Hu Tao wrote: > From: Tang Chen <tangchen(a)cn.fujitsu.com&gt; > > 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?