On Mon, 24 Jul 2017 21:20:07 +1000
Nicholas Piggin <npig...@gmail.com> wrote:
> On Mon, 24 Jul 2017 14:28:00 +1000
> Benjamin Herrenschmidt <b...@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 <b...@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 <npig...@gmail.com>
This one out of the series is the bugfix so it should go to stable
as well, right?
Thanks,
Nick
