From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Date: Mon, 13 Oct 2008 06:22:54 +0000 Subject: Re: [PATCH 1/3] kvmppc: optimize irq delivery path Message-Id: <48F2E93E.50604@linux.vnet.ibm.com> List-Id: References: <1223636372-8946-3-git-send-email-ehrhardt@linux.vnet.ibm.com> In-Reply-To: <1223636372-8946-3-git-send-email-ehrhardt@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: kvm-ppc@vger.kernel.org Hollis Blanchard wrote: > On Fri, 2008-10-10 at 12:59 +0200, ehrhardt@linux.vnet.ibm.com wrote: > =20 >> From: Christian Ehrhardt >> >> In kvmppc_deliver_interrupt is just one case left in the switch and it i= s a >> rare one (less than 8%) when looking at the exit numbers. Therefore we c= an >> at least drop the switch/case and if an if. I inserted an unlikely too, = but >> that's open for discussion. >> >> In kvmppc_can_deliver_interrupt all frequent cases are in the default ca= se. >> I know compilers are smart but we can make it easier for them. By writing >> down all options and removing the default case combined with the fact th= at >> ithe values are constants 0..15 should allow the compiler to write an ea= sy >> jump table. >> Modifying kvmppc_can_deliver_interrupt pointed me to the fact that gcc s= eems >> to be unable to reduce priority_exception[x] to a build time constant. >> Therefore I changed the usage of the translation arrays in the interrupt >> delivery path completely. It is now using priority without translation t= o irq >> on the full irq delivery path. >> To be able to do that ivpr regs are stored by their priority now. >> =20 > > I like this, but a few notes: > > Why did you convert exception_priority[] to int? AFAICS it still only > holds values 0-15, and using chars would shrink it from 2 32-byte > cachelines (3 unaligned) to half of one. > =20 Your right, I changed it while modifying the code to match the type used=20 to call functions for a test and forgot to revert it. Will be changed in v2. > It looks like you applied this on top of the pvmem patches. I guess the > performance benefit is significant enough to apply those, but I'll have > to find the patches again. > =20 I can rebase them to bring them in pre pvmem, no need you do that work. This also fixed the dependencies to e.g. the exit timing patches. Applying pv patches too would be nice, we can talk this week about that=20 more in detail (if/when/how to do it). > I think the IVOR emulation has gotten wide enough that you can add some > newlines. > =20 I need to run checkpatch once more on the current version anyway - I=20 just wanted to put that out for discussion on friday, thats why I missed=20 those 80+ lines :-/ > AFAICS the *_PRIO constants aren't used in assembly, so do they need to > go into kvm_asm.h? > =20 I wanted to place them near the non _PRIO irq constants, not needed for=20 the code but nice to understand the code. But your right, officially I think they should stay in asm/kvm_ppc.h - I=20 can relocate them in v2 of the patches. --=20 Gr=FCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization