Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-30 Thread Michael S. Tsirkin
On Thu, Aug 30, 2012 at 07:24:39PM +0900, Takuya Yoshikawa wrote: > On Thu, 30 Aug 2012 13:10:33 +0300 > "Michael S. Tsirkin" wrote: > > > > OK, I'll do these on top of this patch. > > > > Tweaking these 5 lines for readability across multiple > > patches is just not worth it. > > As long as we

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-30 Thread Takuya Yoshikawa
On Thu, 30 Aug 2012 13:10:33 +0300 "Michael S. Tsirkin" wrote: > > OK, I'll do these on top of this patch. > > Tweaking these 5 lines for readability across multiple > patches is just not worth it. > As long as we do random cleanups of this function it's probably easier > to just do them all in

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-30 Thread Michael S. Tsirkin
On Thu, Aug 30, 2012 at 06:50:52PM +0900, Takuya Yoshikawa wrote: > On Thu, 30 Aug 2012 09:37:02 +0300 > "Michael S. Tsirkin" wrote: > > > After staring at your code for a while it does appear to > > do the right thing, and looks cleaner than what > > we have now. commit log could be clearer. > >

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-30 Thread Takuya Yoshikawa
On Thu, 30 Aug 2012 09:37:02 +0300 "Michael S. Tsirkin" wrote: > After staring at your code for a while it does appear to > do the right thing, and looks cleaner than what > we have now. commit log could be clearer. > It should state something like: > > > Clean up code in find_highest_vector: >

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-29 Thread Michael S. Tsirkin
On Thu, Aug 30, 2012 at 10:09:06AM +0900, Takuya Yoshikawa wrote: > On Thu, 30 Aug 2012 01:51:20 +0300 > "Michael S. Tsirkin" wrote: > > > This text: > > + if (likely(!word_offset && !word[0])) > > + return -1; > > is a left-over from the original implementation. > > > > Ther

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-29 Thread Takuya Yoshikawa
On Thu, 30 Aug 2012 01:51:20 +0300 "Michael S. Tsirkin" wrote: > This text: > + if (likely(!word_offset && !word[0])) > + return -1; > is a left-over from the original implementation. > > There we did a ton of gratitious calls to interrupt > injection so it was important to s

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-29 Thread Michael S. Tsirkin
On Wed, Aug 29, 2012 at 04:10:57PM -0300, Marcelo Tosatti wrote: > On Tue, Aug 28, 2012 at 06:57:56PM +0900, Takuya Yoshikawa wrote: > > On Mon, 27 Aug 2012 17:25:42 -0300 > > Marcelo Tosatti wrote: > > > > > On Fri, Aug 24, 2012 at 06:15:49PM +0900, Takuya Yoshikawa wrote: > > > > Although retur

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-29 Thread Marcelo Tosatti
On Tue, Aug 28, 2012 at 06:57:56PM +0900, Takuya Yoshikawa wrote: > On Mon, 27 Aug 2012 17:25:42 -0300 > Marcelo Tosatti wrote: > > > On Fri, Aug 24, 2012 at 06:15:49PM +0900, Takuya Yoshikawa wrote: > > > Although returning -1 should be likely according to the likely(), > > > the ASSERT in apic_

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-28 Thread Takuya Yoshikawa
On Mon, 27 Aug 2012 17:25:42 -0300 Marcelo Tosatti wrote: > On Fri, Aug 24, 2012 at 06:15:49PM +0900, Takuya Yoshikawa wrote: > > Although returning -1 should be likely according to the likely(), > > the ASSERT in apic_find_highest_irr() will be triggered in such a case. > > It seems that this op

Re: [PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-27 Thread Marcelo Tosatti
On Fri, Aug 24, 2012 at 06:15:49PM +0900, Takuya Yoshikawa wrote: > Although returning -1 should be likely according to the likely(), > the ASSERT in apic_find_highest_irr() will be triggered in such a case. > It seems that this optimization is not working as expected. > > This patch simplifies th

[PATCH] KVM: x86: lapic: Fix the misuse of likely() in find_highest_vector()

2012-08-24 Thread Takuya Yoshikawa
Although returning -1 should be likely according to the likely(), the ASSERT in apic_find_highest_irr() will be triggered in such a case. It seems that this optimization is not working as expected. This patch simplifies the logic to mitigate this issue: search for the first non-zero word in a for