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

Re: [libvirt] [PATCH v3] qemu: Support numad



On 03/08/2012 06:36 AM, Osier Yang wrote:
> numad is an user-level daemon that monitors NUMA topology and
> processes resource consumption to facilitate good NUMA resource
> alignment of applications/virtual machines to improve performance
> and minimize cost of remote memory latencies. It provides a
> pre-placement advisory interface, so significant processes can
> be pre-bound to nodes with sufficient available resources.

Better late than never, but here's a few things you might want to tweak:


> +#if defined(NUMAD)
> +static char *
> +qemuGetNumadAdvice(virDomainDefPtr def)
> +{
> +    virCommandPtr cmd = NULL;
> +    char *args = NULL;
> +    char *output = NULL;
> +
> +    if (virAsprintf(&args, "%d:%lu", def->vcpus, def->mem.cur_balloon) < 0) {
> +        virReportOOMError();
> +        goto out;
> +    }

This prints into a temporary,

> +    cmd = virCommandNewArgList(NUMAD, "-w", args, NULL);

only to then copy it to cmd and have to free the temporary.  It's fewer
lines of code, and less malloc pressure, to avoid the temporary by
printing directly into cmd:

cmd = virCommandNewArgList(NUMAD, "-w", NULL);
virCommandAddArgFormat(&cmd, "%d:%llu", def->vcpus, def->mem.cur_balloon);

> +        if (vm->def->cpumask) {
> +            /* XXX why don't we keep 'cpumask' in the libvirt cpumap
> +             * format to start with ?!?! */
> +            for (i = 0 ; i < maxcpu && i < vm->def->cpumasklen ; i++)
> +                if (vm->def->cpumask[i])
> +                    VIR_USE_CPU(cpumap, i);

This comment just verbalizes what has already been bothering me.  It
would be really nice if someone could convert all of our internal uses
of cpumask (with its inefficient one-byte-per-cpu representation) into
virBitmask(), as well as making virBitmask provide convenience functions
for converting to and from our stringized format as well as from the
public api cpumap (8 cpus per byte).

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

Attachment: signature.asc
Description: OpenPGP digital signature


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