Re: [PATCH] use bitmap ops on a bitmap instead of bit ops

[Sheng Yang <sheng@linux.intel.com>]

On Friday 27 February 2009 21:26:58 Gleb Natapov wrote:
> 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:

Thanks. :)

Acked-by: Sheng Yang <sheng@linux.intel.com>

-- 
regards
Yang, Sheng

>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> 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.