From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753115AbdHBRrq (ORCPT ); Wed, 2 Aug 2017 13:47:46 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:49099 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753084AbdHBRro (ORCPT ); Wed, 2 Aug 2017 13:47:44 -0400 Date: Wed, 2 Aug 2017 10:47:40 -0700 From: "Paul E. McKenney" To: Peter Zijlstra Cc: Boqun Feng , linux-kernel@vger.kernel.org, Steven Rostedt , Krister Johansen , Paul Gortmaker , Thomas Gleixner , Ingo Molnar Subject: Re: [PATCH tip/sched/core] swait: Remove the lockless swait_active() check in swake_up*() Reply-To: paulmck@linux.vnet.ibm.com References: <20170730134737.8374-1-boqun.feng@gmail.com> <20170802160713.GA1352@linux.vnet.ibm.com> <20170802171223.7sfh4f5rbuplu4th@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170802171223.7sfh4f5rbuplu4th@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17080217-0040-0000-0000-0000038981AB X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007472; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000216; SDB=6.00896579; UDB=6.00448531; IPR=6.00676766; BA=6.00005506; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016500; XFM=3.00000015; UTC=2017-08-02 17:47:42 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17080217-0041-0000-0000-0000077DAC86 Message-Id: <20170802174740.GB3730@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-02_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-1706020000 definitions=main-1708020287 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 02, 2017 at 07:12:23PM +0200, Peter Zijlstra wrote: > On Wed, Aug 02, 2017 at 09:07:13AM -0700, Paul E. McKenney wrote: > > On Sun, Jul 30, 2017 at 09:47:35PM +0800, Boqun Feng wrote: > > > Steven Rostedt reported a potential race in RCU core because of > > > swake_up(): > > > > > > CPU0 CPU1 > > > ---- ---- > > > __call_rcu_core() { > > > > > > spin_lock(rnp_root) > > > need_wake = __rcu_start_gp() { > > > rcu_start_gp_advanced() { > > > gp_flags = FLAG_INIT > > > } > > > } > > > > > > rcu_gp_kthread() { > > > swait_event_interruptible(wq, > > > gp_flags & FLAG_INIT) { > > > spin_lock(q->lock) > > > > > > *fetch wq->task_list here! * > > > > > > list_add(wq->task_list, q->task_list) > > > spin_unlock(q->lock); > > > > > > *fetch old value of gp_flags here * > > > > > > spin_unlock(rnp_root) > > > > > > rcu_gp_kthread_wake() { > > > swake_up(wq) { > > > swait_active(wq) { > > > list_empty(wq->task_list) > > > > > > } * return false * > > > > > > if (condition) * false * > > > schedule(); > > > > > > In this case, a wakeup is missed, which could cause the rcu_gp_kthread > > > waits for a long time. > > > > > > The reason of this is that we do a lockless swait_active() check in > > > swake_up(). To fix this, we can either 1) add a smp_mb() in swake_up() > > > before swait_active() to provide the proper order or 2) simply remove > > > the swait_active() in swake_up(). > > > > > > The solution 2 not only fixes this problem but also keeps the swait and > > > wait API as close as possible, as wake_up() doesn't provide a full > > > barrier and doesn't do a lockless check of the wait queue either. > > > Moreover, there are users already using swait_active() to do their quick > > > checks for the wait queues, so it make less sense that swake_up() and > > > swake_up_all() do this on their own. > > > > > > This patch then removes the lockless swait_active() check in swake_up() > > > and swake_up_all(). > > > > > > Reported-by: Steven Rostedt > > > Signed-off-by: Boqun Feng > > > Acked-by: Paul E. McKenney > > > Tested-by: Paul E. McKenney > > > > Hearing no objections but not hearing anything else, either, I have > > queued this for v4.14. If someone else would rather queue it, please > > let me know. > > I have it too. Lets see who can get it into -tip first :-) Heh! We could use it as a test of Ingo's handling of multiple identical patches. ;-) Thanx, Paul