From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752150AbcJCPog (ORCPT ); Mon, 3 Oct 2016 11:44:36 -0400 Received: from merlin.infradead.org ([205.233.59.134]:59446 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751147AbcJCPoe (ORCPT ); Mon, 3 Oct 2016 11:44:34 -0400 Date: Mon, 3 Oct 2016 17:44:22 +0200 From: Peter Zijlstra To: Steven Rostedt Cc: mingo@kernel.org, tglx@linutronix.de, juri.lelli@arm.com, xlpang@redhat.com, bigeasy@linutronix.de, linux-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com, jdesfossez@efficios.com, bristot@redhat.com Subject: Re: [RFC][PATCH 4/4] futex: Rewrite FUTEX_UNLOCK_PI Message-ID: <20161003154422.GE3117@twins.programming.kicks-ass.net> References: <20161003091234.879763059@infradead.org> <20161003091847.704255067@infradead.org> <20161003113624.04f1f9f2@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161003113624.04f1f9f2@gandalf.local.home> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 03, 2016 at 11:36:24AM -0400, Steven Rostedt wrote: > > /* > > - * If current does not own the pi_state then the futex is > > - * inconsistent and user space fiddled with the futex value. > > + * Now that we hold wait_lock, no new waiters can happen on the > > + * rt_mutex and new owner is stable. Drop hb->lock. > > */ > > - if (pi_state->owner != current) > > - return -EINVAL; > > + spin_unlock(&hb->lock); > > > > Also, as Sebastian has said before, I believe this breaks rt's migrate > disable code. As migrate disable and migrate_enable are a nop if > preemption is disabled, thus if you hold a raw_spin_lock across a > spin_unlock() when the migrate enable will be a nop, and the > migrate_disable() will never stop. Its too long since I looked at that trainwreck, but yuck, that would make lock unlock order important :-( Now I think we could do something like so.. but I'm not entirely sure on the various lifetime rules here, its not overly documented. --- a/kernel/futex.c +++ b/kernel/futex.c @@ -1300,15 +1300,14 @@ static int wake_futex_pi(u32 __user *uad WAKE_Q(wake_q); int ret = 0; - raw_spin_lock_irq(&pi_state->pi_mutex.wait_lock); - WARN_ON_ONCE(!atomic_inc_not_zero(&pi_state->refcount)); /* - * Now that we hold wait_lock, no new waiters can happen on the - * rt_mutex and new owner is stable. Drop hb->lock. + * XXX */ spin_unlock(&hb->lock); + raw_spin_lock_irq(&pi_state->pi_mutex.wait_lock); + new_owner = rt_mutex_next_owner(&pi_state->pi_mutex); /*