[Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Sergey Dyasli]

Hello,

While testing pv-shim from a snapshot of staging 4.13 branch (with core-
scheduling patches applied), some sort of scheduling issues were uncovered
which usually leads to a guest lockup (sometimes with soft lockup messages
from Linux kernel).

This happens more frequently on SandyBridge CPUs. After enabling
CONFIG_DEBUG in pv-shim, the following assertions failed:

Null scheduler:

    Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
    (full crash log: https://paste.debian.net/1108861/ )

Credit1 scheduler:

    Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
    (full crash log: https://paste.debian.net/1108862/ )

I'm currently investigation those, but would appreciate any help or
suggestions.

Thanks,
Sergey

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

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

On 21.10.19 11:51, Sergey Dyasli wrote:
> Hello,
> 
> While testing pv-shim from a snapshot of staging 4.13 branch (with core-
> scheduling patches applied), some sort of scheduling issues were uncovered
> which usually leads to a guest lockup (sometimes with soft lockup messages
> from Linux kernel).
> 
> This happens more frequently on SandyBridge CPUs. After enabling
> CONFIG_DEBUG in pv-shim, the following assertions failed:
> 
> Null scheduler:
> 
>      Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>      (full crash log: https://paste.debian.net/1108861/ )
> 
> Credit1 scheduler:
> 
>      Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
>      (full crash log: https://paste.debian.net/1108862/ )
> 
> I'm currently investigation those, but would appreciate any help or
> suggestions.

Hmm, I think that pv_shim_cpu_up() shouldn't do the vcpu_wake(), but the
caller.

Does the attached patch help?


Juergen

[-- Attachment #2: 0001-xen-shim-fix-pv-shim-cpu-up-down.patch --]
[-- Type: text/x-patch, Size: 3753 bytes --]

From 068ea0419d1a67e967b9431ed11e24b731cd357f Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Mon, 21 Oct 2019 12:28:33 +0200
Subject: [PATCH] xen/shim: fix pv-shim cpu up/down

Calling vcpu_wake() from pv_shim_cpu_up() is wrong as it is not yet
sure that the correct scheduler has taken over the cpu. Doing the
call after continue_hypercall_on_cpu() is correct, so let
pv_shim_cpu_up() just online the cpu.

Adapt pv_shim_cpu_down() to be symmetric, even if that is not
required for correctness.

Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/pv/shim.c | 16 ----------------
 xen/common/domain.c    | 31 ++++++++++++++++++-------------
 2 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 5edbcd9ac5..bc6a2f3eb9 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -815,16 +815,11 @@ long pv_shim_cpu_up(void *data)
 {
     struct vcpu *v = data;
     struct domain *d = v->domain;
-    bool wake;
 
     BUG_ON(smp_processor_id() != 0);
 
-    domain_lock(d);
     if ( !v->is_initialised )
-    {
         domain_unlock(d);
-        return -EINVAL;
-    }
 
     if ( !cpu_online(v->vcpu_id) )
     {
@@ -832,18 +827,12 @@ long pv_shim_cpu_up(void *data)
 
         if ( rc )
         {
-            domain_unlock(d);
             gprintk(XENLOG_ERR, "Failed to bring up CPU#%u: %ld\n",
                     v->vcpu_id, rc);
             return rc;
         }
     }
 
-    wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
-    domain_unlock(d);
-    if ( wake )
-        vcpu_wake(v);
-
     return 0;
 }
 
@@ -852,11 +841,6 @@ long pv_shim_cpu_down(void *data)
     struct vcpu *v = data;
     long rc;
 
-    BUG_ON(smp_processor_id() != 0);
-
-    if ( !test_and_set_bit(_VPF_down, &v->pause_flags) )
-        vcpu_sleep_sync(v);
-
     if ( cpu_online(v->vcpu_id) )
     {
         rc = cpu_down_helper((void *)(unsigned long)v->vcpu_id);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 9c7360ed2a..e83657e310 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1428,19 +1428,21 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case VCPUOP_up:
-#ifdef CONFIG_X86
-        if ( pv_shim )
-            rc = continue_hypercall_on_cpu(0, pv_shim_cpu_up, v);
-        else
-#endif
         {
             bool wake = false;
 
             domain_lock(d);
-            if ( !v->is_initialised )
-                rc = -EINVAL;
-            else
-                wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
+#ifdef CONFIG_X86
+            if ( pv_shim )
+                rc = continue_hypercall_on_cpu(0, pv_shim_cpu_up, v);
+#endif
+            if ( !rc )
+            {
+                if ( !v->is_initialised )
+                    rc = -EINVAL;
+                else
+                    wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
+            }
             domain_unlock(d);
             if ( wake )
                 vcpu_wake(v);
@@ -1465,14 +1467,17 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
         rc = 0;
         v = d->vcpu[vcpuid];
 
+#ifdef CONFIG_X86
+        BUG_ON(pv_shim && smp_processor_id() != 0);
+#endif
+
+        if ( !test_and_set_bit(_VPF_down, &v->pause_flags) )
+            vcpu_sleep_nosync(v);
+
 #ifdef CONFIG_X86
         if ( pv_shim )
             rc = continue_hypercall_on_cpu(0, pv_shim_cpu_down, v);
-        else
 #endif
-            if ( !test_and_set_bit(_VPF_down, &v->pause_flags) )
-                vcpu_sleep_nosync(v);
-
         break;
 
     case VCPUOP_is_up:
-- 
2.16.4


[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Roger Pau Monné]

On Mon, Oct 21, 2019 at 12:52:10PM +0200, Jürgen Groß wrote:
> On 21.10.19 11:51, Sergey Dyasli wrote:
> > Hello,
> > 
> > While testing pv-shim from a snapshot of staging 4.13 branch (with core-
> > scheduling patches applied), some sort of scheduling issues were uncovered
> > which usually leads to a guest lockup (sometimes with soft lockup messages
> > from Linux kernel).
> > 
> > This happens more frequently on SandyBridge CPUs. After enabling
> > CONFIG_DEBUG in pv-shim, the following assertions failed:
> > 
> > Null scheduler:
> > 
> >      Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
> >      (full crash log: https://paste.debian.net/1108861/ )
> > 
> > Credit1 scheduler:
> > 
> >      Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
> >      (full crash log: https://paste.debian.net/1108862/ )
> > 
> > I'm currently investigation those, but would appreciate any help or
> > suggestions.
> 
> Hmm, I think that pv_shim_cpu_up() shouldn't do the vcpu_wake(), but the
> caller.
> 
> Does the attached patch help?
> 
> 
> Juergen

> From 068ea0419d1a67e967b9431ed11e24b731cd357f Mon Sep 17 00:00:00 2001
> From: Juergen Gross <jgross@suse.com>
> Date: Mon, 21 Oct 2019 12:28:33 +0200
> Subject: [PATCH] xen/shim: fix pv-shim cpu up/down
> 
> Calling vcpu_wake() from pv_shim_cpu_up() is wrong as it is not yet
> sure that the correct scheduler has taken over the cpu.

I'm not sure why this is wrong, the scheduler should be capable of
handling 2 vCPUs on a single pCPU while the new pCPU is brought
online?

Note that with the current code the vcpu_wake is not performed until
cpu_up_helper has been called and returned successfully.

> Doing the
> call after continue_hypercall_on_cpu() is correct, so let
> pv_shim_cpu_up() just online the cpu.
> 
> Adapt pv_shim_cpu_down() to be symmetric, even if that is not
> required for correctness.
> 
> Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  xen/arch/x86/pv/shim.c | 16 ----------------
>  xen/common/domain.c    | 31 ++++++++++++++++++-------------
>  2 files changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index 5edbcd9ac5..bc6a2f3eb9 100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -815,16 +815,11 @@ long pv_shim_cpu_up(void *data)
>  {
>      struct vcpu *v = data;
>      struct domain *d = v->domain;
> -    bool wake;
>  
>      BUG_ON(smp_processor_id() != 0);
>  
> -    domain_lock(d);
>      if ( !v->is_initialised )
> -    {
>          domain_unlock(d);
> -        return -EINVAL;

I think you wanted to remove the domain_unlock not the return.

> -    }
>  
>      if ( !cpu_online(v->vcpu_id) )
>      {
> @@ -832,18 +827,12 @@ long pv_shim_cpu_up(void *data)
>  
>          if ( rc )
>          {
> -            domain_unlock(d);
>              gprintk(XENLOG_ERR, "Failed to bring up CPU#%u: %ld\n",
>                      v->vcpu_id, rc);
>              return rc;
>          }
>      }
>  
> -    wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
> -    domain_unlock(d);
> -    if ( wake )
> -        vcpu_wake(v);
> -
>      return 0;
>  }
>  
> @@ -852,11 +841,6 @@ long pv_shim_cpu_down(void *data)
>      struct vcpu *v = data;
>      long rc;
>  
> -    BUG_ON(smp_processor_id() != 0);
> -
> -    if ( !test_and_set_bit(_VPF_down, &v->pause_flags) )
> -        vcpu_sleep_sync(v);
> -
>      if ( cpu_online(v->vcpu_id) )
>      {
>          rc = cpu_down_helper((void *)(unsigned long)v->vcpu_id);
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 9c7360ed2a..e83657e310 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1428,19 +1428,21 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>  
>      case VCPUOP_up:
> -#ifdef CONFIG_X86
> -        if ( pv_shim )
> -            rc = continue_hypercall_on_cpu(0, pv_shim_cpu_up, v);
> -        else
> -#endif
>          {
>              bool wake = false;
>  
>              domain_lock(d);
> -            if ( !v->is_initialised )
> -                rc = -EINVAL;
> -            else
> -                wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
> +#ifdef CONFIG_X86
> +            if ( pv_shim )
> +                rc = continue_hypercall_on_cpu(0, pv_shim_cpu_up, v);
> +#endif
> +            if ( !rc )
> +            {
> +                if ( !v->is_initialised )
> +                    rc = -EINVAL;
> +                else
> +                    wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
> +            }

Hm, I'm not sure why this is any different from the current code,
continue_hypercall_on_cpu will schedule pv_shim_cpu_up to be called on
CPU0, but it won't wait for it to execute, so you are clearing the
pause_flags bit without having any guarantee that the physical CPU is
actually up?

The previous code waited for cpu_up_helper to return successfully before
onlining the vCPU, which seems better IMO.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

On 21.10.19 13:36, Roger Pau Monné wrote:
> On Mon, Oct 21, 2019 at 12:52:10PM +0200, Jürgen Groß wrote:
>> On 21.10.19 11:51, Sergey Dyasli wrote:
>>> Hello,
>>>
>>> While testing pv-shim from a snapshot of staging 4.13 branch (with core-
>>> scheduling patches applied), some sort of scheduling issues were uncovered
>>> which usually leads to a guest lockup (sometimes with soft lockup messages
>>> from Linux kernel).
>>>
>>> This happens more frequently on SandyBridge CPUs. After enabling
>>> CONFIG_DEBUG in pv-shim, the following assertions failed:
>>>
>>> Null scheduler:
>>>
>>>       Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>>>       (full crash log: https://paste.debian.net/1108861/ )
>>>
>>> Credit1 scheduler:
>>>
>>>       Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
>>>       (full crash log: https://paste.debian.net/1108862/ )
>>>
>>> I'm currently investigation those, but would appreciate any help or
>>> suggestions.
>>
>> Hmm, I think that pv_shim_cpu_up() shouldn't do the vcpu_wake(), but the
>> caller.
>>
>> Does the attached patch help?
>>
>>
>> Juergen
> 
>>  From 068ea0419d1a67e967b9431ed11e24b731cd357f Mon Sep 17 00:00:00 2001
>> From: Juergen Gross <jgross@suse.com>
>> Date: Mon, 21 Oct 2019 12:28:33 +0200
>> Subject: [PATCH] xen/shim: fix pv-shim cpu up/down
>>
>> Calling vcpu_wake() from pv_shim_cpu_up() is wrong as it is not yet
>> sure that the correct scheduler has taken over the cpu.
> 
> I'm not sure why this is wrong, the scheduler should be capable of
> handling 2 vCPUs on a single pCPU while the new pCPU is brought
> online?

Oh, right, I made some false assumptions.

This patch is pure nonsense.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

On 21.10.19 11:51, Sergey Dyasli wrote:
> Hello,
> 
> While testing pv-shim from a snapshot of staging 4.13 branch (with core-
> scheduling patches applied), some sort of scheduling issues were uncovered
> which usually leads to a guest lockup (sometimes with soft lockup messages
> from Linux kernel).
> 
> This happens more frequently on SandyBridge CPUs. After enabling
> CONFIG_DEBUG in pv-shim, the following assertions failed:
> 
> Null scheduler:
> 
>      Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>      (full crash log: https://paste.debian.net/1108861/ )

I guess this is the known null scheduler hotplug problem?

> 
> Credit1 scheduler:
> 
>      Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
>      (full crash log: https://paste.debian.net/1108862/ )

I think this is a bug in credit scheduler:

In csched_aff_cntl() CSCHED_FLAG_UNIT_PINNED should be set only in case
the cpu the affinity is set to is online. An alternative might be to
add the condition of above ASSERT() to the if () statement guarding it
and dropping the ASSERT().

Dario, George?

Before my patch "xen: let vcpu_create() select processor" in pv-shim
the initial cpu of a vcpu would be set to a not yet online cpu, which
did work just by chance.

So another possibility would be to modify pv_shim_cpu_up() to call a new
scheduler function doing another cpu assignment and the vcpu_wake() if
needed.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Sergey Dyasli]

After printing some debug information I see that:

SMP alternatives: switching to SMP code
(XEN) [    1.473056] == d1v1 master_cpu 0, lock ffff83018e315ec8
(XEN) [    1.473120] sched_null.c:344: 1 <-- d1v1
(XEN) [    1.473165] == d1v1 master_cpu 1, lock ffff8301899c2f48
(XEN) [    1.473223] Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:27

The underlying pCPU was changed for that vCPU and hence the per-cpu lock changed as well.

Thanks,
Sergey


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

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

On 21.10.19 11:51, Sergey Dyasli wrote:
> Hello,
> 
> While testing pv-shim from a snapshot of staging 4.13 branch (with core-
> scheduling patches applied), some sort of scheduling issues were uncovered
> which usually leads to a guest lockup (sometimes with soft lockup messages
> from Linux kernel).
> 
> This happens more frequently on SandyBridge CPUs. After enabling
> CONFIG_DEBUG in pv-shim, the following assertions failed:
> 
> Null scheduler:
> 
>      Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>      (full crash log: https://paste.debian.net/1108861/ )
> 
> Credit1 scheduler:
> 
>      Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
>      (full crash log: https://paste.debian.net/1108862/ )
> 
> I'm currently investigation those, but would appreciate any help or
> suggestions.

And now a more sane patch to try.


Juergen


[-- Attachment #2: 0001-xen-pvhsim-fix-cpu-onlining.patch --]
[-- Type: text/x-patch, Size: 1624 bytes --]

From 205b7622b84bc678f8a0d6ac121dff14439fe331 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
To: xen-devel@lists.xenproject.org
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Date: Tue, 22 Oct 2019 11:14:08 +0200
Subject: [PATCH] xen/pvhsim: fix cpu onlining

Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
the initial processor for all pv-shim vcpus will be 0, as no other cpus
are online when the vcpus are created. Before that commit the vcpus
would have processors set not being online yet, which worked just by
chance.

When the pv-shim vcpu becomes active it will have a hard affinity
not matching its initial processor assignment leading to failing
ASSERT()s or other problems depending on the selected scheduler.

Fix that by redoing the affinity setting after onlining the cpu but
before taking the vcpu up.

Fixes: 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/pv/shim.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 5edbcd9ac5..4329eaaefe 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -837,6 +837,8 @@ long pv_shim_cpu_up(void *data)
                     v->vcpu_id, rc);
             return rc;
         }
+
+        vcpu_set_hard_affinity(v, cpumask_of(v->vcpu_id));
     }
 
     wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
-- 
2.16.4


[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Roger Pau Monné]

On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
> On 21.10.19 11:51, Sergey Dyasli wrote:
> > Hello,
> > 
> > While testing pv-shim from a snapshot of staging 4.13 branch (with core-
> > scheduling patches applied), some sort of scheduling issues were uncovered
> > which usually leads to a guest lockup (sometimes with soft lockup messages
> > from Linux kernel).
> > 
> > This happens more frequently on SandyBridge CPUs. After enabling
> > CONFIG_DEBUG in pv-shim, the following assertions failed:
> > 
> > Null scheduler:
> > 
> >      Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
> >      (full crash log: https://paste.debian.net/1108861/ )
> > 
> > Credit1 scheduler:
> > 
> >      Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
> >      (full crash log: https://paste.debian.net/1108862/ )
> > 
> > I'm currently investigation those, but would appreciate any help or
> > suggestions.
> 
> And now a more sane patch to try.
> 
> 
> Juergen
> 

> From 205b7622b84bc678f8a0d6ac121dff14439fe331 Mon Sep 17 00:00:00 2001
> From: Juergen Gross <jgross@suse.com>
> To: xen-devel@lists.xenproject.org
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> Date: Tue, 22 Oct 2019 11:14:08 +0200
> Subject: [PATCH] xen/pvhsim: fix cpu onlining
> 
> Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
> the initial processor for all pv-shim vcpus will be 0, as no other cpus
> are online when the vcpus are created. Before that commit the vcpus
> would have processors set not being online yet, which worked just by
> chance.
> 
> When the pv-shim vcpu becomes active it will have a hard affinity
> not matching its initial processor assignment leading to failing
> ASSERT()s or other problems depending on the selected scheduler.

I'm slightly lost here, who has set this hard affinity on the pvshim
vCPUs?

> Fix that by redoing the affinity setting after onlining the cpu but
> before taking the vcpu up.

The change seems fine to me, but I don't understand why the lack of
this can cause asserts to trigger, as reported by Sergey. I also
wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
pv_shim_cpu_up is only used for APs.

I would expect that pvshim guest vCPUs have no hard affinity ATM, and
that when a pCPU (from the shim PoV) is brought online it will be
added to the pool of available pCPU for the shim to schedule vCPUs
on.

> Fixes: 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
> Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  xen/arch/x86/pv/shim.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
> index 5edbcd9ac5..4329eaaefe 100644
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -837,6 +837,8 @@ long pv_shim_cpu_up(void *data)
>                      v->vcpu_id, rc);
>              return rc;
>          }
> +
> +        vcpu_set_hard_affinity(v, cpumask_of(v->vcpu_id));
>      }
>  
>      wake = test_and_clear_bit(_VPF_down, &v->pause_flags);
> -- 
> 2.16.4
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

On 22.10.19 12:52, Roger Pau Monné wrote:
> On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
>> On 21.10.19 11:51, Sergey Dyasli wrote:
>>> Hello,
>>>
>>> While testing pv-shim from a snapshot of staging 4.13 branch (with core-
>>> scheduling patches applied), some sort of scheduling issues were uncovered
>>> which usually leads to a guest lockup (sometimes with soft lockup messages
>>> from Linux kernel).
>>>
>>> This happens more frequently on SandyBridge CPUs. After enabling
>>> CONFIG_DEBUG in pv-shim, the following assertions failed:
>>>
>>> Null scheduler:
>>>
>>>       Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>>>       (full crash log: https://paste.debian.net/1108861/ )
>>>
>>> Credit1 scheduler:
>>>
>>>       Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
>>>       (full crash log: https://paste.debian.net/1108862/ )
>>>
>>> I'm currently investigation those, but would appreciate any help or
>>> suggestions.
>>
>> And now a more sane patch to try.
>>
>>
>> Juergen
>>
> 
>>  From 205b7622b84bc678f8a0d6ac121dff14439fe331 Mon Sep 17 00:00:00 2001
>> From: Juergen Gross <jgross@suse.com>
>> To: xen-devel@lists.xenproject.org
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: Wei Liu <wl@xen.org>
>> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>> Date: Tue, 22 Oct 2019 11:14:08 +0200
>> Subject: [PATCH] xen/pvhsim: fix cpu onlining
>>
>> Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
>> the initial processor for all pv-shim vcpus will be 0, as no other cpus
>> are online when the vcpus are created. Before that commit the vcpus
>> would have processors set not being online yet, which worked just by
>> chance.
>>
>> When the pv-shim vcpu becomes active it will have a hard affinity
>> not matching its initial processor assignment leading to failing
>> ASSERT()s or other problems depending on the selected scheduler.
> 
> I'm slightly lost here, who has set this hard affinity on the pvshim
> vCPUs?

That is done in sched_setup_dom0_vcpus().

> 
>> Fix that by redoing the affinity setting after onlining the cpu but
>> before taking the vcpu up.
> 
> The change seems fine to me, but I don't understand why the lack of
> this can cause asserts to trigger, as reported by Sergey. I also
> wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
> pv_shim_cpu_up is only used for APs.

When vcpu 0 is being created pcpu 0 is online already. So the affinity
set in sched_setup_dom0_vcpus() is fine in that case.

> I would expect that pvshim guest vCPUs have no hard affinity ATM, and
> that when a pCPU (from the shim PoV) is brought online it will be
> added to the pool of available pCPU for the shim to schedule vCPUs
> on.

That expectation is wrong. All vcpus are pinned to their respective
pcpus.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Andrew Cooper]

On 22/10/2019 12:01, Jürgen Groß wrote:
> On 22.10.19 12:52, Roger Pau Monné wrote:
>> On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
>>> On 21.10.19 11:51, Sergey Dyasli wrote:
>>>> Hello,
>>>>
>>>> While testing pv-shim from a snapshot of staging 4.13 branch (with
>>>> core-
>>>> scheduling patches applied), some sort of scheduling issues were
>>>> uncovered
>>>> which usually leads to a guest lockup (sometimes with soft lockup
>>>> messages
>>>> from Linux kernel).
>>>>
>>>> This happens more frequently on SandyBridge CPUs. After enabling
>>>> CONFIG_DEBUG in pv-shim, the following assertions failed:
>>>>
>>>> Null scheduler:
>>>>
>>>>       Assertion 'lock ==
>>>> get_sched_res(i->res->master_cpu)->schedule_lock' failed at
>>>> ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>>>>       (full crash log: https://paste.debian.net/1108861/ )
>>>>
>>>> Credit1 scheduler:
>>>>
>>>>       Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) ==
>>>> cpu' failed at sched_credit.c:383
>>>>       (full crash log: https://paste.debian.net/1108862/ )
>>>>
>>>> I'm currently investigation those, but would appreciate any help or
>>>> suggestions.
>>>
>>> And now a more sane patch to try.
>>>
>>>
>>> Juergen
>>>
>>
>>>  From 205b7622b84bc678f8a0d6ac121dff14439fe331 Mon Sep 17 00:00:00 2001
>>> From: Juergen Gross <jgross@suse.com>
>>> To: xen-devel@lists.xenproject.org
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Cc: Wei Liu <wl@xen.org>
>>> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>>> Date: Tue, 22 Oct 2019 11:14:08 +0200
>>> Subject: [PATCH] xen/pvhsim: fix cpu onlining
>>>
>>> Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
>>> the initial processor for all pv-shim vcpus will be 0, as no other cpus
>>> are online when the vcpus are created. Before that commit the vcpus
>>> would have processors set not being online yet, which worked just by
>>> chance.
>>>
>>> When the pv-shim vcpu becomes active it will have a hard affinity
>>> not matching its initial processor assignment leading to failing
>>> ASSERT()s or other problems depending on the selected scheduler.
>>
>> I'm slightly lost here, who has set this hard affinity on the pvshim
>> vCPUs?
>
> That is done in sched_setup_dom0_vcpus().
>
>>
>>> Fix that by redoing the affinity setting after onlining the cpu but
>>> before taking the vcpu up.
>>
>> The change seems fine to me, but I don't understand why the lack of
>> this can cause asserts to trigger, as reported by Sergey. I also
>> wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
>> pv_shim_cpu_up is only used for APs.
>
> When vcpu 0 is being created pcpu 0 is online already. So the affinity
> set in sched_setup_dom0_vcpus() is fine in that case.
>
>> I would expect that pvshim guest vCPUs have no hard affinity ATM, and
>> that when a pCPU (from the shim PoV) is brought online it will be
>> added to the pool of available pCPU for the shim to schedule vCPUs
>> on.
>
> That expectation is wrong. All vcpus are pinned to their respective
> pcpus.

The goal for Shim was always to use the NULL scheduler and always have
vcpu == pcpu.  The only reason we use credit is because NULL (still!)
doesn't work, and bodge the scheduling using hard affinity.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Roger Pau Monné]

On Tue, Oct 22, 2019 at 01:01:09PM +0200, Jürgen Groß wrote:
> On 22.10.19 12:52, Roger Pau Monné wrote:
> > On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
> > > On 21.10.19 11:51, Sergey Dyasli wrote:
> > > > Hello,
> > > > 
> > > > While testing pv-shim from a snapshot of staging 4.13 branch (with core-
> > > > scheduling patches applied), some sort of scheduling issues were uncovered
> > > > which usually leads to a guest lockup (sometimes with soft lockup messages
> > > > from Linux kernel).
> > > > 
> > > > This happens more frequently on SandyBridge CPUs. After enabling
> > > > CONFIG_DEBUG in pv-shim, the following assertions failed:
> > > > 
> > > > Null scheduler:
> > > > 
> > > >       Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
> > > >       (full crash log: https://paste.debian.net/1108861/ )
> > > > 
> > > > Credit1 scheduler:
> > > > 
> > > >       Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
> > > >       (full crash log: https://paste.debian.net/1108862/ )
> > > > 
> > > > I'm currently investigation those, but would appreciate any help or
> > > > suggestions.
> > > 
> > > And now a more sane patch to try.
> > > 
> > > 
> > > Juergen
> > > 
> > 
> > >  From 205b7622b84bc678f8a0d6ac121dff14439fe331 Mon Sep 17 00:00:00 2001
> > > From: Juergen Gross <jgross@suse.com>
> > > To: xen-devel@lists.xenproject.org
> > > Cc: Jan Beulich <jbeulich@suse.com>
> > > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > > Cc: Wei Liu <wl@xen.org>
> > > Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> > > Date: Tue, 22 Oct 2019 11:14:08 +0200
> > > Subject: [PATCH] xen/pvhsim: fix cpu onlining
> > > 
> > > Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
> > > the initial processor for all pv-shim vcpus will be 0, as no other cpus
> > > are online when the vcpus are created. Before that commit the vcpus
> > > would have processors set not being online yet, which worked just by
> > > chance.

So all vCPUs for the shim have their hard affinity set to pCPU#0 if I
understand it correctly. From my reading of sched_setup_dom0_vcpus it
seems like in the shim case all sched units are pinned to their id,
which would imply sched units != 0 are not pinned to CPU#0?

Or maybe there's only one sched unit that contains all the shim vCPUs?

> > > When the pv-shim vcpu becomes active it will have a hard affinity
> > > not matching its initial processor assignment leading to failing
> > > ASSERT()s or other problems depending on the selected scheduler.
> > 
> > I'm slightly lost here, who has set this hard affinity on the pvshim
> > vCPUs?
> 
> That is done in sched_setup_dom0_vcpus().
>
> > 
> > > Fix that by redoing the affinity setting after onlining the cpu but
> > > before taking the vcpu up.
> > 
> > The change seems fine to me, but I don't understand why the lack of
> > this can cause asserts to trigger, as reported by Sergey. I also
> > wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
> > pv_shim_cpu_up is only used for APs.
> 
> When vcpu 0 is being created pcpu 0 is online already. So the affinity
> set in sched_setup_dom0_vcpus() is fine in that case.

IIRC all shim vCPUs where pinned to their identity pCPU at creation, and
there was no need to do this pining when the vCPU is brought online. I
guess this is no longer possible.

What is not clear to me is why having all vCPUs pinned to pCPU#0 can
lead to assertions triggering, such scenario is not desirable, but
shouldn't lead to crashes.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

On 22.10.19 13:25, Roger Pau Monné wrote:
> On Tue, Oct 22, 2019 at 01:01:09PM +0200, Jürgen Groß wrote:
>> On 22.10.19 12:52, Roger Pau Monné wrote:
>>> On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
>>>> On 21.10.19 11:51, Sergey Dyasli wrote:
>>>>> Hello,
>>>>>
>>>>> While testing pv-shim from a snapshot of staging 4.13 branch (with core-
>>>>> scheduling patches applied), some sort of scheduling issues were uncovered
>>>>> which usually leads to a guest lockup (sometimes with soft lockup messages
>>>>> from Linux kernel).
>>>>>
>>>>> This happens more frequently on SandyBridge CPUs. After enabling
>>>>> CONFIG_DEBUG in pv-shim, the following assertions failed:
>>>>>
>>>>> Null scheduler:
>>>>>
>>>>>        Assertion 'lock == get_sched_res(i->res->master_cpu)->schedule_lock' failed at ...are/xen-dir/xen-root/xen/include/xen/sched-if.h:278
>>>>>        (full crash log: https://paste.debian.net/1108861/ )
>>>>>
>>>>> Credit1 scheduler:
>>>>>
>>>>>        Assertion 'cpumask_cycle(cpu, unit->cpu_hard_affinity) == cpu' failed at sched_credit.c:383
>>>>>        (full crash log: https://paste.debian.net/1108862/ )
>>>>>
>>>>> I'm currently investigation those, but would appreciate any help or
>>>>> suggestions.
>>>>
>>>> And now a more sane patch to try.
>>>>
>>>>
>>>> Juergen
>>>>
>>>
>>>>   From 205b7622b84bc678f8a0d6ac121dff14439fe331 Mon Sep 17 00:00:00 2001
>>>> From: Juergen Gross <jgross@suse.com>
>>>> To: xen-devel@lists.xenproject.org
>>>> Cc: Jan Beulich <jbeulich@suse.com>
>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Cc: Wei Liu <wl@xen.org>
>>>> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>>>> Date: Tue, 22 Oct 2019 11:14:08 +0200
>>>> Subject: [PATCH] xen/pvhsim: fix cpu onlining
>>>>
>>>> Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
>>>> the initial processor for all pv-shim vcpus will be 0, as no other cpus
>>>> are online when the vcpus are created. Before that commit the vcpus
>>>> would have processors set not being online yet, which worked just by
>>>> chance.
> 
> So all vCPUs for the shim have their hard affinity set to pCPU#0 if I

No, the hard affinity is set to pcpu#(vcpu-id), but the initial cpu to
run on is pcpu#0 as no other cpu is online when the vcpus are being
created, and v->processor should always be a valid online cpu.

> understand it correctly. From my reading of sched_setup_dom0_vcpus it
> seems like in the shim case all sched units are pinned to their id,
> which would imply sched units != 0 are not pinned to CPU#0?

Right.

> 
> Or maybe there's only one sched unit that contains all the shim vCPUs?

No.

> 
>>>> When the pv-shim vcpu becomes active it will have a hard affinity
>>>> not matching its initial processor assignment leading to failing
>>>> ASSERT()s or other problems depending on the selected scheduler.
>>>
>>> I'm slightly lost here, who has set this hard affinity on the pvshim
>>> vCPUs?
>>
>> That is done in sched_setup_dom0_vcpus().
>>
>>>
>>>> Fix that by redoing the affinity setting after onlining the cpu but
>>>> before taking the vcpu up.
>>>
>>> The change seems fine to me, but I don't understand why the lack of
>>> this can cause asserts to trigger, as reported by Sergey. I also
>>> wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
>>> pv_shim_cpu_up is only used for APs.
>>
>> When vcpu 0 is being created pcpu 0 is online already. So the affinity
>> set in sched_setup_dom0_vcpus() is fine in that case.
> 
> IIRC all shim vCPUs where pinned to their identity pCPU at creation, and
> there was no need to do this pining when the vCPU is brought online. I
> guess this is no longer possible.

The problem is not the pinning, but the initial cpu stored in
v->processor. This results in v->processor not being set in the hard
affinity mask of the vcpu (or better: unit) which then triggers the
problems.

> What is not clear to me is why having all vCPUs pinned to pCPU#0 can
> lead to assertions triggering, such scenario is not desirable, but
> shouldn't lead to crashes.

Right, but your assumption regarding pinning is wrong.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Roger Pau Monné]

On Tue, Oct 22, 2019 at 01:50:44PM +0200, Jürgen Groß wrote:
> On 22.10.19 13:25, Roger Pau Monné wrote:
> > On Tue, Oct 22, 2019 at 01:01:09PM +0200, Jürgen Groß wrote:
> > > On 22.10.19 12:52, Roger Pau Monné wrote:
> > > > On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
> > > > > Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
> > > > > the initial processor for all pv-shim vcpus will be 0, as no other cpus
> > > > > are online when the vcpus are created. Before that commit the vcpus
> > > > > would have processors set not being online yet, which worked just by
> > > > > chance.
> > 
> > So all vCPUs for the shim have their hard affinity set to pCPU#0 if I
> 
> No, the hard affinity is set to pcpu#(vcpu-id), but the initial cpu to
> run on is pcpu#0 as no other cpu is online when the vcpus are being
> created, and v->processor should always be a valid online cpu.

Oh, I didn't know v->processor must always be valid, even for offline
vCPUs. I'm quite sure the shim previously set v->processor to pCPUs
that where not yet online.

> > understand it correctly. From my reading of sched_setup_dom0_vcpus it
> > seems like in the shim case all sched units are pinned to their id,
> > which would imply sched units != 0 are not pinned to CPU#0?
> 
> Right.
> 
> > 
> > Or maybe there's only one sched unit that contains all the shim vCPUs?
> 
> No.
> 
> > 
> > > > > When the pv-shim vcpu becomes active it will have a hard affinity
> > > > > not matching its initial processor assignment leading to failing
> > > > > ASSERT()s or other problems depending on the selected scheduler.
> > > > 
> > > > I'm slightly lost here, who has set this hard affinity on the pvshim
> > > > vCPUs?
> > > 
> > > That is done in sched_setup_dom0_vcpus().
> > > 
> > > > 
> > > > > Fix that by redoing the affinity setting after onlining the cpu but
> > > > > before taking the vcpu up.
> > > > 
> > > > The change seems fine to me, but I don't understand why the lack of
> > > > this can cause asserts to trigger, as reported by Sergey. I also
> > > > wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
> > > > pv_shim_cpu_up is only used for APs.
> > > 
> > > When vcpu 0 is being created pcpu 0 is online already. So the affinity
> > > set in sched_setup_dom0_vcpus() is fine in that case.
> > 
> > IIRC all shim vCPUs where pinned to their identity pCPU at creation, and
> > there was no need to do this pining when the vCPU is brought online. I
> > guess this is no longer possible.
> 
> The problem is not the pinning, but the initial cpu stored in
> v->processor. This results in v->processor not being set in the hard
> affinity mask of the vcpu (or better: unit) which then triggers the
> problems.

I guess just setting v->processor in pv_shim_cpu_up directly would be
too intrusive?

In any case, it seems dangerous to allow vCPUs (even when offline) to
be in a state that when woken up will cause assertions inside the
scheduling logic. Ie: it would be best IMO to not set the hard
affinity in sched_setup_dom0_vcpus and instead set it when the pCPU is
brought online, or maybe have vcpu_wake select a suitable v->processor
value?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] PV-shim 4.13 assertion failures during vcpu_wake()

[Jürgen Groß]

On 22.10.19 15:50, Roger Pau Monné wrote:
> On Tue, Oct 22, 2019 at 01:50:44PM +0200, Jürgen Groß wrote:
>> On 22.10.19 13:25, Roger Pau Monné wrote:
>>> On Tue, Oct 22, 2019 at 01:01:09PM +0200, Jürgen Groß wrote:
>>>> On 22.10.19 12:52, Roger Pau Monné wrote:
>>>>> On Tue, Oct 22, 2019 at 11:27:41AM +0200, Jürgen Groß wrote:
>>>>>> Since commit 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
>>>>>> the initial processor for all pv-shim vcpus will be 0, as no other cpus
>>>>>> are online when the vcpus are created. Before that commit the vcpus
>>>>>> would have processors set not being online yet, which worked just by
>>>>>> chance.
>>>
>>> So all vCPUs for the shim have their hard affinity set to pCPU#0 if I
>>
>> No, the hard affinity is set to pcpu#(vcpu-id), but the initial cpu to
>> run on is pcpu#0 as no other cpu is online when the vcpus are being
>> created, and v->processor should always be a valid online cpu.
> 
> Oh, I didn't know v->processor must always be valid, even for offline
> vCPUs. I'm quite sure the shim previously set v->processor to pCPUs
> that where not yet online.

Yes, that's the reason I wrote it was working just by chance.

>>> understand it correctly. From my reading of sched_setup_dom0_vcpus it
>>> seems like in the shim case all sched units are pinned to their id,
>>> which would imply sched units != 0 are not pinned to CPU#0?
>>
>> Right.
>>
>>>
>>> Or maybe there's only one sched unit that contains all the shim vCPUs?
>>
>> No.
>>
>>>
>>>>>> When the pv-shim vcpu becomes active it will have a hard affinity
>>>>>> not matching its initial processor assignment leading to failing
>>>>>> ASSERT()s or other problems depending on the selected scheduler.
>>>>>
>>>>> I'm slightly lost here, who has set this hard affinity on the pvshim
>>>>> vCPUs?
>>>>
>>>> That is done in sched_setup_dom0_vcpus().
>>>>
>>>>>
>>>>>> Fix that by redoing the affinity setting after onlining the cpu but
>>>>>> before taking the vcpu up.
>>>>>
>>>>> The change seems fine to me, but I don't understand why the lack of
>>>>> this can cause asserts to trigger, as reported by Sergey. I also
>>>>> wonder why a change to pin vCPU#0 to pCPU#0 is not required, because
>>>>> pv_shim_cpu_up is only used for APs.
>>>>
>>>> When vcpu 0 is being created pcpu 0 is online already. So the affinity
>>>> set in sched_setup_dom0_vcpus() is fine in that case.
>>>
>>> IIRC all shim vCPUs where pinned to their identity pCPU at creation, and
>>> there was no need to do this pining when the vCPU is brought online. I
>>> guess this is no longer possible.
>>
>> The problem is not the pinning, but the initial cpu stored in
>> v->processor. This results in v->processor not being set in the hard
>> affinity mask of the vcpu (or better: unit) which then triggers the
>> problems.
> 
> I guess just setting v->processor in pv_shim_cpu_up directly would be
> too intrusive?

Doing that behind the scheduler's back is asking for trouble.

> In any case, it seems dangerous to allow vCPUs (even when offline) to
> be in a state that when woken up will cause assertions inside the
> scheduling logic. Ie: it would be best IMO to not set the hard
> affinity in sched_setup_dom0_vcpus and instead set it when the pCPU is
> brought online, or maybe have vcpu_wake select a suitable v->processor
> value?

Yes, maybe we should remove the affinity setting for all but vcpu0 from
sched_setup_dom0_vcpus().

In case Sergey can confirm the current patch is working I can resend it
with the affinity setting removed in sched_setup_dom0_vcpus().

All other cases should be fine already, so no need to tweak vcpu_wake().


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] (no subject)

[Sergey Dyasli]

On 22/10/2019 10:27, Jürgen Groß wrote:
> And now a more sane patch to try.
> 

The patch definitely fixes the issue for Null scheduler at least:

    Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>

It's still a bit unnerving to have places where sched_set_res() is called
without unit_schedule_lock(), but I hope that's by design.

Thanks,
Sergey

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel