linux-kernel - Re: [RFC][PATCH] mips: Fix arch_spin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Feb 2016 00:19:04 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Paul McKenney <paulmck@...ux.vnet.ibm.com>
Cc:	Boqun Feng <boqun.feng@...il.com>,
	Will Deacon <will.deacon@....com>,
	Peter Zijlstra <peterz@...radead.org>,
	"Maciej W. Rozycki" <macro@...tec.com>,
	David Daney <ddaney@...iumnetworks.com>,
	Måns Rullgård <mans@...sr.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH] mips: Fix arch_spin_unlock()

On Tue, Feb 2, 2016 at 12:07 AM, Linus Torvalds
<torvalds@...ux-foundation.org> 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