Re: [PATCH 3/6] powerpc/mm: Ensure cpumask update is ordered

[Nicholas Piggin <npiggin@gmail.com>]

On Mon, 24 Jul 2017 21:20:07 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> On Mon, 24 Jul 2017 14:28:00 +1000
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > There is no guarantee that the various isync's involved with
> > the context switch will order the update of the CPU mask with
> > the first TLB entry for the new context being loaded by the HW.
> > 
> > Be safe here and add a memory barrier to order any subsequent
> > load/store which may bring entries into the TLB.
> > 
> > The corresponding barrier on the other side already exists as
> > pte updates use pte_xchg() which uses __cmpxchg_u64 which has
> > a sync after the atomic operation.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >  arch/powerpc/include/asm/mmu_context.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > index ed9a36ee3107..ff1aeb2cd19f 100644
> > --- a/arch/powerpc/include/asm/mmu_context.h
> > +++ b/arch/powerpc/include/asm/mmu_context.h
> > @@ -110,6 +110,7 @@ static inline void switch_mm_irqs_off(struct mm_struct *prev,
> >  	/* Mark this context has been used on the new CPU */
> >  	if (!cpumask_test_cpu(smp_processor_id(), mm_cpumask(next))) {
> >  		cpumask_set_cpu(smp_processor_id(), mm_cpumask(next));
> > +		smp_mb();
> >  		new_on_cpu = true;
> >  	}
> >    
> 
> I think this is the right thing to do, but it should be commented.
> Is hwsync the right barrier? (i.e., it will order the page table walk)

After some offline discussion, I think we have an agreement that
this is the right barrier, as it orders with the subsequent load
of next->context.id that the mtpid depends on (or slbmte for HPT).

So we should have a comment here to that effect, and including
the pte_xchg comments from your changelog. Some comment (at least
refer back to here) added at pte_xchg too please.

Other than that your series seems good to me if you repost it you
can add

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

This one out of the series is the bugfix so it should go to stable
as well, right?

Thanks,
Nick