diff mbox

[V2,1/4] x86/kvm: Resolve some missing-initializers warnings

Message ID 20140730211845.127128.56203.stgit@mdrustad-wks.jf.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rustad, Mark D July 30, 2014, 9:18 p.m. UTC
Resolve some missing-initializers warnings that appear in W=2
builds. They are resolved by adding the name as a parameter to
the macros and having the macro generate all four fields of the
structure.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

---
V2: Change macro to supply all four fields instead of using a
    designated initializer. Also fix up the array terminator.
---
 arch/x86/kvm/x86.c |   71 ++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 35 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paolo Bonzini July 31, 2014, 11:50 a.m. UTC | #1
Il 30/07/2014 23:18, Mark D Rustad ha scritto:
> Resolve some missing-initializers warnings that appear in W=2
> builds. They are resolved by adding the name as a parameter to
> the macros and having the macro generate all four fields of the
> structure.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> ---
> V2: Change macro to supply all four fields instead of using a
>     designated initializer. Also fix up the array terminator.
> ---
>  arch/x86/kvm/x86.c |   71 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ef432f891d30..623aea52ceba 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -82,8 +82,9 @@ u64 __read_mostly efer_reserved_bits = ~((u64)(EFER_SCE | EFER_LME | EFER_LMA));
>  static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE);
>  #endif
>  
> -#define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
> -#define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
> +#define VM_STAT(name, x) name, offsetof(struct kvm, stat.x), KVM_STAT_VM, NULL
> +#define VCPU_STAT(name, x) name, offsetof(struct kvm_vcpu, stat.x), \
> +			   KVM_STAT_VCPU, NULL
>  
>  static void update_cr8_intercept(struct kvm_vcpu *vcpu);
>  static void process_nmi(struct kvm_vcpu *vcpu);
> @@ -128,39 +129,39 @@ static struct kvm_shared_msrs_global __read_mostly shared_msrs_global;
>  static struct kvm_shared_msrs __percpu *shared_msrs;
>  
>  struct kvm_stats_debugfs_item debugfs_entries[] = {
> -	{ "pf_fixed", VCPU_STAT(pf_fixed) },
> -	{ "pf_guest", VCPU_STAT(pf_guest) },
> -	{ "tlb_flush", VCPU_STAT(tlb_flush) },
> -	{ "invlpg", VCPU_STAT(invlpg) },
> -	{ "exits", VCPU_STAT(exits) },
> -	{ "io_exits", VCPU_STAT(io_exits) },
> -	{ "mmio_exits", VCPU_STAT(mmio_exits) },
> -	{ "signal_exits", VCPU_STAT(signal_exits) },
> -	{ "irq_window", VCPU_STAT(irq_window_exits) },
> -	{ "nmi_window", VCPU_STAT(nmi_window_exits) },
> -	{ "halt_exits", VCPU_STAT(halt_exits) },
> -	{ "halt_wakeup", VCPU_STAT(halt_wakeup) },
> -	{ "hypercalls", VCPU_STAT(hypercalls) },
> -	{ "request_irq", VCPU_STAT(request_irq_exits) },
> -	{ "irq_exits", VCPU_STAT(irq_exits) },
> -	{ "host_state_reload", VCPU_STAT(host_state_reload) },
> -	{ "efer_reload", VCPU_STAT(efer_reload) },
> -	{ "fpu_reload", VCPU_STAT(fpu_reload) },
> -	{ "insn_emulation", VCPU_STAT(insn_emulation) },
> -	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
> -	{ "irq_injections", VCPU_STAT(irq_injections) },
> -	{ "nmi_injections", VCPU_STAT(nmi_injections) },
> -	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
> -	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
> -	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },
> -	{ "mmu_pde_zapped", VM_STAT(mmu_pde_zapped) },
> -	{ "mmu_flooded", VM_STAT(mmu_flooded) },
> -	{ "mmu_recycled", VM_STAT(mmu_recycled) },
> -	{ "mmu_cache_miss", VM_STAT(mmu_cache_miss) },
> -	{ "mmu_unsync", VM_STAT(mmu_unsync) },
> -	{ "remote_tlb_flush", VM_STAT(remote_tlb_flush) },
> -	{ "largepages", VM_STAT(lpages) },
> -	{ NULL }
> +	{ VCPU_STAT("pf_fixed", pf_fixed) },
> +	{ VCPU_STAT("pf_guest", pf_guest) },
> +	{ VCPU_STAT("tlb_flush", tlb_flush) },
> +	{ VCPU_STAT("invlpg", invlpg) },
> +	{ VCPU_STAT("exits", exits) },
> +	{ VCPU_STAT("io_exits", io_exits) },
> +	{ VCPU_STAT("mmio_exits", mmio_exits) },
> +	{ VCPU_STAT("signal_exits", signal_exits) },
> +	{ VCPU_STAT("irq_window", irq_window_exits) },
> +	{ VCPU_STAT("nmi_window", nmi_window_exits) },
> +	{ VCPU_STAT("halt_exits", halt_exits) },
> +	{ VCPU_STAT("halt_wakeup", halt_wakeup) },
> +	{ VCPU_STAT("hypercalls", hypercalls) },
> +	{ VCPU_STAT("request_irq", request_irq_exits) },
> +	{ VCPU_STAT("irq_exits", irq_exits) },
> +	{ VCPU_STAT("host_state_reload", host_state_reload) },
> +	{ VCPU_STAT("efer_reload", efer_reload) },
> +	{ VCPU_STAT("fpu_reload", fpu_reload) },
> +	{ VCPU_STAT("insn_emulation", insn_emulation) },
> +	{ VCPU_STAT("insn_emulation_fail", insn_emulation_fail) },
> +	{ VCPU_STAT("irq_injections", irq_injections) },
> +	{ VCPU_STAT("nmi_injections", nmi_injections) },
> +	{ VM_STAT("mmu_shadow_zapped", mmu_shadow_zapped) },
> +	{ VM_STAT("mmu_pte_write", mmu_pte_write) },
> +	{ VM_STAT("mmu_pte_updated", mmu_pte_updated) },
> +	{ VM_STAT("mmu_pde_zapped", mmu_pde_zapped) },
> +	{ VM_STAT("mmu_flooded", mmu_flooded) },
> +	{ VM_STAT("mmu_recycled", mmu_recycled) },
> +	{ VM_STAT("mmu_cache_miss", mmu_cache_miss) },
> +	{ VM_STAT("mmu_unsync", mmu_unsync) },
> +	{ VM_STAT("remote_tlb_flush", remote_tlb_flush) },
> +	{ VM_STAT("largepages", lpages) },

Please move the braces inside the macro as well.

> +	{ NULL, 0, 0, NULL }

This is ugly.  Do you really mind having one residual warning? :)

Paolo

>  };
>  
>  u64 __read_mostly host_xcr0;
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rustad, Mark D July 31, 2014, 4:35 p.m. UTC | #2
On Jul 31, 2014, at 4:50 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 30/07/2014 23:18, Mark D Rustad ha scritto:
>> Resolve some missing-initializers warnings that appear in W=2
>> builds. They are resolved by adding the name as a parameter to
>> the macros and having the macro generate all four fields of the
>> structure.
>> 
>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> 
>> ---
>> V2: Change macro to supply all four fields instead of using a
>>    designated initializer. Also fix up the array terminator.
>> ---
>> arch/x86/kvm/x86.c |   71 ++++++++++++++++++++++++++--------------------------
>> 1 file changed, 36 insertions(+), 35 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index ef432f891d30..623aea52ceba 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -82,8 +82,9 @@ u64 __read_mostly efer_reserved_bits = ~((u64)(EFER_SCE | EFER_LME | EFER_LMA));
>> static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE);
>> #endif
>> 
>> -#define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
>> -#define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
>> +#define VM_STAT(name, x) name, offsetof(struct kvm, stat.x), KVM_STAT_VM, NULL
>> +#define VCPU_STAT(name, x) name, offsetof(struct kvm_vcpu, stat.x), \
>> +			   KVM_STAT_VCPU, NULL
>> 
>> static void update_cr8_intercept(struct kvm_vcpu *vcpu);
>> static void process_nmi(struct kvm_vcpu *vcpu);
>> @@ -128,39 +129,39 @@ static struct kvm_shared_msrs_global __read_mostly shared_msrs_global;
>> static struct kvm_shared_msrs __percpu *shared_msrs;
>> 
>> struct kvm_stats_debugfs_item debugfs_entries[] = {
>> -	{ "pf_fixed", VCPU_STAT(pf_fixed) },
>> -	{ "pf_guest", VCPU_STAT(pf_guest) },
>> -	{ "tlb_flush", VCPU_STAT(tlb_flush) },
>> -	{ "invlpg", VCPU_STAT(invlpg) },
>> -	{ "exits", VCPU_STAT(exits) },
>> -	{ "io_exits", VCPU_STAT(io_exits) },
>> -	{ "mmio_exits", VCPU_STAT(mmio_exits) },
>> -	{ "signal_exits", VCPU_STAT(signal_exits) },
>> -	{ "irq_window", VCPU_STAT(irq_window_exits) },
>> -	{ "nmi_window", VCPU_STAT(nmi_window_exits) },
>> -	{ "halt_exits", VCPU_STAT(halt_exits) },
>> -	{ "halt_wakeup", VCPU_STAT(halt_wakeup) },
>> -	{ "hypercalls", VCPU_STAT(hypercalls) },
>> -	{ "request_irq", VCPU_STAT(request_irq_exits) },
>> -	{ "irq_exits", VCPU_STAT(irq_exits) },
>> -	{ "host_state_reload", VCPU_STAT(host_state_reload) },
>> -	{ "efer_reload", VCPU_STAT(efer_reload) },
>> -	{ "fpu_reload", VCPU_STAT(fpu_reload) },
>> -	{ "insn_emulation", VCPU_STAT(insn_emulation) },
>> -	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
>> -	{ "irq_injections", VCPU_STAT(irq_injections) },
>> -	{ "nmi_injections", VCPU_STAT(nmi_injections) },
>> -	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
>> -	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
>> -	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },
>> -	{ "mmu_pde_zapped", VM_STAT(mmu_pde_zapped) },
>> -	{ "mmu_flooded", VM_STAT(mmu_flooded) },
>> -	{ "mmu_recycled", VM_STAT(mmu_recycled) },
>> -	{ "mmu_cache_miss", VM_STAT(mmu_cache_miss) },
>> -	{ "mmu_unsync", VM_STAT(mmu_unsync) },
>> -	{ "remote_tlb_flush", VM_STAT(remote_tlb_flush) },
>> -	{ "largepages", VM_STAT(lpages) },
>> -	{ NULL }
>> +	{ VCPU_STAT("pf_fixed", pf_fixed) },
>> +	{ VCPU_STAT("pf_guest", pf_guest) },
>> +	{ VCPU_STAT("tlb_flush", tlb_flush) },
>> +	{ VCPU_STAT("invlpg", invlpg) },
>> +	{ VCPU_STAT("exits", exits) },
>> +	{ VCPU_STAT("io_exits", io_exits) },
>> +	{ VCPU_STAT("mmio_exits", mmio_exits) },
>> +	{ VCPU_STAT("signal_exits", signal_exits) },
>> +	{ VCPU_STAT("irq_window", irq_window_exits) },
>> +	{ VCPU_STAT("nmi_window", nmi_window_exits) },
>> +	{ VCPU_STAT("halt_exits", halt_exits) },
>> +	{ VCPU_STAT("halt_wakeup", halt_wakeup) },
>> +	{ VCPU_STAT("hypercalls", hypercalls) },
>> +	{ VCPU_STAT("request_irq", request_irq_exits) },
>> +	{ VCPU_STAT("irq_exits", irq_exits) },
>> +	{ VCPU_STAT("host_state_reload", host_state_reload) },
>> +	{ VCPU_STAT("efer_reload", efer_reload) },
>> +	{ VCPU_STAT("fpu_reload", fpu_reload) },
>> +	{ VCPU_STAT("insn_emulation", insn_emulation) },
>> +	{ VCPU_STAT("insn_emulation_fail", insn_emulation_fail) },
>> +	{ VCPU_STAT("irq_injections", irq_injections) },
>> +	{ VCPU_STAT("nmi_injections", nmi_injections) },
>> +	{ VM_STAT("mmu_shadow_zapped", mmu_shadow_zapped) },
>> +	{ VM_STAT("mmu_pte_write", mmu_pte_write) },
>> +	{ VM_STAT("mmu_pte_updated", mmu_pte_updated) },
>> +	{ VM_STAT("mmu_pde_zapped", mmu_pde_zapped) },
>> +	{ VM_STAT("mmu_flooded", mmu_flooded) },
>> +	{ VM_STAT("mmu_recycled", mmu_recycled) },
>> +	{ VM_STAT("mmu_cache_miss", mmu_cache_miss) },
>> +	{ VM_STAT("mmu_unsync", mmu_unsync) },
>> +	{ VM_STAT("remote_tlb_flush", remote_tlb_flush) },
>> +	{ VM_STAT("largepages", lpages) },
> 
> Please move the braces inside the macro as well.

I should have thought of that. I will do that in a new version. That would be much better.

>> +	{ NULL, 0, 0, NULL }
> 
> This is ugly.  Do you really mind having one residual warning? :)

I agree it is ugly. .name = NULL would be enough to silence it. Would that be better? At the moment I am thinking of this as a test case for the other 1000 { } and {0} initializers in the kernel that are throwing warnings. I know we both agree that the compiler really shouldn't be warning on them, but they currently make a lot noise.

How would you feel about a macro called something like ZERO_ENTRY defined something like:

#define ZERO_ENTRY DIAG_PUSH DIAG_IGNORE(missing-field-initializers) { } DIAG_POP

Where the DIAG_ macros pretty much do what you think. I have another patch series that Jeff hasn't gotten to yet that defines such macros. Of course they get put to good use.

At this point, I'll put the terminator back the way it was, but I would still like your opinion on the macro approach to addressing all of these terminators.
Paolo Bonzini July 31, 2014, 4:50 p.m. UTC | #3
Il 31/07/2014 18:35, Rustad, Mark D ha scritto:
> I agree it is ugly. .name = NULL would be enough to silence it. Would
> that be better? At the moment I am thinking of this as a test case
> for the other 1000 { } and {0} initializers in the kernel that are
> throwing warnings. I know we both agree that the compiler really
> shouldn't be warning on them, but they currently make a lot noise.
> 
> How would you feel about a macro called something like ZERO_ENTRY
> defined something like:
> 
> #define ZERO_ENTRY DIAG_PUSH DIAG_IGNORE(missing-field-initializers)
> { } DIAG_POP
> 
> Where the DIAG_ macros pretty much do what you think. I have another
> patch series that Jeff hasn't gotten to yet that defines such macros.
> Of course they get put to good use.
> 
> At this point, I'll put the terminator back the way it was, but I
> would still like your opinion on the macro approach to addressing all
> of these terminators.

If you get such a macro in include/linux, I will of course accept its usage.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ef432f891d30..623aea52ceba 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -82,8 +82,9 @@  u64 __read_mostly efer_reserved_bits = ~((u64)(EFER_SCE | EFER_LME | EFER_LMA));
 static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE);
 #endif
 
-#define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
-#define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
+#define VM_STAT(name, x) name, offsetof(struct kvm, stat.x), KVM_STAT_VM, NULL
+#define VCPU_STAT(name, x) name, offsetof(struct kvm_vcpu, stat.x), \
+			   KVM_STAT_VCPU, NULL
 
 static void update_cr8_intercept(struct kvm_vcpu *vcpu);
 static void process_nmi(struct kvm_vcpu *vcpu);
@@ -128,39 +129,39 @@  static struct kvm_shared_msrs_global __read_mostly shared_msrs_global;
 static struct kvm_shared_msrs __percpu *shared_msrs;
 
 struct kvm_stats_debugfs_item debugfs_entries[] = {
-	{ "pf_fixed", VCPU_STAT(pf_fixed) },
-	{ "pf_guest", VCPU_STAT(pf_guest) },
-	{ "tlb_flush", VCPU_STAT(tlb_flush) },
-	{ "invlpg", VCPU_STAT(invlpg) },
-	{ "exits", VCPU_STAT(exits) },
-	{ "io_exits", VCPU_STAT(io_exits) },
-	{ "mmio_exits", VCPU_STAT(mmio_exits) },
-	{ "signal_exits", VCPU_STAT(signal_exits) },
-	{ "irq_window", VCPU_STAT(irq_window_exits) },
-	{ "nmi_window", VCPU_STAT(nmi_window_exits) },
-	{ "halt_exits", VCPU_STAT(halt_exits) },
-	{ "halt_wakeup", VCPU_STAT(halt_wakeup) },
-	{ "hypercalls", VCPU_STAT(hypercalls) },
-	{ "request_irq", VCPU_STAT(request_irq_exits) },
-	{ "irq_exits", VCPU_STAT(irq_exits) },
-	{ "host_state_reload", VCPU_STAT(host_state_reload) },
-	{ "efer_reload", VCPU_STAT(efer_reload) },
-	{ "fpu_reload", VCPU_STAT(fpu_reload) },
-	{ "insn_emulation", VCPU_STAT(insn_emulation) },
-	{ "insn_emulation_fail", VCPU_STAT(insn_emulation_fail) },
-	{ "irq_injections", VCPU_STAT(irq_injections) },
-	{ "nmi_injections", VCPU_STAT(nmi_injections) },
-	{ "mmu_shadow_zapped", VM_STAT(mmu_shadow_zapped) },
-	{ "mmu_pte_write", VM_STAT(mmu_pte_write) },
-	{ "mmu_pte_updated", VM_STAT(mmu_pte_updated) },
-	{ "mmu_pde_zapped", VM_STAT(mmu_pde_zapped) },
-	{ "mmu_flooded", VM_STAT(mmu_flooded) },
-	{ "mmu_recycled", VM_STAT(mmu_recycled) },
-	{ "mmu_cache_miss", VM_STAT(mmu_cache_miss) },
-	{ "mmu_unsync", VM_STAT(mmu_unsync) },
-	{ "remote_tlb_flush", VM_STAT(remote_tlb_flush) },
-	{ "largepages", VM_STAT(lpages) },
-	{ NULL }
+	{ VCPU_STAT("pf_fixed", pf_fixed) },
+	{ VCPU_STAT("pf_guest", pf_guest) },
+	{ VCPU_STAT("tlb_flush", tlb_flush) },
+	{ VCPU_STAT("invlpg", invlpg) },
+	{ VCPU_STAT("exits", exits) },
+	{ VCPU_STAT("io_exits", io_exits) },
+	{ VCPU_STAT("mmio_exits", mmio_exits) },
+	{ VCPU_STAT("signal_exits", signal_exits) },
+	{ VCPU_STAT("irq_window", irq_window_exits) },
+	{ VCPU_STAT("nmi_window", nmi_window_exits) },
+	{ VCPU_STAT("halt_exits", halt_exits) },
+	{ VCPU_STAT("halt_wakeup", halt_wakeup) },
+	{ VCPU_STAT("hypercalls", hypercalls) },
+	{ VCPU_STAT("request_irq", request_irq_exits) },
+	{ VCPU_STAT("irq_exits", irq_exits) },
+	{ VCPU_STAT("host_state_reload", host_state_reload) },
+	{ VCPU_STAT("efer_reload", efer_reload) },
+	{ VCPU_STAT("fpu_reload", fpu_reload) },
+	{ VCPU_STAT("insn_emulation", insn_emulation) },
+	{ VCPU_STAT("insn_emulation_fail", insn_emulation_fail) },
+	{ VCPU_STAT("irq_injections", irq_injections) },
+	{ VCPU_STAT("nmi_injections", nmi_injections) },
+	{ VM_STAT("mmu_shadow_zapped", mmu_shadow_zapped) },
+	{ VM_STAT("mmu_pte_write", mmu_pte_write) },
+	{ VM_STAT("mmu_pte_updated", mmu_pte_updated) },
+	{ VM_STAT("mmu_pde_zapped", mmu_pde_zapped) },
+	{ VM_STAT("mmu_flooded", mmu_flooded) },
+	{ VM_STAT("mmu_recycled", mmu_recycled) },
+	{ VM_STAT("mmu_cache_miss", mmu_cache_miss) },
+	{ VM_STAT("mmu_unsync", mmu_unsync) },
+	{ VM_STAT("remote_tlb_flush", remote_tlb_flush) },
+	{ VM_STAT("largepages", lpages) },
+	{ NULL, 0, 0, NULL }
 };
 
 u64 __read_mostly host_xcr0;