From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751732AbbKBTRU (ORCPT ); Mon, 2 Nov 2015 14:17:20 -0500 Received: from mail-io0-f179.google.com ([209.85.223.179]:34326 "EHLO mail-io0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751180AbbKBTRS (ORCPT ); Mon, 2 Nov 2015 14:17:18 -0500 MIME-Version: 1.0 In-Reply-To: <20151102183659.GN29657@arm.com> References: <20151102132901.157178466@infradead.org> <20151102134941.005198372@infradead.org> <20151102183659.GN29657@arm.com> Date: Mon, 2 Nov 2015 11:17:17 -0800 X-Google-Sender-Auth: A7HyM3PfblHKZrJy4u7n12fvjsc Message-ID: Subject: Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() From: Linus Torvalds To: Will Deacon Cc: Peter Zijlstra , Ingo Molnar , Oleg Nesterov , Linux Kernel Mailing List , Paul McKenney , boqun.feng@gmail.com, Jonathan Corbet , Michal Hocko , David Howells 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 Mon, Nov 2, 2015 at 10:37 AM, Will Deacon wrote: > On Mon, Nov 02, 2015 at 10:08:24AM -0800, Linus Torvalds wrote: >> On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra wrote: >> > +#define smp_cond_acquire(cond) do { \ >> > + while (!(cond)) \ >> > + cpu_relax(); \ >> > + smp_read_barrier_depends(); /* ctrl */ \ >> > + smp_rmb(); /* ctrl + rmb := acquire */ \ >> > +} while (0) >> >> This code makes absolutely no sense. >> >> smp_read_barrier_depends() is about a memory barrier where there is a >> data dependency between two accesses. The "depends" is very much about >> the data dependency, and very much about *nothing* else. > > Paul wasn't so sure, which I think is why smp_read_barrier_depends() > is already used in, for example, READ_ONCE_CTRL: > > http://lkml.kernel.org/r/20151007154003.GJ3910@linux.vnet.ibm.com Quoting the alpha architecture manual is kind of pointless, when NO OTHER ARCHITECTURE OUT THERE guararantees that whole "read + conditional orders wrt subsequent writes" _either_. (Again, with the exception of x86, which has the sane "we honor causality") Alpha isn't special. And smp_read_barrier_depends() hasn't magically become something new. If people think that control dependency needs a memory barrier on alpha, then it damn well needs on on all other weakly ordered architectuers too, afaik. Either that "you cannot finalize a write unless all conditionals it depends on are finalized" is true or it is not. That argument has *never* been about some architecture-specific memory ordering model, as far as I know. As to READ_ONCE_CTRL - two wrongs don't make a right. That smp_read_barrier_depends() there doesn't make any sense either. And finally, the alpha architecture manual actually does have the notion of "Dependence Constraint" (5.6.1.7) that talks about writes that depend on previous reads (where "depends" is explicitly spelled out to be about conditionals, write data _or_ write address). They are actually constrained on alpha too. Note that a "Dependence Constraint" is not a memory barrier, because it only affects that particular chain of dependencies. So it doesn't order other things in *general*, but it does order a particular read with a particular sef of subsequent write. Which is all we guarantee on anything else too wrt the whole control dependencies. The smp_read_barrier_depends() is a *READ BARRIER*. It's about a *read* that depends on a previous read. Now, it so happens that alpha doesn't have a "read-to-read" barrier, and it's implemented as a full barrier, but it really should be read as a "smp_rmb(). Yes, yes, alpha has the worst memory ordering ever, and the worst barriers for that insane memory ordering, but that still does not make alpha "magically able to reorder more than physically or logically possible". We don't add barriers that don't make sense just because the alpha memory orderings are insane. Paul? I really disagree with how you have totally tried to re-purpose smp_read_barrier_depends() in ways that are insane and make no sense. That is not a control dependency. If it was, then PPC and ARM would need to make it a real barrier too. I really don't see why you have singled out alpha as the victim of your "let's just randomly change the rules". > In this case, control dependencies are only referring to READ -> WRITE > ordering, so they are honoured by ARM and PowerPC. Do ARM and PPC actually guarantee the generic "previous reads always order before subsequent writes"? Because if that's the issue, then we should perhaps add a new ordering primitive that says that (ie "smp_read_to_write_barrier()"). That could be a useful thing in general, where we currently use "smp_mb()" to order an earlier read with a later write. Linus