All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH] xen/pvhsim: fix cpu onlining
@ 2019-10-23 12:12 Juergen Gross
  2019-10-23 12:55 ` Roger Pau Monné
  0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2019-10-23 12:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Wei Liu, George Dunlap, Andrew Cooper,
	Dario Faggioli, Jan Beulich, Roger Pau Monné

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 doing the affinity setting after onlining the cpu but
before taking the vcpu up. For vcpu 0 this is still in
sched_setup_dom0_vcpus(), for the other vcpus setting the affinity
there can be dropped.

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

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);
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index c327c40b92..326f4d3601 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -3102,13 +3102,12 @@ void __init sched_setup_dom0_vcpus(struct domain *d)
     for ( i = 1; i < d->max_vcpus; i++ )
         vcpu_create(d, i);
 
-    for_each_sched_unit ( d, unit )
+    if ( pv_shim )
+        sched_set_affinity(d->vcpu[0]->sched_unit,
+                           cpumask_of(0), cpumask_of(0));
+    else
     {
-        unsigned int id = unit->unit_id;
-
-        if ( pv_shim )
-            sched_set_affinity(unit, cpumask_of(id), cpumask_of(id));
-        else
+        for_each_sched_unit ( d, unit )
         {
             if ( !opt_dom0_vcpus_pin && !dom0_affinity_relaxed )
                 sched_set_affinity(unit, &dom0_cpus, NULL);
-- 
2.16.4


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH] xen/pvhsim: fix cpu onlining
  2019-10-23 12:12 [Xen-devel] [PATCH] xen/pvhsim: fix cpu onlining Juergen Gross
@ 2019-10-23 12:55 ` Roger Pau Monné
  2019-10-23 13:46   ` Jürgen Groß
  2019-10-23 13:53   ` Jan Beulich
  0 siblings, 2 replies; 4+ messages in thread
From: Roger Pau Monné @ 2019-10-23 12:55 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Dario Faggioli,
	Jan Beulich, xen-devel

On Wed, Oct 23, 2019 at 02:12:09PM +0200, Juergen Gross 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.
> 
> 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 doing the affinity setting after onlining the cpu but
> before taking the vcpu up. For vcpu 0 this is still in
> sched_setup_dom0_vcpus(), for the other vcpus setting the affinity
> there can be dropped.
> 
> Fixes: 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
> Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH] xen/pvhsim: fix cpu onlining
  2019-10-23 12:55 ` Roger Pau Monné
@ 2019-10-23 13:46   ` Jürgen Groß
  2019-10-23 13:53   ` Jan Beulich
  1 sibling, 0 replies; 4+ messages in thread
From: Jürgen Groß @ 2019-10-23 13:46 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Dario Faggioli,
	Jan Beulich, xen-devel

On 23.10.19 14:55, Roger Pau Monné wrote:
> On Wed, Oct 23, 2019 at 02:12:09PM +0200, Juergen Gross 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.
>>
>> 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 doing the affinity setting after onlining the cpu but
>> before taking the vcpu up. For vcpu 0 this is still in
>> sched_setup_dom0_vcpus(), for the other vcpus setting the affinity
>> there can be dropped.
>>
>> Fixes: 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
>> Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

And just for the protocol:

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH] xen/pvhsim: fix cpu onlining
  2019-10-23 12:55 ` Roger Pau Monné
  2019-10-23 13:46   ` Jürgen Groß
@ 2019-10-23 13:53   ` Jan Beulich
  1 sibling, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2019-10-23 13:53 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Dario Faggioli, xen-devel,
	Roger Pau Monné

On 23.10.2019 14:55, Roger Pau Monné wrote:
> On Wed, Oct 23, 2019 at 02:12:09PM +0200, Juergen Gross 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.
>>
>> 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 doing the affinity setting after onlining the cpu but
>> before taking the vcpu up. For vcpu 0 this is still in
>> sched_setup_dom0_vcpus(), for the other vcpus setting the affinity
>> there can be dropped.
>>
>> Fixes: 8d3c326f6756d1 ("xen: let vcpu_create() select processor")
>> Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

I have to admit though that I miss a comment on the pv_shim
conditional in schedule.c - such a special case shouldn't
really be there, but since it's needed it should be explained.
I realize though that the patch here only moves the special
case, i.e. the lack of comment is pre-existing.

Jan

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-10-23 13:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23 12:12 [Xen-devel] [PATCH] xen/pvhsim: fix cpu onlining Juergen Gross
2019-10-23 12:55 ` Roger Pau Monné
2019-10-23 13:46   ` Jürgen Groß
2019-10-23 13:53   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.