Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 2 Oct 2018 09:32:54 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Rik van Riel <riel@surriel.com>
Cc:     linux-kernel@vger.kernel.org, kernel-team@fb.com,
        will.deacon@arm.com, songliubraving@fb.com, mingo@kernel.org,
        luto@kernel.org, hpa@zytor.com, npiggin@gmail.com,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Thomas Gleixner <tglx@linutronix.de>, efault@gmx.de
Subject: Re: [PATCH 2/7] x86/mm/tlb: Restructure switch_mm_irqs_off()
Message-ID: <20181002073254.GP3439@hirez.programming.kicks-ass.net>
References: <20180926035844.1420-1-riel@surriel.com>
 <20180926035844.1420-3-riel@surriel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180926035844.1420-3-riel@surriel.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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);
 
 		/*