From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934222AbdDEV7O (ORCPT ); Wed, 5 Apr 2017 17:59:14 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:49951 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751881AbdDEV7H (ORCPT ); Wed, 5 Apr 2017 17:59:07 -0400 Date: Wed, 5 Apr 2017 14:58:58 -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 07/13] futex: Rework inconsistent rt_mutex/futex_q state Message-ID: <20170405215858.GC13494@fury> References: <20170322103547.756091212@infradead.org> <20170322104151.850383690@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170322104151.850383690@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:54AM +0100, Peter Zijlstra wrote: > There is a weird state in the futex_unlock_pi() path when it > interleaves with a concurrent futex_lock_pi() at the point where it > drops hb->lock. > > In this case, it can happen that the rt_mutex wait_list and the > futex_q disagree on pending waiters, in particular rt_mutex will find > no pending waiters where futex_q thinks there are. > > In this case the rt_mutex unlock code cannot assign an owner. > > What the current code does in this case is use the futex_q waiter that > got us here; however when the rt_mutex_timed_futex_lock() has already > failed; this leaves things in a weird state, resulting in much > head-aches in fixup_owner(). > > Simplify all this by changing wake_futex_pi() to return -EAGAIN when > this situation occurs. This then gives the futex_lock_pi() code the > opportunity to continue and the retried futex_unlock_pi() will now > observe a coherent state. > > The only problem is that this breaks RT timeliness guarantees. That > is, consider the following scenario: > > T1 and T2 are both pinned to CPU0. prio(T2) > prio(T1) > > CPU0 > > T1 > lock_pi() > queue_me() <- Waiter is visible > > preemption > > T2 > unlock_pi() > loops with -EAGAIN forever > > Which is undesirable for PI primitives. Future patches will rectify > this. For now we want to get rid of the fixup magic. Errrrm... OK... I don't like the idea of having this broken after this commit, but until I internalize the remaining 5 (that number has never seemed quite so dauntingly large before... 5...) I can't comment on the alternative. I suppose having it documented in the commit log means anyone backporting only up to this point gets what they deserve. A good patch *removing* code from futex.c is always nice though ! -- Darren Hart VMware Open Source Technology Center