From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751542AbdAWUGI (ORCPT ); Mon, 23 Jan 2017 15:06:08 -0500 Received: from mail-pf0-f195.google.com ([209.85.192.195]:36035 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751248AbdAWUGG (ORCPT ); Mon, 23 Jan 2017 15:06:06 -0500 Date: Mon, 23 Jan 2017 12:06:04 -0800 From: Lance Roy To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, jiangshanlai@gmail.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org, 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 v2 tip/core/rcu 2/3] srcu: Force full grace-period ordering Message-ID: <20170123120604.106ee644@gmail.com> In-Reply-To: <20170123191207.GG28085@linux.vnet.ibm.com> References: <20170115224128.GA20492@linux.vnet.ibm.com> <1484520155-21017-2-git-send-email-paulmck@linux.vnet.ibm.com> <20170123003829.3786251c@gmail.com> <20170123191207.GG28085@linux.vnet.ibm.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 23 Jan 2017 11:12:07 -0800 "Paul E. McKenney" wrote: > On Mon, Jan 23, 2017 at 12:38:29AM -0800, Lance Roy wrote: > > On Sun, 15 Jan 2017 14:42:34 -0800 > > "Paul E. McKenney" wrote: > > > > > @@ -413,6 +415,8 @@ static void __synchronize_srcu(struct srcu_struct *sp, > > > int trycount) > > > if (!done) > > > wait_for_completion(&rcu.completion); > > > + > > > + smp_mb(); /* Caller's later accesses after GP. */ > > > > I think that this memory barrier is only necessary when done == false, as > > otherwise srcu_advance_batches() should provide sufficient memory > > ordering. > > Let me make sure that I understand your rationale here. > > The idea is that although srcu_readers_active_idx_check() did a full > memory barrier, this might have happened on some other CPU, which > would not have provided ordering to the current CPU in the race case > where current CPU didn't actually sleep. (This can happen where the > current task is preempted, and then is resumed just as the grace period > completes.) > > Or are you concerned about some other sequence of events? Yes, the problem only occurs when the only full memory barrier is executed on a different CPU. > (I have moved the smp_mb() inside the "if (!done)" in the meantime.) Thanks. > > > @@ -587,6 +591,7 @@ static void srcu_invoke_callbacks(struct srcu_struct > > > *sp) int i; > > > struct rcu_head *head; > > > > > > + smp_mb(); /* Callback accesses after GP. */ > > > > Shouldn't srcu_advance_batches() have already run all necessary memory > > barriers? > > It does look that way: > > o process_srcu() is the only thing that invokes > srcu_invoke_callbacks(). > > o process_srcu() invokes srcu_advance_batches() immediately before > srcu_invoke_callbacks(), so any memory barriers invoked from > srcu_advance_batches() affect process_srcu() (unlike the earlier > example where srcu_advance_batches() might be executed in the > context of some other task). > > o srcu_advance_batches() unconditionally invokes try_check_zero(), > which in turn unconditionally invokes srcu_readers_active_idx_check(), > which in turn invokes smp_mb(). > > This smp_mb() precedes a successful check that all pre-existing > readers are done, otherwise srcu_advance_batches() won't have > returned (or won't have advanced the callbacks, which in turn > will prevent them from being invoked). > > I have removed this memory barrier and added a comment. > > And thank you for your review and comments!!! Thanks, Lance