Re: [PATCH 2/7] x86/mm/tlb: Restructure switch_mm_irqs_off()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Sep 25, 2018 at 11:58:39PM -0400, Rik van Riel wrote:
> Move some code that will be needed for the lazy -> !lazy state
> transition when a lazy TLB CPU has gotten out of date.
> 
> No functional changes, since the if (real_prev == next) branch
> always returns.
> 
> Suggested-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Acked-by: Dave Hansen <dave.hansen@intel.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: efault@gmx.de
> Cc: kernel-team@fb.com
> Link: http://lkml.kernel.org/r/20180716190337.26133-4-riel@surriel.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> (cherry picked from commit 61d0beb5796ab11f7f3bf38cb2eccc6579aaa70b)
> ---
>  arch/x86/mm/tlb.c | 64 ++++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 54a5870190a6..1224f7fb1311 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -187,6 +187,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  	u16 prev_asid = this_cpu_read(cpu_tlbstate.loaded_mm_asid);
>  	unsigned cpu = smp_processor_id();
>  	u64 next_tlb_gen;
> +	bool need_flush;
> +	u16 new_asid;
>  
>  	/*
>  	 * NB: The scheduler will call us with prev == next when switching
> @@ -308,44 +310,44 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  		/* Let nmi_uaccess_okay() know that we're changing CR3. */
>  		this_cpu_write(cpu_tlbstate.loaded_mm, LOADED_MM_SWITCHING);
>  		barrier();
> +	}

Compiling this gives me:

CC      arch/x86/mm/tlb.o
../arch/x86/mm/tlb.c: In function ‘switch_mm_irqs_off’:
../arch/x86/mm/tlb.c:315:5: warning: ‘need_flush’ may be used uninitialized in this function [-Wmaybe-uninitialized]
if (need_flush) {
^
../arch/x86/mm/tlb.c:316:45: warning: ‘new_asid’ may be used uninitialized in this function [-Wmaybe-uninitialized]
this_cpu_write(cpu_tlbstate.ctxs[new_asid].ctx_id, next->context.ctx_id);
^

Because you shadow need_flush and new_asid in a branch. I need the below
delta to make it happy again.

---
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -254,8 +254,6 @@ void switch_mm_irqs_off(struct mm_struct
 
 		return;
 	} else {
-		u16 new_asid;
-		bool need_flush;
 		u64 last_ctx_id = this_cpu_read(cpu_tlbstate.last_ctx_id);
 
 		/*