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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/03/2012 12:36 AM, Hu Tao wrote:
> From: Tang Chen <tangchen@xxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]