From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753080AbdDJJIa (ORCPT ); Mon, 10 Apr 2017 05:08:30 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:40306 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751442AbdDJJI3 (ORCPT ); Mon, 10 Apr 2017 05:08:29 -0400 Date: Mon, 10 Apr 2017 11:08:18 +0200 From: Peter Zijlstra To: Darren Hart 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 13/13] futex: futex_lock_pi() vs PREEMPT_RT_FULL Message-ID: <20170410090818.ztb2fxzb6oy3ee7o@hirez.programming.kicks-ass.net> References: <20170322103547.756091212@infradead.org> <20170322104152.161341537@infradead.org> <20170408022610.GC16143@fury> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170408022610.GC16143@fury> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 07, 2017 at 07:26:10PM -0700, Darren Hart wrote: > Peter, I presume there will be a v7 with the u32 change and hopefully a couple > text updates? Well, tglx already committed these here patches, so no -v7. What I can do however is do a follow up patch that fixes some of the in-code things you mentioned. Something like the below is what I had lying about from your earlier emails; I've not looked to see if there's anything else from your later emails. --- Subject: futex: Small misc fixes.. From: Peter Zijlstra Date: Fri Apr 7 09:04:07 CEST 2017 Feedback from Darren's review. Reported-by: Darren Hart (VMWare) Signed-off-by: Peter Zijlstra (Intel) --- kernel/futex.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) --- a/kernel/futex.c +++ b/kernel/futex.c @@ -1025,7 +1025,8 @@ static int attach_to_pi_state(u32 __user struct futex_pi_state **ps) { pid_t pid = uval & FUTEX_TID_MASK; - int ret, uval2; + u32 uval2; + int ret; /* * Userspace might have messed up non-PI and PI futexes [3] @@ -1441,6 +1442,11 @@ static int wake_futex_pi(u32 __user *uad if (ret) goto out_unlock; + /* + * This is a point of no return; once we modify the uval there is no + * going back and subsequent operations must not fail. + */ + raw_spin_lock(&pi_state->owner->pi_lock); WARN_ON(list_empty(&pi_state->list)); list_del_init(&pi_state->list); @@ -1452,9 +1458,6 @@ static int wake_futex_pi(u32 __user *uad pi_state->owner = new_owner; raw_spin_unlock(&new_owner->pi_lock); - /* - * We've updated the uservalue, this unlock cannot fail. - */ postunlock = __rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q); out_unlock: