Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753382AbcLFPGl (ORCPT ); Tue, 6 Dec 2016 10:06:41 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:49020 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbcLFPGk (ORCPT ); Tue, 6 Dec 2016 10:06:40 -0500 Date: Tue, 6 Dec 2016 16:06:20 +0100 From: Peter Zijlstra To: Nicolai =?iso-8859-1?Q?H=E4hnle?= Cc: linux-kernel@vger.kernel.org, Nicolai =?iso-8859-1?Q?H=E4hnle?= , Ingo Molnar , Maarten Lankhorst , Daniel Vetter , Chris Wilson , dri-devel@lists.freedesktop.org Subject: Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Message-ID: <20161206150620.GT3045@worktop.programming.kicks-ass.net> References: <1480601214-26583-1-git-send-email-nhaehnle@gmail.com> <1480601214-26583-3-git-send-email-nhaehnle@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1480601214-26583-3-git-send-email-nhaehnle@gmail.com> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1660 Lines: 50 On Thu, Dec 01, 2016 at 03:06:45PM +0100, Nicolai H?hnle wrote: > +++ b/kernel/locking/mutex.c > @@ -350,7 +350,8 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock, > * access and not reliable. > */ > static noinline > -bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner) > +bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner, > + bool use_ww_ctx, struct ww_acquire_ctx *ww_ctx) > { > bool ret = true; > > @@ -373,6 +374,28 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner) > break; > } > > + if (use_ww_ctx && ww_ctx->acquired > 0) { > + struct ww_mutex *ww; > + > + ww = container_of(lock, struct ww_mutex, base); > + > + /* > + * If ww->ctx is set the contents are undefined, only > + * by acquiring wait_lock there is a guarantee that > + * they are not invalid when reading. > + * > + * As such, when deadlock detection needs to be > + * performed the optimistic spinning cannot be done. > + * > + * Check this in every inner iteration because we may > + * be racing against another thread's ww_mutex_lock. > + */ > + if (READ_ONCE(ww->ctx)) { > + ret = false; > + break; > + } > + } > + > cpu_relax(); > } > rcu_read_unlock(); Aside from the valid question about mutex_can_spin_on_owner() there's another 'problem' here, mutex_spin_on_owner() is marked noinline, so all the use_ww_ctx stuff doesn't 'work' here. As is, I think we're missing an __always_inline on mutex_optimistic_spin, I'll have to go see what that does for code generation, but given both @use_ww_ctx and @waiter there that makes sense.