From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH -v4] KVM: x86: lapic: Clean up find_highest_vector() and count_vectors() Date: Wed, 5 Sep 2012 13:58:29 +0300 Message-ID: <20120905105829.GA10994@redhat.com> References: <20120830104458.GA19997@redhat.com> <20120830213019.82a822da.yoshikawa.takuya@oss.ntt.co.jp> <20120830132131.GB21132@redhat.com> <20120831010956.000856ead3c6f96dbe2bec46@gmail.com> <20120830164923.GA22024@redhat.com> <20120905173031.59581bb9.yoshikawa.takuya@oss.ntt.co.jp> <20120905092649.GD10326@redhat.com> <20120905184026.9aeddbc1.yoshikawa.takuya@oss.ntt.co.jp> <20120905095126.GA10531@redhat.com> <20120905193001.99d06c26.yoshikawa.takuya@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@redhat.com, mtosatti@redhat.com, kvm@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59252 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750979Ab2IEK5I (ORCPT ); Wed, 5 Sep 2012 06:57:08 -0400 Content-Disposition: inline In-Reply-To: <20120905193001.99d06c26.yoshikawa.takuya@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Sep 05, 2012 at 07:30:01PM +0900, Takuya Yoshikawa wrote: > find_highest_vector() and count_vectors(): > - Instead of using magic values, define and use proper macros. > > find_highest_vector(): > - Remove likely() which is there only for historical reasons and not > doing correct branch predictions anymore. Using such heuristics > to optimize this function is not worth it now. Let CPUs predict > things instead. > > - Stop checking word[0] separately. This was only needed for doing > likely() optimization. > > - Use for loop, not while, to iterate over the register array to make > the code clearer. > > Note that we actually confirmed that the likely() did wrong predictions > by inserting debug code. > > Signed-off-by: Takuya Yoshikawa > Cc: Michael S. Tsirkin Looks like a nice cleanup. Acked-by: Michael S. Tsirkin > --- > arch/x86/kvm/lapic.c | 30 ++++++++++++++++++------------ > 1 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 18d149d..8ace252 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -66,6 +66,7 @@ > #define APIC_DEST_NOSHORT 0x0 > #define APIC_DEST_MASK 0x800 > #define MAX_APIC_VECTOR 256 > +#define APIC_VECTORS_PER_REG 32 > > #define VEC_POS(v) ((v) & (32 - 1)) > #define REG_POS(v) (((v) >> 5) << 4) > @@ -208,25 +209,30 @@ static unsigned int apic_lvt_mask[APIC_LVT_NUM] = { > > static int find_highest_vector(void *bitmap) > { > - u32 *word = bitmap; > - int word_offset = MAX_APIC_VECTOR >> 5; > + int vec; > + u32 *reg; > > - while ((word_offset != 0) && (word[(--word_offset) << 2] == 0)) > - continue; > + for (vec = MAX_APIC_VECTOR - APIC_VECTORS_PER_REG; > + vec >= 0; vec -= APIC_VECTORS_PER_REG) { > + reg = bitmap + REG_POS(vec); > + if (*reg) > + return fls(*reg) - 1 + vec; > + } > > - if (likely(!word_offset && !word[0])) > - return -1; > - else > - return fls(word[word_offset << 2]) - 1 + (word_offset << 5); > + return -1; > } > > static u8 count_vectors(void *bitmap) > { > - u32 *word = bitmap; > - int word_offset; > + int vec; > + u32 *reg; > u8 count = 0; > - for (word_offset = 0; word_offset < MAX_APIC_VECTOR >> 5; ++word_offset) > - count += hweight32(word[word_offset << 2]); > + > + for (vec = 0; vec < MAX_APIC_VECTOR; vec += APIC_VECTORS_PER_REG) { > + reg = bitmap + REG_POS(vec); > + count += hweight32(*reg); > + } > + > return count; > } > > -- > 1.7.5.4