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

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

[Jeff Kirsher]

From: Mark Rustad <mark.d.rustad@intel.com>

Resolve some missing-initializers warnings that appear in W=2
builds. They are resolved by using a designated initialization,
which is enough to resolve the warning.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 arch/x86/kvm/x86.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ef432f8..6d0f47208 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -82,8 +82,8 @@ 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(x) .offset = offsetof(struct kvm, stat.x), KVM_STAT_VM
+#define VCPU_STAT(x) .offset = offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
 
 static void update_cr8_intercept(struct kvm_vcpu *vcpu);
 static void process_nmi(struct kvm_vcpu *vcpu);
-- 
1.9.3

[PATCH 2/4] kvm: Resolve missing-field-initializers warnings

[Jeff Kirsher]

From: Mark Rustad <mark.d.rustad@intel.com>

Resolve missing-field-initializers warnings seen in W=2 kernel
builds by having macros generate more elaborated initializers.
That is enough to silence the warnings.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 virt/kvm/irq_comm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index ced4a54..a228ee8 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -323,13 +323,13 @@ out:
 
 #define IOAPIC_ROUTING_ENTRY(irq) \
 	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
-	  .u.irqchip.irqchip = KVM_IRQCHIP_IOAPIC, .u.irqchip.pin = (irq) }
+	  .u.irqchip = { .irqchip = KVM_IRQCHIP_IOAPIC, .pin = (irq) } }
 #define ROUTING_ENTRY1(irq) IOAPIC_ROUTING_ENTRY(irq)
 
 #ifdef CONFIG_X86
 #  define PIC_ROUTING_ENTRY(irq) \
 	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
-	  .u.irqchip.irqchip = SELECT_PIC(irq), .u.irqchip.pin = (irq) % 8 }
+	  .u.irqchip = { .irqchip = SELECT_PIC(irq), .pin = (irq) % 8 } }
 #  define ROUTING_ENTRY2(irq) \
 	IOAPIC_ROUTING_ENTRY(irq), PIC_ROUTING_ENTRY(irq)
 #else
-- 
1.9.3

[PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Jeff Kirsher]

From: Mark Rustad <mark.d.rustad@intel.com>

Resolve shadow warnings that appear in W=2 builds. In this case,
a macro declared an inner local variable with the same name as an
outer one. This can be a serious hazard in the event that the outer
variable is ever passed as a parameter, as the inner variable will
be referenced instead of what was intended. This macro doesn't have
any parameters - at this time - but prepend an _ to all of the
macro-declared variables as is the custom, to resolve the warnings
and eliminate any future hazard.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 arch/x86/kvm/mmutrace.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ff..2d8d00c 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,26 +22,26 @@
 	__entry->unsync = sp->unsync;
 
 #define KVM_MMU_PAGE_PRINTK() ({				        \
-	const char *ret = p->buffer + p->len;				\
-	static const char *access_str[] = {			        \
+	const char *_ret = p->buffer + p->len;				\
+	static const char *_access_str[] = {			        \
 		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
 	};							        \
-	union kvm_mmu_page_role role;				        \
+	union kvm_mmu_page_role _role;				        \
 								        \
-	role.word = __entry->role;					\
+	_role.word = __entry->role;					\
 									\
 	trace_seq_printf(p, "sp gen %lx gfn %llx %u%s q%u%s %s%s"	\
 			 " %snxe root %u %s%c",	__entry->mmu_valid_gen,	\
-			 __entry->gfn, role.level,			\
-			 role.cr4_pae ? " pae" : "",			\
-			 role.quadrant,					\
-			 role.direct ? " direct" : "",			\
-			 access_str[role.access],			\
-			 role.invalid ? " invalid" : "",		\
-			 role.nxe ? "" : "!",				\
+			 __entry->gfn, _role.level,			\
+			 _role.cr4_pae ? " pae" : "",			\
+			 _role.quadrant,				\
+			 _role.direct ? " direct" : "",			\
+			 _access_str[_role.access],			\
+			 _role.invalid ? " invalid" : "",		\
+			 _role.nxe ? "" : "!",				\
 			 __entry->root_count,				\
 			 __entry->unsync ? "unsync" : "sync", 0);	\
-	ret;								\
+	_ret;								\
 		})
 
 #define kvm_mmu_trace_pferr_flags       \
-- 
1.9.3

[PATCH 4/4] x86/kvm: Resolve shadow warning from min macro

[Jeff Kirsher]

From: Mark Rustad <mark.d.rustad@intel.com>

Resolve a shadow warning generated in W=2 builds by the nested
use of the min macro by instead using the min3 macro for the
minimum of 3 values.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 arch/x86/kvm/emulate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e4e833d..2d6c97a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1315,8 +1315,7 @@ static int pio_in_emulated(struct x86_emulate_ctxt *ctxt,
 		in_page = (ctxt->eflags & EFLG_DF) ?
 			offset_in_page(reg_read(ctxt, VCPU_REGS_RDI)) :
 			PAGE_SIZE - offset_in_page(reg_read(ctxt, VCPU_REGS_RDI));
-		n = min(min(in_page, (unsigned int)sizeof(rc->data)) / size,
-			count);
+		n = min3(in_page, (unsigned int)sizeof(rc->data) / size, count);
 		if (n == 0)
 			n = 1;
 		rc->pos = rc->end = 0;
-- 
1.9.3

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

[Paolo Bonzini]

Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
> From: Mark Rustad <mark.d.rustad@intel.com>
> 
> Resolve some missing-initializers warnings that appear in W=2
> builds. They are resolved by using a designated initialization,
> which is enough to resolve the warning.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  arch/x86/kvm/x86.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ef432f8..6d0f47208 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -82,8 +82,8 @@ 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(x) .offset = offsetof(struct kvm, stat.x), KVM_STAT_VM
> +#define VCPU_STAT(x) .offset = offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
>  
>  static void update_cr8_intercept(struct kvm_vcpu *vcpu);
>  static void process_nmi(struct kvm_vcpu *vcpu);
> 

This is really ugly.  Either add the missing initializer (it's just a
NULL) or, better, move the name into VM_STAT/VCPU_STAT and add the
missing initializer to VM_STAT/VCPU_STAT.

Paolo

Re: [PATCH 2/4] kvm: Resolve missing-field-initializers warnings

[Paolo Bonzini]

Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
> From: Mark Rustad <mark.d.rustad@intel.com>
> 
> Resolve missing-field-initializers warnings seen in W=2 kernel
> builds by having macros generate more elaborated initializers.
> That is enough to silence the warnings.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  virt/kvm/irq_comm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index ced4a54..a228ee8 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -323,13 +323,13 @@ out:
>  
>  #define IOAPIC_ROUTING_ENTRY(irq) \
>  	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
> -	  .u.irqchip.irqchip = KVM_IRQCHIP_IOAPIC, .u.irqchip.pin = (irq) }
> +	  .u.irqchip = { .irqchip = KVM_IRQCHIP_IOAPIC, .pin = (irq) } }
>  #define ROUTING_ENTRY1(irq) IOAPIC_ROUTING_ENTRY(irq)
>  
>  #ifdef CONFIG_X86
>  #  define PIC_ROUTING_ENTRY(irq) \
>  	{ .gsi = irq, .type = KVM_IRQ_ROUTING_IRQCHIP,	\
> -	  .u.irqchip.irqchip = SELECT_PIC(irq), .u.irqchip.pin = (irq) % 8 }
> +	  .u.irqchip = { .irqchip = SELECT_PIC(irq), .pin = (irq) % 8 } }
>  #  define ROUTING_ENTRY2(irq) \
>  	IOAPIC_ROUTING_ENTRY(irq), PIC_ROUTING_ENTRY(irq)
>  #else
> 

Applied.

Paolo

Re: [PATCH 4/4] x86/kvm: Resolve shadow warning from min macro

[Paolo Bonzini]

Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
> From: Mark Rustad <mark.d.rustad@intel.com>
> 
> Resolve a shadow warning generated in W=2 builds by the nested
> use of the min macro by instead using the min3 macro for the
> minimum of 3 values.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  arch/x86/kvm/emulate.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index e4e833d..2d6c97a 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1315,8 +1315,7 @@ static int pio_in_emulated(struct x86_emulate_ctxt *ctxt,
>  		in_page = (ctxt->eflags & EFLG_DF) ?
>  			offset_in_page(reg_read(ctxt, VCPU_REGS_RDI)) :
>  			PAGE_SIZE - offset_in_page(reg_read(ctxt, VCPU_REGS_RDI));
> -		n = min(min(in_page, (unsigned int)sizeof(rc->data)) / size,
> -			count);
> +		n = min3(in_page, (unsigned int)sizeof(rc->data) / size, count);
>  		if (n == 0)
>  			n = 1;
>  		rc->pos = rc->end = 0;
> 

Applied.

Paolo

Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Paolo Bonzini]

Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
> From: Mark Rustad <mark.d.rustad@intel.com>
> 
> Resolve shadow warnings that appear in W=2 builds. In this case,
> a macro declared an inner local variable with the same name as an
> outer one. This can be a serious hazard in the event that the outer
> variable is ever passed as a parameter, as the inner variable will
> be referenced instead of what was intended. This macro doesn't have
> any parameters - at this time - but prepend an _ to all of the
> macro-declared variables as is the custom, to resolve the warnings
> and eliminate any future hazard.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  arch/x86/kvm/mmutrace.h | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
> index 9d2e0ff..2d8d00c 100644
> --- a/arch/x86/kvm/mmutrace.h
> +++ b/arch/x86/kvm/mmutrace.h
> @@ -22,26 +22,26 @@
>  	__entry->unsync = sp->unsync;
>  
>  #define KVM_MMU_PAGE_PRINTK() ({				        \
> -	const char *ret = p->buffer + p->len;				\
> -	static const char *access_str[] = {			        \
> +	const char *_ret = p->buffer + p->len;				\
> +	static const char *_access_str[] = {			        \
>  		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
>  	};							        \
> -	union kvm_mmu_page_role role;				        \
> +	union kvm_mmu_page_role _role;				        \
>  								        \
> -	role.word = __entry->role;					\
> +	_role.word = __entry->role;					\
>  									\
>  	trace_seq_printf(p, "sp gen %lx gfn %llx %u%s q%u%s %s%s"	\
>  			 " %snxe root %u %s%c",	__entry->mmu_valid_gen,	\
> -			 __entry->gfn, role.level,			\
> -			 role.cr4_pae ? " pae" : "",			\
> -			 role.quadrant,					\
> -			 role.direct ? " direct" : "",			\
> -			 access_str[role.access],			\
> -			 role.invalid ? " invalid" : "",		\
> -			 role.nxe ? "" : "!",				\
> +			 __entry->gfn, _role.level,			\
> +			 _role.cr4_pae ? " pae" : "",			\
> +			 _role.quadrant,				\
> +			 _role.direct ? " direct" : "",			\
> +			 _access_str[_role.access],			\
> +			 _role.invalid ? " invalid" : "",		\
> +			 _role.nxe ? "" : "!",				\
>  			 __entry->root_count,				\
>  			 __entry->unsync ? "unsync" : "sync", 0);	\
> -	ret;								\
> +	_ret;								\
>  		})
>  
>  #define kvm_mmu_trace_pferr_flags       \
> 

I think this unnecessarily uglifies the code, so I am not applying it.
Gleb, what do you think?

Paolo

Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Rustad, Mark D]

On Jul 25, 2014, at 7:06 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
>> From: Mark Rustad <mark.d.rustad@intel.com>
>> 
>> Resolve shadow warnings that appear in W=2 builds. In this case,
>> a macro declared an inner local variable with the same name as an
>> outer one. This can be a serious hazard in the event that the outer
>> variable is ever passed as a parameter, as the inner variable will
>> be referenced instead of what was intended. This macro doesn't have
>> any parameters - at this time - but prepend an _ to all of the
>> macro-declared variables as is the custom, to resolve the warnings
>> and eliminate any future hazard.
>> 
>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> arch/x86/kvm/mmutrace.h | 24 ++++++++++++------------
>> 1 file changed, 12 insertions(+), 12 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
>> index 9d2e0ff..2d8d00c 100644
>> --- a/arch/x86/kvm/mmutrace.h
>> +++ b/arch/x86/kvm/mmutrace.h
>> @@ -22,26 +22,26 @@
>> 	__entry->unsync = sp->unsync;
>> 
>> #define KVM_MMU_PAGE_PRINTK() ({				        \
>> -	const char *ret = p->buffer + p->len;				\
>> -	static const char *access_str[] = {			        \
>> +	const char *_ret = p->buffer + p->len;				\
>> +	static const char *_access_str[] = {			        \
>> 		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
>> 	};							        \
>> -	union kvm_mmu_page_role role;				        \
>> +	union kvm_mmu_page_role _role;				        \
>> 								        \
>> -	role.word = __entry->role;					\
>> +	_role.word = __entry->role;					\
>> 									\
>> 	trace_seq_printf(p, "sp gen %lx gfn %llx %u%s q%u%s %s%s"	\
>> 			 " %snxe root %u %s%c",	__entry->mmu_valid_gen,	\
>> -			 __entry->gfn, role.level,			\
>> -			 role.cr4_pae ? " pae" : "",			\
>> -			 role.quadrant,					\
>> -			 role.direct ? " direct" : "",			\
>> -			 access_str[role.access],			\
>> -			 role.invalid ? " invalid" : "",		\
>> -			 role.nxe ? "" : "!",				\
>> +			 __entry->gfn, _role.level,			\
>> +			 _role.cr4_pae ? " pae" : "",			\
>> +			 _role.quadrant,				\
>> +			 _role.direct ? " direct" : "",			\
>> +			 _access_str[_role.access],			\
>> +			 _role.invalid ? " invalid" : "",		\
>> +			 _role.nxe ? "" : "!",				\
>> 			 __entry->root_count,				\
>> 			 __entry->unsync ? "unsync" : "sync", 0);	\
>> -	ret;								\
>> +	_ret;								\
>> 		})
>> 
>> #define kvm_mmu_trace_pferr_flags       \
>> 
> 
> I think this unnecessarily uglifies the code, so I am not applying it.
> Gleb, what do you think?

Would you consider a version that only changes ret to _ret? That would be enough to resolve the warning. I only did the other variables because it seemed to be a best practice for these inner block declarations.

Hmmm. It seems like p should really be a parameter to this macro.

-- 
Mark Rustad, Networking Division, Intel Corporation

Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Paolo Bonzini]

Il 25/07/2014 19:18, Rustad, Mark D ha scritto:
> On Jul 25, 2014, at 7:06 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
>>> From: Mark Rustad <mark.d.rustad@intel.com>
>>>
>>> Resolve shadow warnings that appear in W=2 builds. In this case,
>>> a macro declared an inner local variable with the same name as an
>>> outer one. This can be a serious hazard in the event that the outer
>>> variable is ever passed as a parameter, as the inner variable will
>>> be referenced instead of what was intended. This macro doesn't have
>>> any parameters - at this time - but prepend an _ to all of the
>>> macro-declared variables as is the custom, to resolve the warnings
>>> and eliminate any future hazard.
>>>
>>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>> arch/x86/kvm/mmutrace.h | 24 ++++++++++++------------
>>> 1 file changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
>>> index 9d2e0ff..2d8d00c 100644
>>> --- a/arch/x86/kvm/mmutrace.h
>>> +++ b/arch/x86/kvm/mmutrace.h
>>> @@ -22,26 +22,26 @@
>>> 	__entry->unsync = sp->unsync;
>>>
>>> #define KVM_MMU_PAGE_PRINTK() ({				        \
>>> -	const char *ret = p->buffer + p->len;				\
>>> -	static const char *access_str[] = {			        \
>>> +	const char *_ret = p->buffer + p->len;				\
>>> +	static const char *_access_str[] = {			        \
>>> 		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
>>> 	};							        \
>>> -	union kvm_mmu_page_role role;				        \
>>> +	union kvm_mmu_page_role _role;				        \
>>> 								        \
>>> -	role.word = __entry->role;					\
>>> +	_role.word = __entry->role;					\
>>> 									\
>>> 	trace_seq_printf(p, "sp gen %lx gfn %llx %u%s q%u%s %s%s"	\
>>> 			 " %snxe root %u %s%c",	__entry->mmu_valid_gen,	\
>>> -			 __entry->gfn, role.level,			\
>>> -			 role.cr4_pae ? " pae" : "",			\
>>> -			 role.quadrant,					\
>>> -			 role.direct ? " direct" : "",			\
>>> -			 access_str[role.access],			\
>>> -			 role.invalid ? " invalid" : "",		\
>>> -			 role.nxe ? "" : "!",				\
>>> +			 __entry->gfn, _role.level,			\
>>> +			 _role.cr4_pae ? " pae" : "",			\
>>> +			 _role.quadrant,				\
>>> +			 _role.direct ? " direct" : "",			\
>>> +			 _access_str[_role.access],			\
>>> +			 _role.invalid ? " invalid" : "",		\
>>> +			 _role.nxe ? "" : "!",				\
>>> 			 __entry->root_count,				\
>>> 			 __entry->unsync ? "unsync" : "sync", 0);	\
>>> -	ret;								\
>>> +	_ret;								\
>>> 		})
>>>
>>> #define kvm_mmu_trace_pferr_flags       \
>>>
>>
>> I think this unnecessarily uglifies the code, so I am not applying it.
>> Gleb, what do you think?
> 
> Would you consider a version that only changes ret to _ret?

That would be more acceptable, or even better I guess you could stash
away p->len and return p->buffer + saved_len.

The thing is that it makes no sense for the caller's "ret" to alias any
of the other local variables of the macro.  So the code gets uglier but
the only one to benefit is the compiler.

Paolo

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

[Mark D Rustad]

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) },
+	{ NULL, 0, 0, NULL }
 };
 
 u64 __read_mostly host_xcr0;

[PATCH V2 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Mark D Rustad]

Resolve shadow warnings that appear in W=2 builds. Instead of
using ret to hold the return pointer, save the length in a new
variable saved_len and compute the pointer on exit. This also
resolves a very technical error, in that ret was declared as
a const char *, when it really was a char * const, which
theoretically could have allowed the compiler to do something
wrong.

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

---
Changes in V2:
- Instead of renaming all inner variables, just delete the
  ret variable in favor of the new saved_len variable.
---
 arch/x86/kvm/mmutrace.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ffcb190..5aaf35641768 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,7 +22,7 @@
 	__entry->unsync = sp->unsync;
 
 #define KVM_MMU_PAGE_PRINTK() ({				        \
-	const char *ret = p->buffer + p->len;				\
+	const u32 saved_len = p->len;					\
 	static const char *access_str[] = {			        \
 		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
 	};							        \
@@ -41,7 +41,7 @@
 			 role.nxe ? "" : "!",				\
 			 __entry->root_count,				\
 			 __entry->unsync ? "unsync" : "sync", 0);	\
-	ret;								\
+	p->buffer + saved_len;						\
 		})
 
 #define kvm_mmu_trace_pferr_flags       \

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

[Paolo Bonzini]

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

Re: [PATCH V2 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Paolo Bonzini]

Il 30/07/2014 23:19, Mark D Rustad ha scritto:
> Resolve shadow warnings that appear in W=2 builds. Instead of
> using ret to hold the return pointer, save the length in a new
> variable saved_len and compute the pointer on exit. This also
> resolves a very technical error, in that ret was declared as
> a const char *, when it really was a char * const, which
> theoretically could have allowed the compiler to do something
> wrong.
> 
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> ---
> Changes in V2:
> - Instead of renaming all inner variables, just delete the
>   ret variable in favor of the new saved_len variable.
> ---
>  arch/x86/kvm/mmutrace.h |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
> index 9d2e0ffcb190..5aaf35641768 100644
> --- a/arch/x86/kvm/mmutrace.h
> +++ b/arch/x86/kvm/mmutrace.h
> @@ -22,7 +22,7 @@
>  	__entry->unsync = sp->unsync;
>  
>  #define KVM_MMU_PAGE_PRINTK() ({				        \
> -	const char *ret = p->buffer + p->len;				\
> +	const u32 saved_len = p->len;					\
>  	static const char *access_str[] = {			        \
>  		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
>  	};							        \
> @@ -41,7 +41,7 @@
>  			 role.nxe ? "" : "!",				\
>  			 __entry->root_count,				\
>  			 __entry->unsync ? "unsync" : "sync", 0);	\
> -	ret;								\
> +	p->buffer + saved_len;						\
>  		})
>  
>  #define kvm_mmu_trace_pferr_flags       \
> 

Applying this patch, thanks.

Paolo

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

[Rustad, Mark D]

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.

-- 
Mark Rustad, Networking Division, Intel Corporation

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

[Paolo Bonzini]

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

[PATCH V3 3/4] x86/kvm: Resolve shadow warnings in macro expansion

[Mark D Rustad]

Resolve shadow warnings that appear in W=2 builds. Instead of
using ret to hold the return pointer, save the length in a new
variable saved_len and compute the pointer on exit. This also
resolves a very technical error, in that ret was declared as
a const char *, when it really was a char * const, which
theoretically could have allowed the compiler to do something
wrong.

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

---
Changes in V2:
- Instead of renaming all inner variables, just delete the
  ret variable in favor of the new saved_len variable.
---
 arch/x86/kvm/mmutrace.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ffcb190..5aaf35641768 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,7 +22,7 @@
 	__entry->unsync = sp->unsync;
 
 #define KVM_MMU_PAGE_PRINTK() ({				        \
-	const char *ret = p->buffer + p->len;				\
+	const u32 saved_len = p->len;					\
 	static const char *access_str[] = {			        \
 		"---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux"  \
 	};							        \
@@ -41,7 +41,7 @@
 			 role.nxe ? "" : "!",				\
 			 __entry->root_count,				\
 			 __entry->unsync ? "unsync" : "sync", 0);	\
-	ret;								\
+	p->buffer + saved_len;						\
 		})
 
 #define kvm_mmu_trace_pferr_flags       \