On 17/08/16 18:19, Dario Faggioli wrote:
> This is done by means of the "usual" two steps loop:
>  - soft affinity balance step;
>  - hard affinity balance step.
> 
> The entire logic implemented in runq_tickle() is
> applied, during the first step, considering only the
> CPUs in the vcpu's soft affinity. In the second step,
> we fall back to use all the CPUs from its hard
> affinity (as it is doing now, without this patch).
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Signed-off-by: Justin T. Weaver <jtweaver@hawaii.edu>
> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Anshul Makkar <anshul.makkar@citrix.com>
> ---
>  xen/common/sched_credit2.c |  243 ++++++++++++++++++++++++++++----------------
>  1 file changed, 157 insertions(+), 86 deletions(-)
> 
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index 0d83bd7..3aef1b4 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -902,6 +902,42 @@ __runq_remove(struct csched2_vcpu *svc)
>      list_del_init(&svc->runq_elem);
>  }
>  
> +/*
> + * During the soft-affinity step, only actually preempt someone if
> + * he does not have soft-affinity with cpu (while we have).
> + *
> + * BEWARE that this uses cpumask_scratch, trowing away what's in there!
> + */
> +static inline bool_t soft_aff_check_preempt(unsigned int bs, unsigned int cpu)
> +{
> +    struct csched2_vcpu * cur = CSCHED2_VCPU(curr_on_cpu(cpu));
> +
> +    /*
> +     * If we're doing hard-affinity, always check whether to preempt cur.
> +     * If we're doing soft-affinity, but cur doesn't have one, check as well.
> +     */
> +    if ( bs == BALANCE_HARD_AFFINITY ||
> +         !has_soft_affinity(cur->vcpu, cur->vcpu->cpu_hard_affinity) )
> +        return 1;
> +
> +    /*
> +     * We're doing soft-affinity, and we know that the current vcpu on cpu
> +     * has a soft affinity. We now want to know whether cpu itself is in
> +     * such affinity. In fact, since we now that new (in runq_tickle()) is:

This is a bit confusing.  I think you mean, "We know that the vcpu we
want to place has soft affinity with the target cpu; now we want to know
whether the vcpu running on the target cpu has soft affinity with that
cpu or not."

> +     *  - if cpu is not in cur's soft-affinity, we should indeed check to
> +     *    see whether new should preempt cur. If that will be the case, that
> +     *    would be an improvement wrt respecting soft affinity;
> +     *  - if cpu is in cur's soft-affinity, we leave it alone and (in
> +     *    runq_tickle()) move on to another cpu. In fact, we don't want to
> +     *    be too harsh with someone which is running within its soft-affinity.
> +     *    This is safe because later, if we don't fine anyone else during the
> +     *    soft-affinity step, we will check cpu for preemption anyway, when
> +     *    doing hard-affinity.

But by doing this, isn't it actually more likely that we'll end up
somewhere outside our soft affinity, even though there are cpus inside
our soft affinity where we have higher credit?

It would be nice if we could pre-empt somebody outside their soft
affinity before pre-empting somebody inside their soft affinity.

It occurs to me -- in the normal case, the number of cpus involved here
should be lower than 8, and often lower than that.  Rather than loop
around twice, with potentially two "inner" loops, would it make sense
just to sweep through the hard affinity once, calculating a "score" that
factored in the different things we want to factor in, and then choosing
the highest score (if any)?

Something like the attached (compile-tested only)?

 -George