linux-kernel - Re: [GIT PULL] locking changes for v4.4

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]

Message-ID: <CACT4Y+bPVaeSsJbpG2fsQ-C+zNoSnywTZZD1yPt-9hNY3tmgLQ@mail.gmail.com>
Date:	Wed, 4 Nov 2015 15:51:01 +0100
From:	Dmitry Vyukov <dvyukov@...gle.com>
To:	Paul McKenney <paulmck@...ux.vnet.ibm.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] locking changes for v4.4

On Wed, Nov 4, 2015 at 5:16 AM, Paul E. McKenney
<paulmck@...ux.vnet.ibm.com> wrote:
> On Tue, Nov 03, 2015 at 05:30:29PM -0800, Linus Torvalds wrote:
>> On Tue, Nov 3, 2015 at 3:58 PM, Linus Torvalds
>> <torvalds@...ux-foundation.org> wrote:
>> >
>> > I think I'll pull this, but then just make a separate commit to remove
>> > all the bogus games with "control" dependencies that seem to have no
>> > basis is reality.
>>
>> So the attached is what I committed in my tree. It took much longer to
>> try to write the rationale than it took to actually remove the
>> atomic_read_ctrl() functions, and even so I'm not sure how good that
>> commit message is. But at least it tries to explain what's going on.
>>
>> Note the final part of the rationale:
>>
>>     I may have to eat my words at some point, but in the absense of clear
>>     proof that alpha actually needs this, or indeed even an explanation of
>>     how alpha could _possibly_ need it, I do not believe these functions are
>>     called for.
>>
>>     And if it turns out that alpha really _does_ need a barrier for this
>>     case, that barrier still should not be "smp_read_barrier_depends()".
>>     We'd have to make up some new speciality barrier just for alpha, along
>>     with the documentation for why it really is necessary.
>
> For whatever it is worth, the patch looks good to me.  The reasons I
> could imagine why we might want to mark control dependencies are things
> like documentation and tooling, but given that we currently only have a
> very small number of them, it is hard to argue that this is of immediate
> concern, if it is ever of concern.


To clarify, yes, documentation and tooling was my main motivation.
It is usually helpful to see acquire/release, rmb/wmb pairs, and so it
is useful to know that something below is ordered wrt this load by
means of a control dependency (which effectively becomes an acquire,
and there must be a pairing release somewhere).
As for the tooling, as you may know, we are working on
KernelThreadSanitizer, which is dynamic happens-before-based race
detector (dynamically tracks acquire/release pairs to establish
synchronizes-with relation). As shown by memory_order_consume tracking
dependencies in compiler is practically impossible, and so the tool
needs some hint for these.
But we don't need to sort it out right now. And I agree we need a
clear idea as to why we are doing this. And we definitely must not
hide it under the alpha excuse. And our main headache for now is
"benign" data races anyway (missing READ/WRITE_ONCE).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/