From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755148AbcBBMDH (ORCPT ); Tue, 2 Feb 2016 07:03:07 -0500 Received: from e38.co.us.ibm.com ([32.97.110.159]:51311 "EHLO e38.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754775AbcBBMDE (ORCPT ); Tue, 2 Feb 2016 07:03:04 -0500 X-IBM-Helo: d03dlp03.boulder.ibm.com X-IBM-MailFrom: paulmck@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Date: Tue, 2 Feb 2016 04:02:52 -0800 From: "Paul E. McKenney" To: Linus Torvalds Cc: Boqun Feng , Will Deacon , Peter Zijlstra , "Maciej W. Rozycki" , David Daney , =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= , Ralf Baechle , Linux Kernel Mailing List Subject: Re: [RFC][PATCH] mips: Fix arch_spin_unlock() Message-ID: <20160202120252.GI6719@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20160128095718.GC30928@arm.com> <20160128223131.GV4503@linux.vnet.ibm.com> <20160129095958.GA4541@arm.com> <20160129102253.GG4503@linux.vnet.ibm.com> <20160201135621.GD6828@arm.com> <20160202035458.GF6719@linux.vnet.ibm.com> <20160202051904.GC1239@fixme-laptop.cn.ibm.com> <20160202064433.GG6719@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16020212-0029-0000-0000-00001025A858 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 02, 2016 at 12:19:04AM -0800, Linus Torvalds wrote: > On Tue, Feb 2, 2016 at 12:07 AM, Linus Torvalds > 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. I don't believe that anyone is arguing that this particular example should not work the way that you want it to. > 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. The sorts of things I am really worried about are abominations like this (and far worse): void thread0(void) { r1 = smp_load_acquire(&a); smp_store_release(&b, 1); } void thread1(void) { r2 = smp_load_acquire(&b); smp_store_release(&c, 1); } void thread2(void) { WRITE_ONCE(c, 2); smp_mb(); r3 = READ_ONCE(d); } void thread3(void) { WRITE_ONCE(d, 1); smp_store_release(&a, 1); } r1 == 1 && r2 == 1 && c == 2 && r3 == 0 ??? I advise discouraging this sort of thing. But it is your kernel, so what is your preference? Thanx, Paul