From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754796AbaIDRDT (ORCPT ); Thu, 4 Sep 2014 13:03:19 -0400 Received: from e35.co.us.ibm.com ([32.97.110.153]:38733 "EHLO e35.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751673AbaIDRDR (ORCPT ); Thu, 4 Sep 2014 13:03:17 -0400 Date: Thu, 4 Sep 2014 10:03:08 -0700 From: "Paul E. McKenney" To: Peter Zijlstra Cc: Oleg Nesterov , Kautuk Consul , Ingo Molnar , Andrew Morton , Michal Hocko , David Rientjes , Ionut Alexa , Guillaume Morin , linux-kernel@vger.kernel.org, Kirill Tkhai Subject: Re: [PATCH 1/1] do_exit(): Solve possibility of BUG() due to race with try_to_wake_up() Message-ID: <20140904170308.GH5001@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20140901153935.GQ27892@worktop.ger.corp.intel.com> <20140901175851.GA15210@redhat.com> <20140901190931.GD5806@worktop.ger.corp.intel.com> <20140902155208.GA28668@redhat.com> <20140902164714.GA17033@redhat.com> <20140902173910.GF27892@worktop.ger.corp.intel.com> <20140903133640.GA25439@redhat.com> <20140903144450.GB7083@twins.programming.kicks-ass.net> <20140903151848.GA6312@redhat.com> <20140904071526.GI3190@worktop.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140904071526.GI3190@worktop.ger.corp.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14090417-6688-0000-0000-00000483EAF7 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 04, 2014 at 09:15:26AM +0200, Peter Zijlstra wrote: > On Wed, Sep 03, 2014 at 05:18:48PM +0200, Oleg Nesterov wrote: > > On 09/03, Peter Zijlstra wrote: > > > > > > On Wed, Sep 03, 2014 at 03:36:40PM +0200, Oleg Nesterov wrote: > > > > > > > > // Ensure that the previous __set_current_state(RUNNING) can't > > > > // leak after spin_unlock_wait() > > > > smp_mb(); > > > > spin_unlock_wait(); > > > > // Another mb to ensure this too can't be reordered with unlock_wait > > > > set_current_state(TASK_DEAD); > > > > > > > > What do you think looks better? > > > > > > spin_unlock_wait() would be a control dependency right? Therefore that > > > store could not creep up anyhow. > > > > Hmm. indeed, thanks! This probably means that task_work_run() can use > > rmb() instead of mb(). > > > > What I can't understand is do we still need a compiler barrier or not. > > Probably "in theory yes" ? > > Yes, this is where I'm forever in doubt as well. The worry is the > compiler reordering things, but I'm not sure how it would do that in > this case, then again, I've been shown to not be creative enough in > these cases many times before. > > Paul might know, he's had much more exposure to compiler people. Well, if we are talking about the code sequence above, spin_unlock_wait() does reads followed by a conditional. And set_current_state() does a write. The one thing that might be missing is that for this to work is that Alpha might need an ACCESS_ONCE() to avoid read reordering. (Yes, ACCESS_ONCE() is required for the other architectures to meet the letter of the law in memory-barriers.txt, but if you know that your particular architecture and compiler won't mess you up, you have more freedom in your arch-specific code.) Thanx, Paul