Re: [PATCH] xen: Always ask the scheduler to re-place the vcpu when the affinity changes

[George Dunlap <george.dunlap@eu.citrix.com>]

On 04/03/13 12:35, Jan Beulich wrote:
>>>> On 04.03.13 at 13:19, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> It's probably a good idea to re-evaluate placement whenever the
>> affinity changes.
>>
>> This additionally has the benefit of removing scheduler-specific
>> exceptions introduced in git c/s e6a6fd63.
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>>   xen/common/schedule.c |    7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
>> index de11110..dbef5af 100644
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -613,9 +613,10 @@ int vcpu_set_affinity(struct vcpu *v, const cpumask_t
>> *affinity)
>>       vcpu_schedule_lock_irq(v);
>>   
>>       cpumask_copy(v->cpu_affinity, affinity);
>> -    if ( VCPU2OP(v)->sched_id == XEN_SCHEDULER_SEDF ||
>> -         !cpumask_test_cpu(v->processor, v->cpu_affinity) )
>> -        set_bit(_VPF_migrating, &v->pause_flags);
>> +
>> +    /* Always ask the scheduler to re-evaluate placement
>> +     * when changing the affinity */
>> +    set_bit(_VPF_migrating, &v->pause_flags);
>>   
>>       vcpu_schedule_unlock_irq(v);
>>   
> The code right below the context visible here is
>
>      if ( test_bit(_VPF_migrating, &v->pause_flags) )
>      {
>          vcpu_sleep_nosync(v);
>          vcpu_migrate(v);
>      }
>
> and I think the conditional could (and should) now be removed.

Yeah, I wasn't sure what to make of that one -- it looked as though it 
was coded such that someone else might be able to clear _VPF_migrating 
between releasing the lock and this test.  But if that can happen, it's 
racy anyway.  vcpu_force_reschedule() has this "set, unlock, re-check" 
pattern.

It looks like there actually is a way that it could conceivably be 
cleared: if the vcpu is running on another pcpu, if after we release the 
lock the vcpu is de-scheduled and context_saved() is called, it will 
check for _VPF_migrating and call vcpu_migrate(), which can clear the flag.

But that's probably a rare enough occurrence that it's better overall to 
take the occasional double-migrate.

  -George