Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1306591

Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm() -vs-flush synchronization

Path csiph.com!news.mixmin.net!weretis.net!feeder4.news.weretis.net!storethat.news.telefonica.de!telefonica.de!news.panservice.it!bofh.it!news.nic.it!robomod
From Peter Zijlstra <peterz@infradead.org>
Newsgroups linux.kernel
Subject Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm() -vs-flush synchronization
Date Mon, 11 Jan 2016 19:30:03 +0100
Message-ID <qPPo7-67U-27@gated-at.bofh.it> (permalink)
References <qPPo7-67U-29@gated-at.bofh.it>
MIME-Version 1.0
Content-Type text/plain; charset=us-ascii
Content-Disposition inline
User-Agent Mutt/1.5.21 (2012-12-30)
Sender robomod@news.nic.it
List-ID <linux-kernel.vger.kernel.org>
X-Mailing-List linux-kernel@vger.kernel.org
Approved robomod@news.nic.it
Lines 90
Organization linux.* mail to news gateway
X-Original-Cc linux-tip-commits@vger.kernel.org
X-Original-Date Mon, 11 Jan 2016 19:25:48 +0100
X-Original-Message-ID <20160111182548.GF6344@twins.programming.kicks-ass.net>
X-Original-References <tip-71b3c126e61177eb693423f2e18a1914205b165e@git.kernel.org>
X-Original-Sender linux-kernel-owner@vger.kernel.org
Xref csiph.com linux.kernel:1306591

Show key headers only | View raw


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/ ?

> +		 * execute full barriers to prevent this from happening.
> +		 *
> +		 * Thus, switch_mm needs a full barrier between the
> +		 * store to mm_cpumask and any operation that could load
> +		 * from next->pgd.  This barrier synchronizes with
> +		 * remote TLB flushers.  Fortunately, load_cr3 is
> +		 * serializing and thus acts as a full barrier.
> +		 *
> +		 */
>  		load_cr3(next->pgd);
> +
>  		trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL);
>  
>  		/* Stop flush ipis for the previous mm */
> @@ -156,10 +182,15 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  			 * schedule, protecting us from simultaneous changes.
>  			 */
>  			cpumask_set_cpu(cpu, mm_cpumask(next));
> +
>  			/*
>  			 * 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.

>  			 */
>  			load_cr3(next->pgd);
>  			trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL);
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 8ddb5d0..8f4cc3d 100644


> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c

> @@ -188,17 +191,29 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,

>  	if (!current->mm) {
>  		leave_mm(smp_processor_id());
> +
> +		/* Synchronize with switch_mm. */
> +		smp_mb();
> +
>  		goto out;
>  	}

> +		} else {
>  			leave_mm(smp_processor_id());
> +
> +			/* Synchronize with switch_mm. */
> +			smp_mb();
> +		}
>  	}

The alternative is making leave_mm() unconditionally imply a full
barrier. I've not looked at other sites using it though.

Back to linux.kernel | Previous | NextNext in thread | Find similar | Unroll thread


Thread

Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm()  -vs-flush synchronization Peter Zijlstra <peterz@infradead.org> - 2016-01-11 19:30 +0100
  Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm()  -vs-flush synchronization Andy Lutomirski <luto@amacapital.net> - 2016-01-11 23:00 +0100
    Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm()  -vs-flush synchronization Peter Zijlstra <peterz@infradead.org> - 2016-01-11 23:30 +0100
    Re: [tip:x86/urgent] x86/mm: Add barriers and document switch_mm()  -vs-flush synchronization Ingo Molnar <mingo@kernel.org> - 2016-01-12 11:30 +0100
      [PATCH] x86/mm: Improve switch_mm barrier comments Andy Lutomirski <luto@kernel.org> - 2016-01-12 21:20 +0100
      [PATCH v2] x86/mm: Improve switch_mm barrier comments Andy Lutomirski <luto@kernel.org> - 2016-01-12 21:50 +0100
        [tip:x86/urgent] x86/mm: Improve switch_mm() barrier comments tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-01-14 10:10 +0100

csiph-web