Messages in this thread | | | Date | Thu, 27 Jul 2017 15:56:10 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH tip/core/rcu 4/5] sys_membarrier: Add expedited option |
| |
On Thu, Jul 27, 2017 at 06:08:16AM -0700, Paul E. McKenney wrote:
> > So I think we need either switch_mm() or switch_to() to imply a full > > barrier for this to work, otherwise we get: > > > > CPU0 CPU1 > > > > > > lock rq->lock > > mb > > > > rq->curr = A > > > > unlock rq->lock > > > > lock rq->lock > > mb > > > > sys_membarrier() > > > > mb > > > > for_each_online_cpu() > > p = A > > // no match no IPI > > > > mb > > rq->curr = B > > > > unlock rq->lock > > > > > > And that's bad, because now CPU0 doesn't have an MB happening _after_ > > sys_membarrier() if B matches. > > Yes, this looks somewhat similar to the scenario that Mathieu pointed out > back in 2010: https://marc.info/?l=linux-kernel&m=126349766324224&w=2
Yes. Minus the mm_cpumask() worries.
> > So without audit, I only know of PPC and Alpha not having a barrier in > > either switch_*(). > > > > x86 obviously has barriers all over the place, arm has a super duper > > heavy barrier in switch_to(). > > Agreed, if we are going to rely on ->mm, we need ordering on assignment > to it.
Right, Boqun provided this reordering to show the problem:
CPU0 CPU1
<in process X> lock rq->lock mb
rq->curr = A
unlock rq->lock
<switch to process A>
lock rq->lock mb read Y(reordered)<---+ | store to Y | | sys_membarrier() | | mb | | for_each_online_cpu() | p = A | // no match no IPI | | mb | | store to X rq->curr = B | | unlock rq->lock | <switch to B> | read X | | read Y --------------+
| |