From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934301AbcALKVT (ORCPT ); Tue, 12 Jan 2016 05:21:19 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:33000 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752562AbcALKVK (ORCPT ); Tue, 12 Jan 2016 05:21:10 -0500 Date: Tue, 12 Jan 2016 11:21:05 +0100 From: Ingo Molnar To: Andy Lutomirski Cc: Peter Zijlstra , "linux-kernel@vger.kernel.org" , Dave Hansen , Rik van Riel , Brian Gerst , Andrew Morton , Denys Vlasenko , "H. Peter Anvin" , Thomas Gleixner , Borislav Petkov , Andrew Lutomirski , Linus Torvalds , "linux-tip-commits@vger.kernel.org" Subject: Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm() -vs-flush synchronization Message-ID: <20160112102105.GA4878@gmail.com> References: <20160111182548.GF6344@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Andy Lutomirski wrote: > On Mon, Jan 11, 2016 at 10:25 AM, Peter Zijlstra wrote: > > On Mon, Jan 11, 2016 at 03:42:40AM -0800, tip-bot for Andy Lutomirski wrote: > >> --- a/arch/x86/include/asm/mmu_context.h > >> +++ b/arch/x86/include/asm/mmu_context.h > >> @@ -116,8 +116,34 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, > >> #endif > >> cpumask_set_cpu(cpu, mm_cpumask(next)); > >> > >> - /* Re-load page tables */ > >> + /* > >> + * Re-load page tables. > >> + * > >> + * This logic has an ordering constraint: > >> + * > >> + * CPU 0: Write to a PTE for 'next' > >> + * CPU 0: load bit 1 in mm_cpumask. if nonzero, send IPI. > >> + * CPU 1: set bit 1 in next's mm_cpumask > >> + * CPU 1: load from the PTE that CPU 0 writes (implicit) > >> + * > >> + * We need to prevent an outcome in which CPU 1 observes > >> + * the new PTE value and CPU 0 observes bit 1 clear in > >> + * mm_cpumask. (If that occurs, then the IPI will never > >> + * be sent, and CPU 0's TLB will contain a stale entry.) > >> + * > >> + * The bad outcome can occur if either CPU's load is > >> + * reordered before that CPU's store, so both CPUs much > > > > s/much/must/ ? > > Indeed. Is this worth a follow-up patch? Absolutely! Any typos in code noticed by humans are worth fixing, especially when it's comments around tricky code. Could be done together with improving this part of the comments: > > + > > /* > > * We were in lazy tlb mode and leave_mm disabled > > * tlb flush IPI delivery. We must reload CR3 > > * to make sure to use no freed page tables. > > + * > > + * As above, this is a barrier that forces > > + * TLB repopulation to be ordered after the > > + * store to mm_cpumask. > > somewhat confused by this comment, cpumask_set_cpu() is a LOCK BTS, that is > already fully ordered. ... as pretty much any barriers related comment that confuses Peter needs to be improved, by definition. ;-) Thanks, Ingo