[PATCH] KVM: arm64: vgic: declear local variable 'flags' before for loop starts

[PATCH] KVM: arm64: vgic: declear local variable 'flags' before for loop starts

[Austin Kim]

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.

Signed-off-by: Austin Kim <austin.kim@lge.com>
---
 arch/arm64/kvm/vgic/vgic-mmio.c | 2 +-
 arch/arm64/kvm/vgic/vgic-v4.c   | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

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);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: vgic: declear local variable 'flags' before for loop starts

[Marc Zyngier]

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.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] KVM: arm64: vgic: declear local variable 'flags' before for loop starts

[Austin Kim]

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel