From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling Date: Thu, 17 Sep 2015 18:36:23 +0200 Message-ID: <1442507783.15327.135.camel@citrix.com> References: <1441960146-10569-1-git-send-email-feng.wu@intel.com> <1441960146-10569-16-git-send-email-feng.wu@intel.com> <1442423887.15327.29.camel@citrix.com> <1442479704.15327.65.camel@citrix.com> <55FA8A02.30705@citrix.com> <55FAA7AC.3010909@citrix.com> <1442493604.15327.80.camel@citrix.com> <55FACE83.6050607@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3486877492831418534==" Return-path: In-Reply-To: <55FACE83.6050607@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: "Tian, Kevin" , Keir Fraser , George Dunlap , Andrew Cooper , "xen-devel@lists.xen.org" , Jan Beulich , "Wu, Feng" List-Id: xen-devel@lists.xenproject.org --===============3486877492831418534== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-LIBemIvVJi4YhCqc5Zei" --=-LIBemIvVJi4YhCqc5Zei Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2015-09-17 at 15:30 +0100, George Dunlap wrote: > On 09/17/2015 01:40 PM, Dario Faggioli wrote: > >> I haven't yet decided whether I prefer my original suggestion of > >> switching the interrupt and putting things on the wake-up list in > >> vcpu_block(), or of deferring adding things to the wake-up list until > >> the actual context switch. > >> > > Sorry but I don't get what you mean with the latter. > >=20 > > I particular, I don't think I understand what you mean with and how it > > would work to "defer[ring] adding things to the wake-up list until > > actual context switch"... In what case would you defer stuff to context > > switch? >=20 > So one option is to do the "blocking" stuff in an arch-specific call > from vcpu_block(): >=20 > vcpu_block() > set(_VPF_blocked) > v->arch.block() > - Add v to pcpu.pi_blocked_vcpu > - NV =3D> pi_wakeup_vector > local_events_need_delivery() > hvm_vcpu_has_pending_irq() >=20 > ... > context_switch(): nothing >=20 > The other is to do the "blocking" stuff in the context switch (similar > to what's done now): >=20 > vcpu_block() > set(_VPF_blocked) > local_events_need_delivery() > hvm_vcpu_has_pending_irq() > ... > context_switch > v->arch.block() > - Add v to pcpu.pi_blocked_vcpu > - NV =3D> pi_wakeup_vector >=20 Ok, thanks for elaborating. > If we do it the first way, and an interrupt comes in before the context > switch is finished, it will call pi_wakeup_vector, which will DTRT -- > take v off the pi_blocked_vcpu list and call vcpu_unblock() (which in > turn will set NV back to posted_intr_vector). >=20 > If we do it the second way, and an interrupt comes in before the context > switch is finished, it will call posted_intr_vector. We can, at that > point, check to see if the current vcpu is marked as blocked. If it is, > we can call vcpu_unblock() without having to modify NV or worry about > adding / removing the vcpu from the pi_blocked_vcpu list. >=20 Right. > The thing I like about the first one is that it makes PI blocking the > same as normal blocking -- everything happens in the same place; so the > code is cleaner and easier to understand. >=20 Indeed. > The thing I like about the second one is that it cleverly avoids having > to do all the work of adding the vcpu to the list and then searching to > remove it if the vcpu in question gets woken up on the way to being > blocked. So the code may end up being faster for workloads where that > happens frequently. >=20 Maybe some instrumentation to figure out how frequent this is, at least in a couple of (thought to be) common workloads can be added, and a few test performed? Feng? One thing that may be worth giving some thinking at is whether interrupts are enabled or not. I mean, during vcpu_block() SCHEDULE_SOFTIRQ is raised, for invoking the scheduler. Then (at least) during schedule(), IRQs are disabled. They're re-enabled near the end of schedule(), and re-disabled for the actual context switch ( ~=3D around __context_switch()). My point being that IRQs are going to be disabled for a significant amount of time, between vcpu_block() and the actual context being switched. And solution 2 requires IRQs to be enabled in order to avoid a potentially pointless NV update, doesn't it? If yes, that may (negatively) affect the probability of being able to actually benefit from the optimization... Anyway, I'm not sure. I think my gut feelings are in favour of solution 1, but, really, it's hard to tell without actually trying.=20 Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-LIBemIvVJi4YhCqc5Zei 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 v2 iEYEABECAAYFAlX67AcACgkQk4XaBE3IOsRUPwCeMAf/dNuTCnf+d2demCRp+xW6 MZQAn17EL7Kmllp698DNXnmV/xwlLrrT =vw4r -----END PGP SIGNATURE----- --=-LIBemIvVJi4YhCqc5Zei-- --===============3486877492831418534== 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 --===============3486877492831418534==--