From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS Date: Wed, 4 Nov 2015 16:52:01 +0100 Message-ID: <1446652321.3829.127.camel@citrix.com> References: <20151029225158.25219.4625.stgit@Solace.station> <20151029230420.25219.74544.stgit@Solace.station> <1446451425.2750.3.camel@citrix.com> <1446646375.3829.108.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6660038360266568332==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Zu0M4-0006EZ-3K for xen-devel@lists.xenproject.org; Wed, 04 Nov 2015 15:52:08 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Meng Xu Cc: George Dunlap , "xen-devel@lists.xenproject.org" , Meng Xu List-Id: xen-devel@lists.xenproject.org --===============6660038360266568332== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-0J+crj5l6JOu6loDmQvE" --=-0J+crj5l6JOu6loDmQvE Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2015-11-04 at 10:01 -0500, Meng Xu wrote: > 2015-11-04 9:12 GMT-05:00 Dario Faggioli : > > Just FTR (and for next time :-D), is the above something that can > > be > > interpreted as a 'Reviewed-by: Meng Xu ' ? If no (e.g., > > because > > you haven't looking thoroughly enough to feel confident to express > > it), > > then fine, I was just asking. > Thank you very much for explaining this for me. :-)=20 >=20 > I feel confident about the changes for RTDS scheduler.=20 > Ok. > I'm not so confident about the change in the schedule.c. To be > specific, this patch removes insert_vcpu in schedule_cpu_switch() in > schedule.c;=20 > It removes the attempt of inserting the idle vCPU in the runqueue of the scheduler of the target cpupool for the pCPU. More specifically, this line: SCHED_OP(new_ops, insert_vcpu, idle); If we look at the various ways in which insert_vcpu is implemented, we have: csched_vcpu_insert: if ( !__vcpu_on_runq(svc) && vcpu_runnable(vc) && !vc->is_running ) __runq_insert(vc->processor, svc); but the pCPU being switched is free, i.e., it is not in any cpupool, and it is idling. So, the idle vCPU is running, and the condition above is false, which means __runq_insert() is not really called. csched2_vcpu_insert: if ( ! is_idle_vcpu(vc) ) { ... } so trying to insert the idle vCPU is actually a nop. rt_vcpu_insert: if ( is_idle_vcpu(vc) ) return; a nop again. My point being that this patch actually removes nothing but a bunch of if()-s, with no effect at all. > I'm not so sure if it is ok to insert_vcpu when a domain is moved. > Hopefully, I addressed your doubts. BTW, we're not talking about a domain being moved between pools. That is done with another function. schedule_cpu_switch() is about taking a pCPU off from a cpupool, or assigning a pCPU to cpupool. > (Next time, I will stand out and ask although it may be a stupid > question. ;-) ) >=20 Sure! And this does not look a stupid question to me. :-) > So as to this patch, I will say: > As far as the RTDS scheduler is concerned: Reviewed-by: Meng Xu < > mengxu@cis.upenn.edu> > =20 Ok, I haven't sent v4 yet, so I'll apply it there. Thanks. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-0J+crj5l6JOu6loDmQvE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEABECAAYFAlY6KaEACgkQk4XaBE3IOsQWdACfSkvbg5qC2Nz8hUupoWe9goHn awQAn3gecP6TYqhnXLSmW22L64+QD6nG =Xypa -----END PGP SIGNATURE----- --=-0J+crj5l6JOu6loDmQvE-- --===============6660038360266568332== 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.xen.org http://lists.xen.org/xen-devel --===============6660038360266568332==--