diff mbox series

[v2,2/3] xen/sched: don't call sync_vcpu_execstate() in sched_unit_migrate_finish()

Message ID 20200511112829.5500-3-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series xen: Fix some bugs in scheduling | expand

Commit Message

Jürgen Groß May 11, 2020, 11:28 a.m. UTC
With support of core scheduling sched_unit_migrate_finish() gained a
call of sync_vcpu_execstate() as it was believed to be called as a
result of vcpu migration in any case.

In case of migrating a vcpu away from a physical cpu for a short period
of time only this might not be true, so drop the call and let the lazy
state syncing do its job.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
 xen/common/sched/core.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

Jan Beulich May 14, 2020, 1:57 p.m. UTC | #1
On 11.05.2020 13:28, Juergen Gross wrote:
> With support of core scheduling sched_unit_migrate_finish() gained a
> call of sync_vcpu_execstate() as it was believed to be called as a
> result of vcpu migration in any case.
> 
> In case of migrating a vcpu away from a physical cpu for a short period
> of time only this might not be true, so drop the call and let the lazy
> state syncing do its job.

Replying here instead of on the patch 3 thread (and I'm sorry
for mixing up function names there): By saying "for a short
period of time only", do you imply without ever getting scheduled
on the new (temporary) CPU? If so, I think I understand this
change now, but then this could do with saying here. If not, I'm
afraid I'm still lost.

Jan
Jürgen Groß May 14, 2020, 2:21 p.m. UTC | #2
On 14.05.20 15:57, Jan Beulich wrote:
> On 11.05.2020 13:28, Juergen Gross wrote:
>> With support of core scheduling sched_unit_migrate_finish() gained a
>> call of sync_vcpu_execstate() as it was believed to be called as a
>> result of vcpu migration in any case.
>>
>> In case of migrating a vcpu away from a physical cpu for a short period
>> of time only this might not be true, so drop the call and let the lazy
>> state syncing do its job.
> 
> Replying here instead of on the patch 3 thread (and I'm sorry
> for mixing up function names there): By saying "for a short
> period of time only", do you imply without ever getting scheduled
> on the new (temporary) CPU? If so, I think I understand this
> change now, but then this could do with saying here. If not, I'm
> afraid I'm still lost.

I'll change the commit message to:

... for a short period of time only without ever being scheduled on the
selected new cpu ...


Juergen
Jan Beulich May 14, 2020, 2:59 p.m. UTC | #3
On 14.05.2020 16:21, Jürgen Groß wrote:
> On 14.05.20 15:57, Jan Beulich wrote:
>> On 11.05.2020 13:28, Juergen Gross wrote:
>>> With support of core scheduling sched_unit_migrate_finish() gained a
>>> call of sync_vcpu_execstate() as it was believed to be called as a
>>> result of vcpu migration in any case.
>>>
>>> In case of migrating a vcpu away from a physical cpu for a short period
>>> of time only this might not be true, so drop the call and let the lazy
>>> state syncing do its job.
>>
>> Replying here instead of on the patch 3 thread (and I'm sorry
>> for mixing up function names there): By saying "for a short
>> period of time only", do you imply without ever getting scheduled
>> on the new (temporary) CPU? If so, I think I understand this
>> change now, but then this could do with saying here. If not, I'm
>> afraid I'm still lost.
> 
> I'll change the commit message to:
> 
> ... for a short period of time only without ever being scheduled on the
> selected new cpu ...

And then
Reviewed-by: Jan Beulich <jbeulich@suse.com>
for both this one and patch 3 (ideally with the one unnecessary hunk
dropped).

Jan
diff mbox series

Patch

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 5df66cbf9b..cb49a8bc02 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -1078,12 +1078,7 @@  static void sched_unit_migrate_finish(struct sched_unit *unit)
     sched_spin_unlock_double(old_lock, new_lock, flags);
 
     if ( old_cpu != new_cpu )
-    {
-        /* Vcpus are moved to other pcpus, commit their states to memory. */
-        for_each_sched_unit_vcpu ( unit, v )
-            sync_vcpu_execstate(v);
         sched_move_irqs(unit);
-    }
 
     /* Wake on new CPU. */
     for_each_sched_unit_vcpu ( unit, v )