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

[Dario Faggioli <dfaggioli@xxxxxxxx>]

Mmm... this patch has a few hard-tabs in it! Apologies for that. :-/

I'm attaching a version without them, but I surely can resubmit the
series with this fixed, it that's easier.

Regards, and Sorry again

On Tue, 2022-08-02 at 15:51 +0200, 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")
> Reported-by: Olaf Hering <ohering@xxxxxxx>
> Signed-off-by: Dario Faggioli <dfaggioli@xxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> ---
> Changes from "RFC" [1]:
> - Moved handling of the shim case
> - Added some more explanation (in particular, about why we stick to
> all
>   CPUs for the soft affinity) in both commit message and comment
> - Remove spurious (and non-necessary) credit2 change
> 
> [1]
> https://lore.kernel.org/xen-devel/e061a647cd77a36834e2085a96a07caa785c5066.camel@xxxxxxxx/
> ---
>  xen/common/sched/core.c |   63 +++++++++++++++++++++++++++++--------
> ----------
>  1 file changed, 39 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 379a791d62..0585c643a5 100644
> --- 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 )
> +    {
> +        /*
> +         * 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) )
> +    {
> +        /*
> +         * In absence of dom0_vcpus_pin instead, the hard and soft
> affinity of
> +         * dom0 is controlled by the (x86 only) dom0_nodes
> parameter. At this
> +         * point it has been parsed and decoded into the dom0_cpus
> mask.
> +        *
> +        * Note that we always honor what user explicitly requested,
> for both
> +        * hard and soft affinity, without doing any dynamic
> computation of
> +        * either of them.
> +         */
> +        if ( !dom0_affinity_relaxed )
> +            sched_set_affinity(unit, &dom0_cpus, &cpumask_all);
> +        else
> +            sched_set_affinity(unit, &cpumask_all, &dom0_cpus);
> +    }
> +#endif
>      else
>          sched_set_affinity(unit, &cpumask_all, &cpumask_all);
>  
> @@ -3402,29 +3436,10 @@ void wait(void)
>  void __init sched_setup_dom0_vcpus(struct domain *d)
>  {
>      unsigned int i;
> -    struct sched_unit *unit;
>  
>      for ( i = 1; i < d->max_vcpus; i++ )
>          vcpu_create(d, i);
>  
> -    /*
> -     * 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.
> -     */
> -    if ( pv_shim )
> -        sched_set_affinity(d->vcpu[0]->sched_unit,
> -                           cpumask_of(0), cpumask_of(0));
> -    else
> -    {
> -        for_each_sched_unit ( d, unit )
> -        {
> -            if ( !opt_dom0_vcpus_pin && !dom0_affinity_relaxed )
> -                sched_set_affinity(unit, &dom0_cpus, NULL);
> -            sched_set_affinity(unit, NULL, &dom0_cpus);
> -        }
> -    }
> -
>      domain_update_node_affinity(d);
>  }
>  #endif
> 
> 
> 

-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: 0002-xen-sched-setup-dom0-vCPUs-affinity-only-once.patch
Description: Text Data

Attachment: signature.asc
Description: This is a digitally signed message part