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

Groups > linux.kernel > #1681496 > unrolled thread

Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID

Started byPeter Zijlstra <peterz@infradead.org>
First post2017-07-05 14:20 +0200
Last post2017-07-18 19:10 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using  PCID Peter Zijlstra <peterz@infradead.org> - 2017-07-05 14:20 +0200
    Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-07-05 18:10 +0200
      Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using  PCID Peter Zijlstra <peterz@infradead.org> - 2017-07-05 19:10 +0200
        Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using  PCID Ingo Molnar <mingo@kernel.org> - 2017-07-18 11:00 +0200
          Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID Andy Lutomirski <luto@kernel.org> - 2017-07-18 19:10 +0200

#1681496 — Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID

FromPeter Zijlstra <peterz@infradead.org>
Date2017-07-05 14:20 +0200
SubjectRe: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID
Message-ID<tZRrI-67q-17@gated-at.bofh.it>
On Thu, Jun 29, 2017 at 08:53:22AM -0700, Andy Lutomirski wrote:
> @@ -104,18 +140,20 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  
>  		/* Resume remote flushes and then read tlb_gen. */
>  		cpumask_set_cpu(cpu, mm_cpumask(next));

Barriers should have a comment... what is being ordered here against
what?

> +		smp_mb__after_atomic();
>  		next_tlb_gen = atomic64_read(&next->context.tlb_gen);
>  
> +		if (this_cpu_read(cpu_tlbstate.ctxs[prev_asid].tlb_gen) <
> +		    next_tlb_gen) {
>  			/*
>  			 * Ideally, we'd have a flush_tlb() variant that
>  			 * takes the known CR3 value as input.  This would
>  			 * be faster on Xen PV and on hypothetical CPUs
>  			 * on which INVPCID is fast.
>  			 */
> +			this_cpu_write(cpu_tlbstate.ctxs[prev_asid].tlb_gen,
>  				       next_tlb_gen);
> +			write_cr3(__pa(next->pgd) | prev_asid);
>  			trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH,
>  					TLB_FLUSH_ALL);
>  		}

> @@ -152,14 +190,25 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  		 * Start remote flushes and then read tlb_gen.
>  		 */
>  		cpumask_set_cpu(cpu, mm_cpumask(next));
> +		smp_mb__after_atomic();

idem

>  		next_tlb_gen = atomic64_read(&next->context.tlb_gen);
>  
> +		choose_new_asid(next, next_tlb_gen, &new_asid, &need_flush);
>  
> +		if (need_flush) {
> +			this_cpu_write(cpu_tlbstate.ctxs[new_asid].ctx_id, next->context.ctx_id);
> +			this_cpu_write(cpu_tlbstate.ctxs[new_asid].tlb_gen, next_tlb_gen);
> +			write_cr3(__pa(next->pgd) | new_asid);
> +			trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH,
> +					TLB_FLUSH_ALL);
> +		} else {
> +			/* The new ASID is already up to date. */
> +			write_cr3(__pa(next->pgd) | new_asid | CR3_NOFLUSH);
> +			trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH, 0);
> +		}
> +
> +		this_cpu_write(cpu_tlbstate.loaded_mm, next);
> +		this_cpu_write(cpu_tlbstate.loaded_mm_asid, new_asid);
>  	}
>  
>  	load_mm_cr4(next);

[toc] | [next] | [standalone]

#1681630 — Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID

FromAndy Lutomirski <luto@kernel.org>
Date2017-07-05 18:10 +0200
SubjectRe: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID
Message-ID<tZV2j-8vO-51@gated-at.bofh.it>
In reply to#1681496
On Wed, Jul 5, 2017 at 5:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jun 29, 2017 at 08:53:22AM -0700, Andy Lutomirski wrote:
>> @@ -104,18 +140,20 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>>
>>               /* Resume remote flushes and then read tlb_gen. */
>>               cpumask_set_cpu(cpu, mm_cpumask(next));
>
> Barriers should have a comment... what is being ordered here against
> what?

How's this comment?

        /*
         * Resume remote flushes and then read tlb_gen.  We need to do
         * it in this order: any inc_mm_tlb_gen() caller that writes a
         * larger tlb_gen than we read here must see our cpu set in
         * mm_cpumask() so that it will know to flush us.  The barrier
         * here synchronizes with inc_mm_tlb_gen().
         */

[toc] | [prev] | [next] | [standalone]

#1681663

FromPeter Zijlstra <peterz@infradead.org>
Date2017-07-05 19:10 +0200
Message-ID<tZVYm-Ge-9@gated-at.bofh.it>
In reply to#1681630
On Wed, Jul 05, 2017 at 09:04:39AM -0700, Andy Lutomirski wrote:
> On Wed, Jul 5, 2017 at 5:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Jun 29, 2017 at 08:53:22AM -0700, Andy Lutomirski wrote:
> >> @@ -104,18 +140,20 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
> >>
> >>               /* Resume remote flushes and then read tlb_gen. */
> >>               cpumask_set_cpu(cpu, mm_cpumask(next));
> >
> > Barriers should have a comment... what is being ordered here against
> > what?
> 
> How's this comment?
> 
>         /*
>          * Resume remote flushes and then read tlb_gen.  We need to do
>          * it in this order: any inc_mm_tlb_gen() caller that writes a
>          * larger tlb_gen than we read here must see our cpu set in
>          * mm_cpumask() so that it will know to flush us.  The barrier
>          * here synchronizes with inc_mm_tlb_gen().
>          */

Slightly confusing, you mean this, right?


	cpumask_set_cpu(cpu, mm_cpumask());			inc_mm_tlb_gen();

	MB							MB

	next_tlb_gen = atomic64_read(&next->context.tlb_gen);	flush_tlb_others(mm_cpumask());


which seems to make sense.

[toc] | [prev] | [next] | [standalone]

#1689893

FromIngo Molnar <mingo@kernel.org>
Date2017-07-18 11:00 +0200
Message-ID<u4wwj-9E-49@gated-at.bofh.it>
In reply to#1681663
* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Jul 05, 2017 at 09:04:39AM -0700, Andy Lutomirski wrote:
> > On Wed, Jul 5, 2017 at 5:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Thu, Jun 29, 2017 at 08:53:22AM -0700, Andy Lutomirski wrote:
> > >> @@ -104,18 +140,20 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
> > >>
> > >>               /* Resume remote flushes and then read tlb_gen. */
> > >>               cpumask_set_cpu(cpu, mm_cpumask(next));
> > >
> > > Barriers should have a comment... what is being ordered here against
> > > what?
> > 
> > How's this comment?
> > 
> >         /*
> >          * Resume remote flushes and then read tlb_gen.  We need to do
> >          * it in this order: any inc_mm_tlb_gen() caller that writes a
> >          * larger tlb_gen than we read here must see our cpu set in
> >          * mm_cpumask() so that it will know to flush us.  The barrier
> >          * here synchronizes with inc_mm_tlb_gen().
> >          */
> 
> Slightly confusing, you mean this, right?
> 
> 
> 	cpumask_set_cpu(cpu, mm_cpumask());			inc_mm_tlb_gen();
> 
> 	MB							MB
> 
> 	next_tlb_gen = atomic64_read(&next->context.tlb_gen);	flush_tlb_others(mm_cpumask());
> 
> 
> which seems to make sense.

Btw., I'll wait for a v5 iteration before applying this last patch to tip:x86/mm.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1690488 — Re: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID

FromAndy Lutomirski <luto@kernel.org>
Date2017-07-18 19:10 +0200
SubjectRe: [PATCH v4 10/10] x86/mm: Try to preserve old TLB entries using PCID
Message-ID<u4Eax-5aT-87@gated-at.bofh.it>
In reply to#1689893
On Tue, Jul 18, 2017 at 1:53 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> On Wed, Jul 05, 2017 at 09:04:39AM -0700, Andy Lutomirski wrote:
>> > On Wed, Jul 5, 2017 at 5:18 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > > On Thu, Jun 29, 2017 at 08:53:22AM -0700, Andy Lutomirski wrote:
>> > >> @@ -104,18 +140,20 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>> > >>
>> > >>               /* Resume remote flushes and then read tlb_gen. */
>> > >>               cpumask_set_cpu(cpu, mm_cpumask(next));
>> > >
>> > > Barriers should have a comment... what is being ordered here against
>> > > what?
>> >
>> > How's this comment?
>> >
>> >         /*
>> >          * Resume remote flushes and then read tlb_gen.  We need to do
>> >          * it in this order: any inc_mm_tlb_gen() caller that writes a
>> >          * larger tlb_gen than we read here must see our cpu set in
>> >          * mm_cpumask() so that it will know to flush us.  The barrier
>> >          * here synchronizes with inc_mm_tlb_gen().
>> >          */
>>
>> Slightly confusing, you mean this, right?
>>
>>
>>       cpumask_set_cpu(cpu, mm_cpumask());                     inc_mm_tlb_gen();
>>
>>       MB                                                      MB
>>
>>       next_tlb_gen = atomic64_read(&next->context.tlb_gen);   flush_tlb_others(mm_cpumask());
>>
>>
>> which seems to make sense.
>
> Btw., I'll wait for a v5 iteration before applying this last patch to tip:x86/mm.

I'll send it shortly.  I think I'll also add a patch to factor out the
flush calls a bit more to prepare for Mel's upcoming fix.

>
> Thanks,
>
>         Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web