From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760051AbcHaLck (ORCPT ); Wed, 31 Aug 2016 07:32:40 -0400 Received: from gate.crashing.org ([63.228.1.57]:34530 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759300AbcHaLca (ORCPT ); Wed, 31 Aug 2016 07:32:30 -0400 Message-ID: <1472640944.2388.82.camel@kernel.crashing.org> Subject: Re: [RFC][PATCH] Fix a race between rwsem and the scheduler From: Benjamin Herrenschmidt To: Peter Zijlstra Cc: Oleg Nesterov , Balbir Singh , LKML , Nicholas Piggin Date: Wed, 31 Aug 2016 20:55:44 +1000 In-Reply-To: <20160831072041.GA10138@twins.programming.kicks-ass.net> References: <4050f2ce-1aee-d2aa-39e3-36e995b56252@gmail.com> <20160830121937.GQ10138@twins.programming.kicks-ass.net> <20160830130426.GA17795@redhat.com> <1472592301.2388.37.camel@kernel.crashing.org> <20160831072041.GA10138@twins.programming.kicks-ass.net> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 (3.20.5-1.fc24) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-08-31 at 09:20 +0200, Peter Zijlstra wrote: > On Wed, Aug 31, 2016 at 07:25:01AM +1000, Benjamin Herrenschmidt wrote: > > > > On Tue, 2016-08-30 at 15:04 +0200, Oleg Nesterov wrote: > > > > > > > > > Confused... how this connects to UNLOCK+LOCK on rq->lock? A LOAD can > > > leak into the critical section. > > > > > > But context switch should imply mb() we can rely on? > > > > Between setting of ->on_rq and returning to the task so it can > > change its state back to [UN]INTERRUPTIBLE, there will be at least one > > write barrier (spin unlock of the rq), > > spin-unlock is _not_ a write barrier, its a RELEASE barrier, and is not > sufficient for this. Ah yes well it's an lwsync so it's a wmb for us :-) . > > possibly even a full barrier > > (context switch). The write barrier is enough so I didn't dig to make > > sure we always context switch in the scenario we're looking at but I > > think we do. > > There is enough, you just need to pair the RELEASE with an ACQUIRE to > get a full load-store barrier. Right so I *think* there will be at least the release of the rq_lock by the IPI followed by schedule itself taking and releasing it again, but I can't vouch for it. As I said, I didn't dig deeper on that side of things as for us a spin_unlock is a write barrier and for the write side that concerns me here it's sufficient ;-) It's the read side that has a problem. That said you may want to investigate more to make sure there is no way out of schedule where that spin_unlock is the only thing between setting on_rq and coming out (which leads to setting the task state). I suspect there will be at least one more re-aquisition & release of the rq lock but I may be wrong. Cheers, Ben.