Re: [PATCH] Avoid endless loop for vcpu migration

[Juergen Gross <juergen.gross@ts.fujitsu.com>]

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

On 03/15/11 10:01, Jan Beulich wrote:
>>>> On 15.03.11 at 09:46, Juergen Gross<juergen.gross@ts.fujitsu.com>  wrote:
>> On 03/15/11 08:57, Jan Beulich wrote:
>>>>>> On 15.03.11 at 06:50, Juergen Gross<juergen.gross@ts.fujitsu.com>   wrote:
>>>> On 03/14/11 16:03, Jan Beulich wrote:
>>>>>>>> On 14.03.11 at 15:39, Juergen Gross<juergen.gross@ts.fujitsu.com>    wrote:
>>>>>> On multi-thread multi-core systems an endless loop can occur in vcpu_migrate()
>>>>>> with credit scheduler. Avoid this loop by changing the interface of pick_cpu
>>>>>> to indicate a repeated call in this case.
>>>>>
>>>>> But you're not changing in any way the loop that doesn't get
>>>>> exited - did you perhaps read my original description as the
>>>>> pick function itself looping (which - afaict - it doesn't)?
>>>>
>>>> I'm changing the way the pick_cpu function is reacting on multiple calls in
>>>> a loop. If I've understood the idle_bias correctly, updating it in each
>>>> loop iteration did result in returning another cpu for each call.
>>>> By updating idle_bias only once, it should return the same cpu in subsequent
>>>> calls. This should exit the loop in vcpu_migrate.
>>>
>>> You're only decreasing the likelihood of a live lock, as the return
>>> value of pick_cpu not only depends on idle_bias.
>>
>> Hmm, then another solution would be to let pick_cpu really return the
>> proposed cpu from the first iteration, if it doesn't contradict the
>> allowed settings. It could be sub-optimal, but I don't think this is
>> critical, as vcpu_migrate is called rarely.
>>
>> Patch attached.
>
> That candidate-is-valid check seems absolutely independent of the
> particular scheduler used, and hence could be done in the (sole)
> caller, thus not requiring any change to the scheduler interface.
>
> Which at once would eliminate unnecessary calls into pick_cpu (i.e.
> you'd call it a second time only if the previously selected CPU really
> is no longer valid to be used for that vCPU).

True.

The patch seems to become smaller :-)


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

[-- Attachment #2: vcpu_migrate.patch --]
[-- Type: text/x-patch, Size: 1148 bytes --]

diff -r 9f6dec0d25cd xen/common/schedule.c
--- a/xen/common/schedule.c	Mon Mar 14 17:20:11 2011 +0000
+++ b/xen/common/schedule.c	Tue Mar 15 10:19:15 2011 +0100
@@ -395,6 +395,7 @@ static void vcpu_migrate(struct vcpu *v)
     unsigned long flags;
     unsigned int old_cpu, new_cpu;
     spinlock_t *old_lock, *new_lock;
+    int pick_called = 0;
 
     old_cpu = new_cpu = v->processor;
     for ( ; ; )
@@ -430,11 +431,18 @@ static void vcpu_migrate(struct vcpu *v)
         old_cpu = v->processor;
         if ( old_lock == per_cpu(schedule_data, old_cpu).schedule_lock )
         {
+            if ( pick_called && cpu_isset(new_cpu, v->cpu_affinity) &&
+                 cpu_isset(new_cpu, v->domain->cpupool->cpu_valid) )
+                break;
+
             new_cpu = SCHED_OP(VCPU2OP(v), pick_cpu, v);
             if ( (new_lock == per_cpu(schedule_data, new_cpu).schedule_lock) &&
                  cpu_isset(new_cpu, v->domain->cpupool->cpu_valid) )
                 break;
+            pick_called = 1;
         }
+        else
+            pick_called = 0;
 
         if ( old_lock != new_lock )
             spin_unlock(new_lock);

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

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