Re: [PATCH 3 of 8] xen: let the (credit) scheduler know about `node affinity`

[Juergen Gross <juergen.gross@ts.fujitsu.com>]

Am 05.10.2012 16:08, schrieb Dario Faggioli:
> As vcpu-affinity tells where vcpus can run, node-affinity tells
> where a domain's vcpus prefer to run. Respecting vcpu-affinity is
> the primary concern, but honouring node-affinity will likely
> result in some performances benefit.
>
> This change modifies the vcpu load balancing algorithm (for the
> credit scheduler only), introducing a two steps logic.
> During the first step, we use the node-affinity mask. The aim is
> giving precedence to the CPUs where it is known to be preferrable
> for the domain to run. If that fails in finding a valid CPU, the
> node-affinity is just ignored and, in the second step, we fall
> back to using cpu-affinity only.
>
> Signed-off-by: Dario Faggioli<dario.faggioli@citrix.com>
>
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
...
>   static int
>   _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit)
>   {
> -    cpumask_t cpus;
> +    cpumask_t cpus, start_cpus;
>       cpumask_t idlers;
>       cpumask_t *online;
> +    struct csched_dom *sdom = CSCHED_DOM(vc->domain);
>       struct csched_pcpu *spc = NULL;
>       int cpu;
>
>       /*
> -     * Pick from online CPUs in VCPU's affinity mask, giving a
> -     * preference to its current processor if it's in there.
> +     * Pick an online CPU from the&&  of vcpu-affinity and node-affinity
> +     * masks (if not empty, in which case only the vcpu-affinity mask is
> +     * used). Also, try to give a preference to its current processor if
> +     * it's in there.
>        */
>       online = cpupool_scheduler_cpumask(vc->domain->cpupool);
>       cpumask_and(&cpus, online, vc->cpu_affinity);
> -    cpu = cpumask_test_cpu(vc->processor,&cpus)
> +    cpumask_and(&start_cpus,&cpus, sdom->node_affinity_cpumask);
> +    if ( unlikely(cpumask_empty(&start_cpus)) )
> +        cpumask_copy(&start_cpus,&cpus);
> +    cpu = cpumask_test_cpu(vc->processor,&start_cpus)
>               ? vc->processor
> -            : cpumask_cycle(vc->processor,&cpus);
> +            : cpumask_cycle(vc->processor,&start_cpus);
>       ASSERT( !cpumask_empty(&cpus)&&  cpumask_test_cpu(cpu,&cpus) );

Shouldn't the ASSERT be changed to start_cpus, too?


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PBG PDG ES&S SWE OS6                   Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html