From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756320AbdCGVbd (ORCPT ); Tue, 7 Mar 2017 16:31:33 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:59999 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751113AbdCGVb0 (ORCPT ); Tue, 7 Mar 2017 16:31:26 -0500 Date: Tue, 7 Mar 2017 07:27:15 -0800 From: "Paul E. McKenney" To: Dmitry Vyukov Cc: Boqun Feng , josh@joshtriplett.org, Steven Rostedt , Mathieu Desnoyers , jiangshanlai@gmail.com, LKML , syzkaller Subject: Re: rcu: WARNING in rcu_seq_end Reply-To: paulmck@linux.vnet.ibm.com References: <20170304204052.GC30506@linux.vnet.ibm.com> <20170305184736.GD30506@linux.vnet.ibm.com> <20170306100741.GJ30506@linux.vnet.ibm.com> <20170306230800.GK30506@linux.vnet.ibm.com> <20170307142740.uh2nnaw44albn3t2@tardis> 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-GCONF: 00 x-cbid: 17030715-0044-0000-0000-000002BD1F5F X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006739; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000206; SDB=6.00831264; UDB=6.00407818; IPR=6.00608855; BA=6.00005194; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014544; XFM=3.00000012; UTC=2017-03-07 15:27:18 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17030715-0045-0000-0000-000006EB286C Message-Id: <20170307152715.GM30506@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-07_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703070127 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 07, 2017 at 03:43:42PM +0100, Dmitry Vyukov wrote: > On Tue, Mar 7, 2017 at 3:27 PM, Boqun Feng wrote: > > On Tue, Mar 07, 2017 at 08:05:19AM +0100, Dmitry Vyukov wrote: > > [...] > >> >> > >> >> What is that mutex? And what locks/unlocks provide synchronization? I > >> >> see that one uses exp_mutex and another -- exp_wake_mutex. > >> > > >> > Both of them. > >> > > >> > ->exp_mutex is acquired by the task requesting the grace period, and > >> > the counter's first increment is done by that task under that mutex. > >> > This task then schedules a workqueue, which drives forward the grace > >> > period. Upon grace-period completion, the workqueue handler does the > >> > second increment (the one that your patch addressed). The workqueue > >> > handler then acquires ->exp_wake_mutex and wakes the task that holds > >> > ->exp_mutex (along with all other tasks waiting for this grace period), > >> > and that task releases ->exp_mutex, which allows the next grace period to > >> > start (and the first increment for that next grace period to be carried > >> > out under that lock). The workqueue handler releases ->exp_wake_mutex > >> > after finishing its wakeups. > >> > >> Then we need the following for the case when task requesting the grace > >> period does not block, right? > > > > Won't be necessary I think, as the smp_mb() in rcu_seq_end() and the > > smp_mb__before_atomic() in sync_exp_work_done() already provide the > > required ordering, no? > > smp_mb() is probably fine, but smp_mb__before_atomic() is release not > acquire. If we want to play that game, then I guess we also need > smp_mb__after_atomic() there. But it would be way easier to understand > what's happens there and prove that it's correct, if we use > store_release/load_acquire. Fair point, how about the following? Thanx, Paul ------------------------------------------------------------------------ commit 6fd8074f1976596898e39f5b7ea1755652533906 Author: Paul E. McKenney Date: Tue Mar 7 07:21:23 2017 -0800 rcu: Add smp_mb__after_atomic() to sync_exp_work_done() The sync_exp_work_done() function needs to fully order the counter-check operation against anything happening after the corresponding grace period. This is a theoretical bug, as all current architectures either provide full ordering for atomic operation on the one hand or implement, however, a little future-proofing is a good thing. This commit therefore adds smp_mb__after_atomic() after the atomic_long_inc() in sync_exp_work_done(). Reported-by: Dmitry Vyukov Signed-off-by: Paul E. McKenney diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h index 027e123d93c7..652071abd9b4 100644 --- a/kernel/rcu/tree_exp.h +++ b/kernel/rcu/tree_exp.h @@ -247,6 +247,7 @@ static bool sync_exp_work_done(struct rcu_state *rsp, atomic_long_t *stat, /* Ensure test happens before caller kfree(). */ smp_mb__before_atomic(); /* ^^^ */ atomic_long_inc(stat); + smp_mb__after_atomic(); /* ^^^ */ return true; } return false;