From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965554AbbKDOvW (ORCPT ); Wed, 4 Nov 2015 09:51:22 -0500 Received: from mail-oi0-f53.google.com ([209.85.218.53]:35263 "EHLO mail-oi0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965450AbbKDOvV (ORCPT ); Wed, 4 Nov 2015 09:51:21 -0500 MIME-Version: 1.0 In-Reply-To: <20151104041644.GO29027@linux.vnet.ibm.com> References: <20151103091636.GA23350@gmail.com> <20151104041644.GO29027@linux.vnet.ibm.com> From: Dmitry Vyukov Date: Wed, 4 Nov 2015 15:51:01 +0100 Message-ID: Subject: Re: [GIT PULL] locking changes for v4.4 To: Paul McKenney Cc: Linus Torvalds , Ingo Molnar , Linux Kernel Mailing List , Peter Zijlstra , Thomas Gleixner , Andrew Morton Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 4, 2015 at 5:16 AM, Paul E. McKenney 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 >> 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).