From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753181AbdDFRnF (ORCPT ); Thu, 6 Apr 2017 13:43:05 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:55012 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753736AbdDFRm4 (ORCPT ); Thu, 6 Apr 2017 13:42:56 -0400 Date: Thu, 6 Apr 2017 10:42:48 -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 08/13] futex: Pull rt_mutex_futex_unlock() out from under hb->lock Message-ID: <20170406174248.GD7030@fury> References: <20170322103547.756091212@infradead.org> <20170322104151.900002056@infradead.org> <20170405235225.GD13494@fury> <20170406124248.i7ibgne76yojnizh@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170406124248.i7ibgne76yojnizh@hirez.programming.kicks-ass.net> 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 Thu, Apr 06, 2017 at 02:42:48PM +0200, Peter Zijlstra wrote: > On Wed, Apr 05, 2017 at 04:52:25PM -0700, Darren Hart wrote: > > On Wed, Mar 22, 2017 at 11:35:55AM +0100, Peter Zijlstra wrote: > > > There's a number of 'interesting' problems, all caused by holding > > > hb->lock while doing the rt_mutex_unlock() equivalient. > > > > > > Notably: > > > > > > - a PI inversion on hb->lock; and, > > > > > > - a DL crash because of pointer instability. > > > > A DL crash? What is this? Can you elaborate a bit? > > See here: > > https://lkml.kernel.org/r/20170323145606.480214279@infradead.org Ah, DeadLine, thanks. ... > > > newval = (uval & FUTEX_OWNER_DIED) | newtid; > > > > > > if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval)) > > > @@ -2345,6 +2343,10 @@ static int fixup_owner(u32 __user *uaddr > > > /* > > > * Got the lock. We might not be the anticipated owner if we > > > * did a lock-steal - fix up the PI-state in that case: > > > + * > > > + * We can safely read pi_state->owner without holding wait_lock > > > + * because we now own the rt_mutex, only the owner will attempt > > > + * to change it. > > > > This seems to contradict the Serialization and lifetime rules: > > > > + * pi_mutex->wait_lock: > > + * > > + * {uval, pi_state} > > + * > > + * (and pi_mutex 'obviously') > > > > It would seem that simply holding pi_mutex is sufficient for serialization on > > pi_state->owner then. > > Not a contradiction; just a very specific special case. If current is > the owner of a lock, said owner will not be going anywhere. OK. > > > + > > > + /* > > > + * Grab a reference on the pi_state and drop hb->lock. > > > + * > > > + * The reference ensures pi_state lives, dropping the hb->lock > > > + * is tricky.. wake_futex_pi() will take rt_mutex::wait_lock to > > > + * close the races against futex_lock_pi(), but in case of > > > + * _any_ fail we'll abort and retry the whole deal. > > > > s/fail/failure/ > > I don't think that survives the patch-set. That is, I cannot find it in > the current code. Ah right, intermediate documentation. Kudos for that! :-) -- Darren Hart VMware Open Source Technology Center