On Fri, 2013-10-11 at 17:07 -0500, Scott Wood wrote: > On Fri, 2013-10-11 at 10:51 +1100, Benjamin Herrenschmidt wrote: > > 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... > > Oops, I was seeing the conditional sync from SYNC_IO in the disassembly. > BTW, it's a bug that we don't do SYNC_IO on e500mc -- the assumption > that lwsync is 64-bit-only is no longer true.
Patch welcome :) > > > 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. > > Yes, and the lwsync is good enough for software reading the PTE. So it > becomes a question of how much spurious faults with hardware tablewalk > hurt performance, and at least for the lmbench fork test, the sync is > worse (or maybe lwsync happens to be good enough for hw tablewalk on > e6500?). > > > 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. > > But are callers of ioremap() expected to use a barrier before exposing > the pointer (and what type)? I don't think that's common practice. > > map_kernel_page() should not be performance critical, so it shouldn't be > a big deal to put mb() in there. Yup, go for it. Cheers, Ben. > -Scott > > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
