From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH] use bitmap ops on a bitmap instead of bit ops Date: Fri, 27 Feb 2009 15:26:58 +0200 Message-ID: <20090227132658.GA10118@redhat.com> References: <20090226130339.GG8810@redhat.com> <200902271119.12359.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@redhat.com, kvm@vger.kernel.org To: Sheng Yang Return-path: Received: from mx2.redhat.com ([66.187.237.31]:51374 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754607AbZB0NaL (ORCPT ); Fri, 27 Feb 2009 08:30:11 -0500 Content-Disposition: inline In-Reply-To: <200902271119.12359.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Feb 27, 2009 at 11:19:09AM +0800, Sheng Yang wrote: > > - *mask = 0; > > I think the caller should not assume anything about the output, so let > function itself do the clean is better. (however, not in this way for "mask" > should be KVM_MAX_VCPUS long.) > Ok. I'll leave it although all callers of the function zeroes bitmask anyway. > And, I found some other things are not proper after looking back code, I would > send a patch to fix it (deliver_bitmask in kvm_get_intr_delivery_bitmask). > > > if (dest_mode == 0) { /* Physical mode. */ > > if (dest == 0xFF) { /* Broadcast. */ > > for (i = 0; i < KVM_MAX_VCPUS; ++i) > > if (kvm->vcpus[i] && kvm->vcpus[i]->arch.apic) > > - *mask |= 1 << i; > > + __set_bit(i, *mask); > > Should be __set_bit(i, mask)? > Oh. Here is updated patch: Signed-off-by: Gleb Natapov diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index 7c2cb2b..7bd27aa 100644 --- a/virt/kvm/ioapic.c +++ b/virt/kvm/ioapic.c @@ -170,12 +170,13 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest, ioapic_debug("dest %d dest_mode %d\n", dest, dest_mode); - *mask = 0; + bitmap_zero(mask, KVM_MAX_VCPUS); + if (dest_mode == 0) { /* Physical mode. */ if (dest == 0xFF) { /* Broadcast. */ for (i = 0; i < KVM_MAX_VCPUS; ++i) if (kvm->vcpus[i] && kvm->vcpus[i]->arch.apic) - *mask |= 1 << i; + __set_bit(i, mask); return; } for (i = 0; i < KVM_MAX_VCPUS; ++i) { @@ -184,7 +185,7 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest, continue; if (kvm_apic_match_physical_addr(vcpu->arch.apic, dest)) { if (vcpu->arch.apic) - *mask = 1 << i; + __set_bit(i, mask); break; } } @@ -195,7 +196,7 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest, continue; if (vcpu->arch.apic && kvm_apic_match_logical_addr(vcpu->arch.apic, dest)) - *mask |= 1 << vcpu->vcpu_id; + __set_bit(i, mask); } ioapic_debug("mask %x\n", *mask); } -- Gleb.