Re: [PATCH 2/2] xen/sched: setup dom0 vCPUs affinity only once

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.08.2022 15:51, Dario Faggioli wrote:
> Right now, affinity for dom0 vCPUs is setup in two steps. This is a
> problem as, at least in Credit2, unit_insert() sees and uses the
> "intermediate" affinity, and place the vCPUs on CPUs where they cannot
> be run. And this in turn results in boot hangs, if the "dom0_nodes"
> parameter is used.
> 
> Fix this by setting up the affinity properly once and for all, in
> sched_init_vcpu() called by create_vcpu().
> 
> Note that, unless a soft-affinity is explicitly specified for dom0 (by
> using the relaxed mode of "dom0_nodes") we set it to the default, which
> is all CPUs, instead of computing it basing on hard affinity (if any).
> This is because hard and soft affinity should be considered as
> independent user controlled properties. In fact, if we dor derive dom0's
> soft-affinity from its boot-time hard-affinity, such computed value will
> continue to be used even if later the user changes the hard-affinity.
> And this could result in the vCPUs behaving differently than what the
> user wanted and expects.
> 
> Fixes: dafd936ddd ("Make credit2 the default scheduler")

Nit: Please specify the first 12 digits of the hash here, as per
docs/process/sending-patches.pandoc.

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -571,12 +571,46 @@ int sched_init_vcpu(struct vcpu *v)
>          return 1;
>      }
>  
> -    /*
> -     * Initialize affinity settings. The idler, and potentially
> -     * domain-0 VCPUs, are pinned onto their respective physical CPUs.
> -     */
> -    if ( is_idle_domain(d) || (is_control_domain(d) && opt_dom0_vcpus_pin) )
> +    if ( is_idle_domain(d) )
> +    {
> +        /* Idle vCPUs are always pinned onto their respective pCPUs */
> +        sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
> +    }
> +    else if ( pv_shim && v->vcpu_id == 0 )
> +    {
> +        /*
> +         * PV-shim: vcpus are pinned 1:1. Initially only 1 cpu is online,
> +         * others will be dealt with when onlining them. This avoids pinning
> +         * a vcpu to a not yet online cpu here.
> +         */
> +        sched_set_affinity(unit, cpumask_of(0), cpumask_of(0));
> +    }
> +    else if ( is_control_domain(d) && opt_dom0_vcpus_pin )

Like with patch one: d->domain_id == 0?

> +    {
> +        /*
> +         * If dom0_vcpus_pin is specified, dom0 vCPUs are pinned 1:1 to
> +         * their respective pCPUs too.
> +         */
>          sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
> +    }
> +#ifdef CONFIG_X86
> +    else if ( is_control_domain(d) )

Same here then. With this and with the hard tabs taken care of
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan