[PATCH 0/2] xen: credit2: fix boot hangs if dom0_nodes is used

[PATCH 0/2] xen: credit2: fix boot hangs if dom0_nodes is used

[Dario Faggioli]

Currently, if the dom0_nodes parameter is used, to limit the hard or
soft affinity of dom0's vCPUs, the boot hangs. This is because the vCPU
affinity is set in two steps, and is only correct after we've done both.
Credit2, however, manages to see and use the result of the first one and
vCPUs are put on pCPUs where they can't run.

This has been reported here:
https://bugzilla.suse.com/show_bug.cgi?id=1197081

And a fix has been discussed in the thread of this message:
https://lore.kernel.org/xen-devel/e061a647cd77a36834e2085a96a07caa785c5066.camel@suse.com/

The solution adopted in this series is to change the vCPU affinity
setting code in such a way that it happens alltogether and in one place
(patch 2).

While there, make the dom0_vcpus_pin boot parameter more precise, by
making sure that it is applied only to actual dom0's vCPUs, and not to
the vCPUs of the hardware domain, which may or may not be dom0 itself
(patch 2).

Regards
---
Dario Faggioli (2):
      xen: sched: dom0_vcpus_pin should only affect dom0
      xen/sched: setup dom0 vCPUs affinity only once


 xen/common/sched/core.c | 63 +++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 24 deletions(-)
--
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)

[PATCH 1/2] xen: sched: dom0_vcpus_pin should only affect dom0

[Dario Faggioli]

If dom0_vcpus_pin is used, make sure the pinning is only done for
dom0 vcpus, instead of for the hardware domain (which might not be
dom0 at all!).

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
---
Difference from "RFC" [1]:
- new patch

[1] https://lore.kernel.org/xen-devel/e061a647cd77a36834e2085a96a07caa785c5066.camel@suse.com/
---
 xen/common/sched/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index f689b55783..379a791d62 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
      * Initialize affinity settings. The idler, and potentially
      * domain-0 VCPUs, are pinned onto their respective physical CPUs.
      */
-    if ( is_idle_domain(d) || (is_hardware_domain(d) && opt_dom0_vcpus_pin) )
+    if ( is_idle_domain(d) || (is_control_domain(d) && opt_dom0_vcpus_pin) )
         sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
     else
         sched_set_affinity(unit, &cpumask_all, &cpumask_all);

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

[Dario Faggioli]

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@suse.de>
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
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@suse.com/
---
 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

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 6114 bytes --]

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@suse.de>
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
> ---
> 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@suse.com/
> ---
>  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 #1.2: 0002-xen-sched-setup-dom0-vCPUs-affinity-only-once.patch --]
[-- Type: text/x-patch, Size: 4906 bytes --]

From 52fd2b18f6a396f56e1f70f842360c9a8daa7ef7 Mon Sep 17 00:00:00 2001
From: Dario Faggioli <dfaggioli@suse.com>
Date: Wed, 27 Apr 2022 13:21:30 +0200
Subject: [PATCH 2/2] xen/sched: setup dom0 vCPUs affinity only once

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@suse.de>
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
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@suse.com/
---
 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..b79c3dda66 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
-- 
2.37.1


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] xen: sched: dom0_vcpus_pin should only affect dom0

[Jan Beulich]

On 02.08.2022 15:51, Dario Faggioli wrote:
> If dom0_vcpus_pin is used, make sure the pinning is only done for
> dom0 vcpus, instead of for the hardware domain (which might not be
> dom0 at all!).

Hmm, but the control domain may not be either, as it's derived from
d->is_privileged. I think ...

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
>       * Initialize affinity settings. The idler, and potentially
>       * domain-0 VCPUs, are pinned onto their respective physical CPUs.
>       */
> -    if ( is_idle_domain(d) || (is_hardware_domain(d) && opt_dom0_vcpus_pin) )
> +    if ( is_idle_domain(d) || (is_control_domain(d) && opt_dom0_vcpus_pin) )

... for it to be strictly only Dom0, you want to check d->domain_id here.

Or else I guess the description wants adjusting.

Jan

Re: [PATCH 1/2] xen: sched: dom0_vcpus_pin should only affect dom0

[Dario Faggioli]

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

On Tue, 2022-08-02 at 16:56 +0200, Jan Beulich wrote:
> On 02.08.2022 15:51, Dario Faggioli wrote:
> > If dom0_vcpus_pin is used, make sure the pinning is only done for
> > dom0 vcpus, instead of for the hardware domain (which might not be
> > dom0 at all!).
> 
> Hmm, but the control domain may not be either, as it's derived from
> d->is_privileged. I think ...
> 
Mmm... Right.

> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
> >       * Initialize affinity settings. The idler, and potentially
> >       * domain-0 VCPUs, are pinned onto their respective physical
> > CPUs.
> >       */
> > -    if ( is_idle_domain(d) || (is_hardware_domain(d) &&
> > opt_dom0_vcpus_pin) )
> > +    if ( is_idle_domain(d) || (is_control_domain(d) &&
> > opt_dom0_vcpus_pin) )
> 
> ... for it to be strictly only Dom0, you want to check d->domain_id
> here.
> 
Ok, I'll send an update that does that.

Thanks and Regards
-- 
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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] xen: sched: dom0_vcpus_pin should only affect dom0

[Jan Beulich]

On 02.08.2022 18:08, Dario Faggioli wrote:
> On Tue, 2022-08-02 at 16:56 +0200, Jan Beulich wrote:
>> On 02.08.2022 15:51, Dario Faggioli wrote:
>>> If dom0_vcpus_pin is used, make sure the pinning is only done for
>>> dom0 vcpus, instead of for the hardware domain (which might not be
>>> dom0 at all!).
>>
>> Hmm, but the control domain may not be either, as it's derived from
>> d->is_privileged. I think ...
>>
> Mmm... Right.
> 
>>> --- a/xen/common/sched/core.c
>>> +++ b/xen/common/sched/core.c
>>> @@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
>>>       * Initialize affinity settings. The idler, and potentially
>>>       * domain-0 VCPUs, are pinned onto their respective physical
>>> CPUs.
>>>       */
>>> -    if ( is_idle_domain(d) || (is_hardware_domain(d) &&
>>> opt_dom0_vcpus_pin) )
>>> +    if ( is_idle_domain(d) || (is_control_domain(d) &&
>>> opt_dom0_vcpus_pin) )
>>
>> ... for it to be strictly only Dom0, you want to check d->domain_id
>> here.
>>
> Ok, I'll send an update that does that.

Well - if you agree, I'd be happy to make the change while committing
and then adding
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

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

[Jan Beulich]

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@suse.com>

Jan

Re: [PATCH 1/2] xen: sched: dom0_vcpus_pin should only affect dom0

[Dario Faggioli]

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

On Wed, 2022-08-03 at 08:19 +0200, Jan Beulich wrote:
> On 02.08.2022 18:08, Dario Faggioli wrote:
> > > ... for it to be strictly only Dom0, you want to check d-
> > > >domain_id
> > > here.
> > > 
> > Ok, I'll send an update that does that.
> 
> Well - if you agree, I'd be happy to make the change while committing
> and then adding
>
Thanks for offering that!

Since there were a couple of things to fix, I've just sent a v2, which
hopefully handles all of them. I hope this makes things easier. :-)

> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
Great, added to v2.

Thanks again and Regards
-- 
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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]