| Message ID | 20210612110014.GA1211@raspberrypi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: vgic: declear local variable 'flags' before for loop starts | expand |
On Sat, 12 Jun 2021 12:00:14 +0100, Austin Kim <austindh.kim@gmail.com> wrote: > > From: Austin Kim <austin.kim@lge.com> > > Normally local variable 'flags' is defined out of for loop, > when 'flags' is used as the second parameter in a call to > spinlock_irq[save/restore] function. > > So it had better declear local variable 'flags' ahead of for loop. Why better? Reducing the scope of a variable is in general good practice. Do you see any material advantage in moving this variable out of the loop? Does the compiler generate better code? Thanks, M.
2021년 6월 12일 (토) 오후 8:10, Marc Zyngier <maz@kernel.org>님이 작성: > > On Sat, 12 Jun 2021 12:00:14 +0100, > Austin Kim <austindh.kim@gmail.com> wrote: > > > > From: Austin Kim <austin.kim@lge.com> > > > > Normally local variable 'flags' is defined out of for loop, > > when 'flags' is used as the second parameter in a call to > > spinlock_irq[save/restore] function. > > > > So it had better declear local variable 'flags' ahead of for loop. > > Why better? Reducing the scope of a variable is in general good > practice. Do you see any material advantage in moving this variable > out of the loop? Does the compiler generate better code? First all of, thanks for feedback. I checked how the compiler generate assembly code(before/after) using objdump utility. And then found out that compiler generates the same assembly code. <compiler version: gcc-linaro-7.5.0-2019.12-x86_64_aarch64> ffff80001005f5c8 <vgic_mmio_read_pending>: ffff80001005f5c8: d503233f paciasp ... ffff80001005f63c: 97ffe9af bl ffff800010059cf8 <vgic_get_irq> ffff80001005f640: aa0003f3 mov x19, x0 ffff80001005f644: 943886c3 bl ffff800010e81150 <_raw_spin_lock_irqsave> Let me keep in mind how compiler generate assembly code with new patch, which leads to material advantage. BR, Austin Kim > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c index 48c6067fc5ec..ae32428d9f87 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio.c +++ b/arch/arm64/kvm/vgic/vgic-mmio.c @@ -232,11 +232,11 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu, u32 intid = VGIC_ADDR_TO_INTID(addr, 1); u32 value = 0; int i; + unsigned long flags; /* Loop over all IRQs affected by this read */ for (i = 0; i < len * 8; i++) { struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); - unsigned long flags; bool val; raw_spin_lock_irqsave(&irq->irq_lock, flags); diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c index c1845d8f5f7e..a0c743f83bbe 100644 --- a/arch/arm64/kvm/vgic/vgic-v4.c +++ b/arch/arm64/kvm/vgic/vgic-v4.c @@ -116,7 +116,7 @@ static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu) { struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe; int i; - + unsigned long flags; /* * With GICv4.1, every virtual SGI can be directly injected. So * let's pretend that they are HW interrupts, tied to a host @@ -125,7 +125,6 @@ static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu) for (i = 0; i < VGIC_NR_SGIS; i++) { struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, i); struct irq_desc *desc; - unsigned long flags; int ret; raw_spin_lock_irqsave(&irq->irq_lock, flags); @@ -158,11 +157,11 @@ static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu) static void vgic_v4_disable_vsgis(struct kvm_vcpu *vcpu) { int i; + unsigned long flags; for (i = 0; i < VGIC_NR_SGIS; i++) { struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, i); struct irq_desc *desc; - unsigned long flags; int ret; raw_spin_lock_irqsave(&irq->irq_lock, flags);