Re: [RFC][PATCH 5/5] powerpc: Remove SYNC from _switch

On Thu, 8 Jun 2017 08:54:00 +0200
Peter Zijlstra [off-list ref] wrote:

On Thu, Jun 08, 2017 at 10:32:44AM +1000, Nicholas Piggin wrote:

quoted

On Wed, 07 Jun 2017 18:15:06 +0200
Peter Zijlstra [off-list ref] wrote:
  

quoted

Now that the scheduler's rq->lock is RCsc and thus provides full
transitivity between scheduling actions. And since we cannot migrate
current, a task needs a switch-out and a switch-in in order to
migrate, in which case the RCsc provides all the ordering we need.  

Hi Peter,

I'm actually just working on removing this right now too, so
good timing.

I think we can't "just" remove it, because it is required to order
MMIO on powerpc as well.  

How is MMIO special? That is, there is only MMIO before we call into
schedule() right? So the rq->lock should be sufficient to order that
too.

MMIO uses different barriers. spinlock and smp_ type barriers do
not order it.

quoted

But what I have done is to comment that some other primitives are
already providing the hwsync for other, so we don't have to add
another one in _switch.  

Right, so this patch relies on the smp_mb__before_spinlock ->
smp_mb__after_spinlock conversion that makes the rq->lock RCsc and
should thus provide the required SYNC for migrations.

AFAIKS either one will do, so long as there is a hwsync there. The
point is just that I have added some commentary in the generic and
powerpc parts to make it clear we're relying on that behavior of
the primitive. smp_mb* is not guaranteed to order MMIO, it's just
that it does on powerpc.

That said, I think you can already use the smp_mb__before_spinlock() as
that is done with IRQs disabled, but its a more difficult argument. The
rq->lock RCsc property should be more obvious.

This is what I got.

https://patchwork.ozlabs.org/patch/770154/

But I'm not sure if I followed I'm not sure why it's a more
difficult argument: any time a process moves it must first execute
a hwsync on the current CPU after it has performed all its access
there, and then it must execute hwsync on the new CPU before it
performs any new access.