* [PATCH] use bitmap ops on a bitmap instead of bit ops
@ 2009-02-26 13:03 Gleb Natapov
2009-02-27 3:19 ` Sheng Yang
0 siblings, 1 reply; 5+ messages in thread
From: Gleb Natapov @ 2009-02-26 13:03 UTC (permalink / raw)
To: avi; +Cc: Sheng Yang, kvm
And don't zero bitmap twice. Assume that caller zeros it.
Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 7c2cb2b..21627b9 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -170,12 +170,11 @@ 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;
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 +183,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 +194,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.
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] use bitmap ops on a bitmap instead of bit ops
2009-02-26 13:03 [PATCH] use bitmap ops on a bitmap instead of bit ops Gleb Natapov
@ 2009-02-27 3:19 ` Sheng Yang
2009-02-27 13:26 ` Gleb Natapov
0 siblings, 1 reply; 5+ messages in thread
From: Sheng Yang @ 2009-02-27 3:19 UTC (permalink / raw)
To: Gleb Natapov; +Cc: avi, kvm
On Thursday 26 February 2009 21:03:39 Gleb Natapov wrote:
> And don't zero bitmap twice. Assume that caller zeros it.
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 7c2cb2b..21627b9 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -170,12 +170,11 @@ void kvm_ioapic_get_delivery_bitmask(struct
> kvm_ioapic *ioapic, u8 dest,
>
> ioapic_debug("dest %d dest_mode %d\n", dest, dest_mode);
>
Hi Gleb
> - *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.)
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)?
--
regards
Yang, Sheng
> return;
> }
> for (i = 0; i < KVM_MAX_VCPUS; ++i) {
> @@ -184,7 +183,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 +194,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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] use bitmap ops on a bitmap instead of bit ops
2009-02-27 3:19 ` Sheng Yang
@ 2009-02-27 13:26 ` Gleb Natapov
2009-03-02 7:12 ` Sheng Yang
0 siblings, 1 reply; 5+ messages in thread
From: Gleb Natapov @ 2009-02-27 13:26 UTC (permalink / raw)
To: Sheng Yang; +Cc: avi, kvm
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 <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.
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] use bitmap ops on a bitmap instead of bit ops
2009-02-27 13:26 ` Gleb Natapov
@ 2009-03-02 7:12 ` Sheng Yang
2009-03-03 1:28 ` Sheng Yang
0 siblings, 1 reply; 5+ messages in thread
From: Sheng Yang @ 2009-03-02 7:12 UTC (permalink / raw)
To: Gleb Natapov; +Cc: avi, kvm
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] use bitmap ops on a bitmap instead of bit ops
2009-03-02 7:12 ` Sheng Yang
@ 2009-03-03 1:28 ` Sheng Yang
0 siblings, 0 replies; 5+ messages in thread
From: Sheng Yang @ 2009-03-03 1:28 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Gleb Natapov, avi, kvm
On Monday 02 March 2009 15:12:00 Sheng Yang wrote:
> 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>
Just a reminder that I have merge this patch into the patch title"[PATCH] KVM:
Merge kvm_ioapic_get_delivery_bitmask into kvm_get_intr_delivery_bitmask" with
Gleb's agreement.
Thanks. :)
--
regards
Yang, Sheng
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-03-03 1:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-26 13:03 [PATCH] use bitmap ops on a bitmap instead of bit ops Gleb Natapov
2009-02-27 3:19 ` Sheng Yang
2009-02-27 13:26 ` Gleb Natapov
2009-03-02 7:12 ` Sheng Yang
2009-03-03 1:28 ` Sheng Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox