From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754070AbbGBOOd (ORCPT ); Thu, 2 Jul 2015 10:14:33 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:53022 "EHLO e33.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754405AbbGBONk (ORCPT ); Thu, 2 Jul 2015 10:13:40 -0400 X-Helo: d03dlp03.boulder.ibm.com X-MailFrom: paulmck@linux.vnet.ibm.com X-RcptTo: linux-kernel@vger.kernel.org Date: Thu, 2 Jul 2015 07:13:30 -0700 From: "Paul E. McKenney" To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, laijs@cn.fujitsu.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com, dvhart@linux.intel.com, fweisbec@gmail.com, oleg@redhat.com, bobby.prani@gmail.com Subject: Re: [PATCH RFC tip/core/rcu 05/14] rcu: Abstract sequence counting from synchronize_sched_expedited() Message-ID: <20150702141330.GI3717@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20150630222530.GA12044@linux.vnet.ibm.com> <1435703154-14659-1-git-send-email-paulmck@linux.vnet.ibm.com> <1435703154-14659-5-git-send-email-paulmck@linux.vnet.ibm.com> <20150701102717.GT19282@twins.programming.kicks-ass.net> <20150701221804.GW3717@linux.vnet.ibm.com> <20150702085041.GI25159@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150702085041.GI25159@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15070214-0009-0000-0000-00000C2D7BE3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 02, 2015 at 10:50:41AM +0200, Peter Zijlstra wrote: > On Wed, Jul 01, 2015 at 03:18:04PM -0700, Paul E. McKenney wrote: > > On Wed, Jul 01, 2015 at 12:27:17PM +0200, Peter Zijlstra wrote: > > > > That wants to be an ACQUIRE, right? > > > > I cannot put the acquire in the WARN_ON_ONCE() because there > > are configurations where WARN_ON_ONCE() is compiled out. I could > > conditionally compile, but given that this is nothing like a fastpath, > > I cannot really justify doing that. > > Fair enough. > > > We could define an smp_store_acquire(), but that would require a full > > barrier against subsequent loads. The C++ committee hit this one when > > trying to implement seqeunce locking using the C/C++11 atomics. ;-) > > Yeah, I'm not sure how much sense smp_store_acquire() makes, but I'm > fairly sure this isn't the first time I've wondered about it. > > > > > +static bool rcu_seq_done(unsigned long *sp, unsigned long s) > > > > +{ > > > > + return ULONG_CMP_GE(READ_ONCE(*sp), s); > > > > > > I'm always amused you're not wanting to rely on 2s complement for > > > integer overflow. I _know_ its undefined behaviour in the C rule book, > > > but the entire rest of the kernel hard assumes it. > > > > I take it you have never seen the demonic glow in the eyes of a compiler > > implementer when thinking of all the code that can be broken^W^W^W^W^W > > optimizations that are enabled by relying on undefined behavior for > > signed integer overflow? ;-) > > Note that this is unsigned integers, but yes I know, you've said. But > they cannot unilaterally change this 'undefined' behaviour because its > been defined as 'whatever the hardware does' for such a long time. For pure unsigned arithmetic, their options are indeed limited. For a cast to signed, I am not so sure. I have been using time_before() and friends for jiffy comparisons, which does a cast to signed after the subtraction. Signed overflow is already unsafe with current compilers, though the kernel suppresses these. > Likewise they can dream all they want about breaking our concurrent code > and state we should use the brand spanking new primitives, sod 30 years > of existing code, but that's just not realistic either. > > Even if we didn't 'have' to support a wide range of compiler versions, > most of which do not even support these new fangled primitives, who is > going to audit our existing many million lines of code? Not to mention > the many more million lines of code in other projects that rely on these > same things. > > Its really time for them to stop wanking and stare reality in the face. Indeed, I have been and will be continuing to make myself unpopular with that topic. ;-) > > > > +/* Wrapper functions for expedited grace periods. */ > > > > +static void rcu_exp_gp_seq_start(struct rcu_state *rsp) > > > > +{ > > > > + rcu_seq_start(&rsp->expedited_sequence); > > > > +} > > > > +static void rcu_exp_gp_seq_end(struct rcu_state *rsp) > > > > +{ > > > > + rcu_seq_end(&rsp->expedited_sequence); > > > > +} > > > > +static unsigned long rcu_exp_gp_seq_snap(struct rcu_state *rsp) > > > > +{ > > > > + return rcu_seq_snap(&rsp->expedited_sequence); > > > > +} > > > > +static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s) > > > > +{ > > > > + return rcu_seq_done(&rsp->expedited_sequence, s); > > > > +} > > > > > > This is wrappers for wrappers sake? Why? > > > > For _rcu_barrier(), as noted in the commit log. > > Yes it said; but why? Surely _rcu_barrier() can do the > ->expedited_sequence thing itself, that hardly seems worthy of a > wrapper. Ah, you want synchronize_rcu_expedited() and synchronize_sched_expedited() to use rcu_seq_start() and friends directly. I can certainly do that. Thanx, Paul