Re: [RFC][PATCH] mips: Fix arch_spin_unlock()

From: Linus Torvalds
Date: Tue Feb 02 2016 - 03:19:10 EST


On Tue, Feb 2, 2016 at 12:07 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> So we *absolutely* should say that *OF COURSE* these things work:
>
> - CPU A:
>
> .. initialize data structure -> smp_wmb() -> WRITE_ONCE(ptr);
>
> - CPU B:
>
> smp_load_acquire(ptr) - we can rely on things behind "ptr" being initialized

That's a bad example, btw. I shouldn't have made it be a "pointer",
because then we get the whole address dependency chain ordering
anyway.

So instead of "ptr", read "state flag". It might just be an "int" that
says "data has been initialized".

So

.. initialize memory ..
smp_wmb();
WRITE_ONCE(&is_initialized, 1);

should pair with

if (smp_load_acquire(&is_initialized))
... we can read and write the data, knowing it has been initialized ..

exactly because "smp_wmb()" (cheap write barrier) might be cheaper
than "smp_store_release()" (expensive full barrier) and thus
preferred.

So mixing ordering metaphors actually does make sense, and should be
entirely well-defined.

There's likely less reason to do it the other way (ie
"smp_store_release()" on one side pairing with "LOAD_ONCE() +
smp_rmb()" on the other) since there likely isn't the same kind of
performance reason for that pairing. But even if we would never
necessarily want to do it, I think our memory ordering rules would be
*much* better for strongly stating that it has to work, than being
timid and trying to make the rules weak.

Memory ordering is confusing enough as it is. We should not make
people worry more than they already have to. Strong rules are good.

Linus