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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Jul 31, 2014, at 4:50 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> 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@xxxxxxxxx>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@xxxxxxxxx>
>> 
>> ---
>> 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.

-- 
Mark Rustad, Networking Division, Intel Corporation

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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux