[PATCH] trivial fix for vcpu_set_affinity

[PATCH] trivial fix for vcpu_set_affinity

[Tian, Kevin]

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

No need to try migration in current context since self migrate
will be handled by next vcpu after context switch.

Signed-off-by Kevin Tian <kevin.tian@intel.com>

diff -r e66f047bc97e xen/common/schedule.c
--- a/xen/common/schedule.c	Tue Jan 09 18:56:44 2007 -0800
+++ b/xen/common/schedule.c	Thu Jan 11 16:31:37 2007 +0800
@@ -243,7 +243,7 @@ int vcpu_set_affinity(struct vcpu *v, cp
 
     vcpu_schedule_unlock_irqrestore(v, flags);
 
-    if ( test_bit(_VCPUF_migrating, &v->vcpu_flags) )
+    if ( (v != current) && test_bit(_VCPUF_migrating, &v->vcpu_flags) )
     {
         vcpu_sleep_nosync(v);
         vcpu_migrate(v);

Thanks,
Kevin

[-- Attachment #2: set_affinity.patch --]
[-- Type: application/octet-stream, Size: 627 bytes --]

No need to try migrate in current context since self migrate
will be handled by next vcpu after context switch.

Signed-off-by Kevin Tian <kevin.tian@intel.com>
diff -r e66f047bc97e xen/common/schedule.c
--- a/xen/common/schedule.c	Tue Jan 09 18:56:44 2007 -0800
+++ b/xen/common/schedule.c	Thu Jan 11 16:31:37 2007 +0800
@@ -243,7 +243,7 @@ int vcpu_set_affinity(struct vcpu *v, cp
 
     vcpu_schedule_unlock_irqrestore(v, flags);
 
-    if ( test_bit(_VCPUF_migrating, &v->vcpu_flags) )
+    if ( (v != current) && test_bit(_VCPUF_migrating, &v->vcpu_flags) )
     {
         vcpu_sleep_nosync(v);
         vcpu_migrate(v);

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] trivial fix for vcpu_set_affinity

[Keir Fraser]

It's true that the vcpu_migrate() is a no-op if v==current, but the
vcpu_sleep_nosync() does have to be executed, otherwise current will not be
migrated before returning to guest context (because the schedule softirq
will not be asserted). This would mean that a dom0 vcpu could no longer
synchronously migrate itself. In any case this patch has no upside -- the
existing code works just fine.

 -- Keir

On 11/1/07 08:42, "Tian, Kevin" <kevin.tian@intel.com> wrote:

> No need to try migration in current context since self migrate
> will be handled by next vcpu after context switch.
> 
> Signed-off-by Kevin Tian <kevin.tian@intel.com>
> 
> diff -r e66f047bc97e xen/common/schedule.c
> --- a/xen/common/schedule.c Tue Jan 09 18:56:44 2007 -0800
> +++ b/xen/common/schedule.c Thu Jan 11 16:31:37 2007 +0800
> @@ -243,7 +243,7 @@ int vcpu_set_affinity(struct vcpu *v, cp
>  
>      vcpu_schedule_unlock_irqrestore(v, flags);
>  
> -    if ( test_bit(_VCPUF_migrating, &v->vcpu_flags) )
> +    if ( (v != current) && test_bit(_VCPUF_migrating, &v->vcpu_flags) )
>      {
>          vcpu_sleep_nosync(v);
>          vcpu_migrate(v);
> 
> Thanks,
> Kevin
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] trivial fix for vcpu_set_affinity

[Emmanuel Ackaouy]

I don't get it.

If you don't at least call vcpu_sleep_nosync() on the current context
when it changes affinity, how do you initiate a reschedule event?

What's wrong with the code as is?

On Jan 11, 2007, at 9:42, Tian, Kevin wrote:

> No need to try migration in current context since self migrate
> will be handled by next vcpu after context switch.
>
> Signed-off-by Kevin Tian <kevin.tian@intel.com>
>
> diff -r e66f047bc97e xen/common/schedule.c
> --- a/xen/common/schedule.c	Tue Jan 09 18:56:44 2007 -0800
> +++ b/xen/common/schedule.c	Thu Jan 11 16:31:37 2007 +0800
> @@ -243,7 +243,7 @@ int vcpu_set_affinity(struct vcpu *v, cp
>
>      vcpu_schedule_unlock_irqrestore(v, flags);
>
> -    if ( test_bit(_VCPUF_migrating, &v->vcpu_flags) )
> +    if ( (v != current) && test_bit(_VCPUF_migrating, &v->vcpu_flags) 
> )
>      {
>          vcpu_sleep_nosync(v);
>          vcpu_migrate(v);
>
> Thanks,
> Kevin
> <set_affinity.patch>_______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [PATCH] trivial fix for vcpu_set_affinity

[Tian, Kevin]

Yes, you're right. Though I test this patch working, it may be instead 
migrated at later schedule point instead of immediately by this hypercall.

Thanks,
Kevin

>-----Original Message-----
>From: Keir Fraser [mailto:keir@xensource.com]
>Sent: 2007年1月11日 17:44
>To: Tian, Kevin; xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] [PATCH] trivial fix for vcpu_set_affinity
>
>It's true that the vcpu_migrate() is a no-op if v==current, but the
>vcpu_sleep_nosync() does have to be executed, otherwise current will
>not be
>migrated before returning to guest context (because the schedule softirq
>will not be asserted). This would mean that a dom0 vcpu could no longer
>synchronously migrate itself. In any case this patch has no upside -- the
>existing code works just fine.
>
> -- Keir
>
>On 11/1/07 08:42, "Tian, Kevin" <kevin.tian@intel.com> wrote:
>
>> No need to try migration in current context since self migrate
>> will be handled by next vcpu after context switch.
>>
>> Signed-off-by Kevin Tian <kevin.tian@intel.com>
>>
>> diff -r e66f047bc97e xen/common/schedule.c
>> --- a/xen/common/schedule.c Tue Jan 09 18:56:44 2007 -0800
>> +++ b/xen/common/schedule.c Thu Jan 11 16:31:37 2007 +0800
>> @@ -243,7 +243,7 @@ int vcpu_set_affinity(struct vcpu *v, cp
>>
>>      vcpu_schedule_unlock_irqrestore(v, flags);
>>
>> -    if ( test_bit(_VCPUF_migrating, &v->vcpu_flags) )
>> +    if ( (v != current) && test_bit(_VCPUF_migrating,
>&v->vcpu_flags) )
>>      {
>>          vcpu_sleep_nosync(v);
>>          vcpu_migrate(v);
>>
>> Thanks,
>> Kevin
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel

RE: [PATCH] trivial fix for vcpu_set_affinity

[Tian, Kevin]

Yes, I forgot about the reschedule event here, and thus migration is 
scheduled in other schedule points triggered later. Please forget this 
patch. :-)

Thanks
Kevin

>-----Original Message-----
>From: Emmanuel Ackaouy [mailto:ack@xensource.com]
>Sent: 2007年1月11日 17:49
>To: Tian, Kevin
>Cc: xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] [PATCH] trivial fix for vcpu_set_affinity
>
>I don't get it.
>
>If you don't at least call vcpu_sleep_nosync() on the current context
>when it changes affinity, how do you initiate a reschedule event?
>
>What's wrong with the code as is?
>
>On Jan 11, 2007, at 9:42, Tian, Kevin wrote:
>
>> No need to try migration in current context since self migrate
>> will be handled by next vcpu after context switch.
>>
>> Signed-off-by Kevin Tian <kevin.tian@intel.com>
>>
>> diff -r e66f047bc97e xen/common/schedule.c
>> --- a/xen/common/schedule.c	Tue Jan 09 18:56:44 2007 -0800
>> +++ b/xen/common/schedule.c	Thu Jan 11 16:31:37 2007 +0800
>> @@ -243,7 +243,7 @@ int vcpu_set_affinity(struct vcpu *v, cp
>>
>>      vcpu_schedule_unlock_irqrestore(v, flags);
>>
>> -    if ( test_bit(_VCPUF_migrating, &v->vcpu_flags) )
>> +    if ( (v != current) && test_bit(_VCPUF_migrating,
>&v->vcpu_flags)
>> )
>>      {
>>          vcpu_sleep_nosync(v);
>>          vcpu_migrate(v);
>>
>> Thanks,
>> Kevin
>>
><set_affinity.patch>__________________________________________
>_____
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel