From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] Avoid endless loop for vcpu migration Date: Tue, 15 Mar 2011 10:21:28 +0100 Message-ID: <4D7F2F98.8090900@ts.fujitsu.com> References: <9d164ce877a75cab847b.1300113594@nehalem1> <4D7E3C640200007800036564@vpn.id2.novell.com> <4D7EFE43.7070900@ts.fujitsu.com> <4D7F29ED02000078000367F4@vpn.id2.novell.com> <4D7F2750.4010607@ts.fujitsu.com> <4D7F390D020000780003683C@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------090900020208090706000600" Return-path: In-Reply-To: <4D7F390D020000780003683C@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------090900020208090706000600 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 03/15/11 10:01, Jan Beulich wrote: >>>> On 15.03.11 at 09:46, Juergen Gross wrote: >> On 03/15/11 08:57, Jan Beulich wrote: >>>>>> On 15.03.11 at 06:50, Juergen Gross wrote: >>>> On 03/14/11 16:03, Jan Beulich wrote: >>>>>>>> On 14.03.11 at 15:39, Juergen Gross 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 --------------090900020208090706000600 Content-Type: text/x-patch; name="vcpu_migrate.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="vcpu_migrate.patch" 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); --------------090900020208090706000600 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------090900020208090706000600--