From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966774AbdCXVQh (ORCPT ); Fri, 24 Mar 2017 17:16:37 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:41508 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752546AbdCXVQb (ORCPT ); Fri, 24 Mar 2017 17:16:31 -0400 Date: Fri, 24 Mar 2017 14:16:22 -0700 From: Darren Hart To: Peter Zijlstra Cc: tglx@linutronix.de, mingo@kernel.org, juri.lelli@arm.com, rostedt@goodmis.org, xlpang@redhat.com, bigeasy@linutronix.de, linux-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com, jdesfossez@efficios.com, bristot@redhat.com Subject: Re: [PATCH -v6 02/13] futex: Use smp_store_release() in mark_wake_futex() Message-ID: <20170324211622.GB18290@fury> References: <20170322103547.756091212@infradead.org> <20170322104151.604296452@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170322104151.604296452@infradead.org> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 22, 2017 at 11:35:49AM +0100, Peter Zijlstra wrote: > Since the futex_q can dissapear the instruction after assigning NULL, > this really should be a RELEASE barrier. That stops loads from hitting > dead memory too. > > Signed-off-by: Peter Zijlstra (Intel) I reviewed this carefully in the previous thread, confirming that despite the move to wake queues, spurious wakeups can still lead to the situration Peter describes. As such: Reviewed-by: Darren Hart (VMware) My only suggestion would be to clarify the language in the preceding comment to make that obvious, as well as clarify which plist_del it is referring to since it has been moved under the __unqueue_futex. I can do that as a follow-on though. > --- > kernel/futex.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -1288,8 +1288,7 @@ static void mark_wake_futex(struct wake_ > * memory barrier is required here to prevent the following > * store to lock_ptr from getting ahead of the plist_del. > */ > - smp_wmb(); > - q->lock_ptr = NULL; > + smp_store_release(&q->lock_ptr, NULL); > } > > static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *top_waiter, > > > -- Darren Hart VMware Open Source Technology Center