From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix mutex->owner inspection race under DEBUG_MUTEXES Date: Wed, 07 Jan 2015 14:12:14 +0200 Message-ID: <87r3v6zt5t.fsf@intel.com> References: <1420192030-10206-1-git-send-email-chris@chris-wilson.co.uk> <20150105105123.GV2711@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain Return-path: In-Reply-To: <20150105105123.GV2711@phenom.ffwll.local> Sender: stable-owner@vger.kernel.org To: Daniel Vetter , Chris Wilson Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Mon, 05 Jan 2015, Daniel Vetter wrote: > On Fri, Jan 02, 2015 at 09:47:10AM +0000, Chris Wilson wrote: >> If CONFIG_DEBUG_MUTEXES is set, the mutex->owner field is only cleared >> if the mutex debugging is enabled which introduces a race in our >> mutex_is_locked_by() - i.e. we may inspect the old owner value before it >> is acquired by the new task. >> >> This is the root cause of this error: >> >> diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c >> index 5cf6731..3ef3736 100644 >> --- a/kernel/locking/mutex-debug.c >> +++ b/kernel/locking/mutex-debug.c >> @@ -80,13 +80,13 @@ void debug_mutex_unlock(struct mutex *lock) >> DEBUG_LOCKS_WARN_ON(lock->owner != current); >> >> DEBUG_LOCKS_WARN_ON(!lock->wait_list.prev && !lock->wait_list.next); >> - mutex_clear_owner(lock); >> } >> >> /* >> * __mutex_slowpath_needs_to_unlock() is explicitly 0 for debug >> * mutexes so that we can do it here after we've verified state. >> */ >> + mutex_clear_owner(lock); >> atomic_set(&lock->count, 1); >> } >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87955 >> Signed-off-by: Chris Wilson >> Cc: stable@vger.kernel.org > > Reviewed-by: Daniel Vetter Pushed to drm-intel-fixes, thanks for the patch and review. BR, Jani. > >> --- >> drivers/gpu/drm/i915/i915_gem.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index ad55b06a3cb1..4c14db9587a6 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -5024,7 +5024,7 @@ static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task) >> if (!mutex_is_locked(mutex)) >> return false; >> >> -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_MUTEXES) >> +#if defined(CONFIG_SMP) && !defined(CONFIG_DEBUG_MUTEXES) >> return mutex->owner == task; >> #else >> /* Since UP may be pre-empted, we cannot assume that we own the lock */ >> -- >> 2.1.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center