Re: [PATCH v2 1/3] powerpc/booke64: add sync after writing PTE

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2013-10-10 at 18:25 -0500, Scott Wood wrote:

> Looking at some of the code in mm/, I suspect that the normal callers of
> set_pte_at() already have an unlock (and thus a sync) 

Unlock is lwsync actually...

> already, so we may
> not even be relying on those retries.  Certainly some of them do; it
> would take some effort to verify all of them.
> 
> Also, without such a sync in map_kernel_page(), even with software
> tablewalk, couldn't we theoretically have a situation where a store to
> pointer X that exposes a new mapping gets reordered before the PTE store
> as seen by another CPU?  The other CPU could see non-NULL X and
> dereference it, but get the stale PTE.  Callers of ioremap() generally
> don't do a barrier of their own prior to exposing the result.

Hrm, we transition to the new PTE either restricts the access permission
in which case it flushes the TLB (and synchronizes with other CPUs) or
extends access (adds dirty, set pte from 0 -> populated, ...) in which
case the worst case is we see the old one and take a spurrious fault.

So the problem would only be with kernel mappings and in that case I
think we are fine. A driver doing an ioremap shouldn't then start using
that mapping on another CPU before having *informed* that other CPU of
the existence of the mapping and that should be ordered.

Ben.