| Message ID | 20240902100355.3032079-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: Cleanup following bitops improvements | expand |
On 02/09/2024 12:03, Andrew Cooper wrote: > > > There are no bits set in lr_mask beyond nr_lrs, so when substituting > bitmap_for_each() for for_each_set_bit(), we don't need to worry about the > upper bound. > > However, the type of lr_mask does matter, so switch it to be uint64_t * and > move unsigned long * override until the find_next_zero_bit() call. > > Move lr_val into a narrower scope and drop used_lr as it's declared by > for_each_set_bit() itself. > > Drop the nr_lrs variable and use gic_get_nr_lrs() in the one location its now > used. It hides a triple pointer defererence, and while it may not be needed s/defererence/dereference > in the PRISTINE case, it certainly doesn't need to be live across the rest of > the function. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
On 03/09/2024 9:46 am, Michal Orzel wrote: > > On 02/09/2024 12:03, Andrew Cooper wrote: >> >> There are no bits set in lr_mask beyond nr_lrs, so when substituting >> bitmap_for_each() for for_each_set_bit(), we don't need to worry about the >> upper bound. >> >> However, the type of lr_mask does matter, so switch it to be uint64_t * and >> move unsigned long * override until the find_next_zero_bit() call. >> >> Move lr_val into a narrower scope and drop used_lr as it's declared by >> for_each_set_bit() itself. >> >> Drop the nr_lrs variable and use gic_get_nr_lrs() in the one location its now >> used. It hides a triple pointer defererence, and while it may not be needed > s/defererence/dereference Fixed. > >> in the PRISTINE case, it certainly doesn't need to be live across the rest of >> the function. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Michal Orzel <michal.orzel@amd.com> Thanks. ~Andrew
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c index 3f14aab2efc7..ea48c5375a91 100644 --- a/xen/arch/arm/gic-vgic.c +++ b/xen/arch/arm/gic-vgic.c @@ -102,25 +102,23 @@ static unsigned int gic_find_unused_lr(struct vcpu *v, struct pending_irq *p, unsigned int lr) { - unsigned int nr_lrs = gic_get_nr_lrs(); - unsigned long *lr_mask = (unsigned long *) &this_cpu(lr_mask); - struct gic_lr lr_val; + uint64_t *lr_mask = &this_cpu(lr_mask); ASSERT(spin_is_locked(&v->arch.vgic.lock)); if ( unlikely(test_bit(GIC_IRQ_GUEST_PRISTINE_LPI, &p->status)) ) { - unsigned int used_lr; - - bitmap_for_each ( used_lr, lr_mask, nr_lrs ) + for_each_set_bit ( used_lr, *lr_mask ) { + struct gic_lr lr_val; + gic_hw_ops->read_lr(used_lr, &lr_val); if ( lr_val.virq == p->irq ) return used_lr; } } - lr = find_next_zero_bit(lr_mask, nr_lrs, lr); + lr = find_next_zero_bit((unsigned long *)lr_mask, gic_get_nr_lrs(), lr); return lr; }
There are no bits set in lr_mask beyond nr_lrs, so when substituting bitmap_for_each() for for_each_set_bit(), we don't need to worry about the upper bound. However, the type of lr_mask does matter, so switch it to be uint64_t * and move unsigned long * override until the find_next_zero_bit() call. Move lr_val into a narrower scope and drop used_lr as it's declared by for_each_set_bit() itself. Drop the nr_lrs variable and use gic_get_nr_lrs() in the one location its now used. It hides a triple pointer defererence, and while it may not be needed in the PRISTINE case, it certainly doesn't need to be live across the rest of the function. No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> CC: Bertrand Marquis <bertrand.marquis@arm.com> CC: Michal Orzel <michal.orzel@amd.com> ARM64: $ ../scripts/bloat-o-meter xen-syms-arm64-{before,after} add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-28 (-28) Function old new delta gic_find_unused_lr.constprop 228 200 -28 inc -2 find_next_bit() ARM32: $ ../scripts/bloat-o-meter xen-syms-arm32-{before,after} add/remove: 0/0 grow/shrink: 1/0 up/down: 48/0 (48) Function old new delta gic_find_unused_lr 260 308 +48 because uint64_t, but -2 _find_{first,next}_bit_le() --- xen/arch/arm/gic-vgic.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)