From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752954AbbCJNEL (ORCPT ); Tue, 10 Mar 2015 09:04:11 -0400 Received: from ou.quest-ce.net ([195.154.187.82]:55750 "EHLO ou.quest-ce.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752708AbbCJNEJ (ORCPT ); Tue, 10 Mar 2015 09:04:09 -0400 Message-ID: <1425992639.3991.11.camel@opteya.com> From: Yann Droneaud To: Matthias Bonne Cc: kernelnewbies@kernelnewbies.org, linux-kernel@vger.kernel.org, Peter Zijlstra , Ingo Molnar Date: Tue, 10 Mar 2015 14:03:59 +0100 In-Reply-To: <54F64E10.7050801@gmail.com> References: <54F64E10.7050801@gmail.com> Organization: OPTEYA Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 (3.12.11-1.fc21) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-SA-Exim-Connect-IP: 37.161.212.107 X-SA-Exim-Mail-From: ydroneaud@opteya.com Subject: Re: Question on mutex code X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on ou.quest-ce.net) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Le mercredi 04 mars 2015 à 02:13 +0200, Matthias Bonne a écrit : > I am trying to understand how mutexes work in the kernel, and I think > there might be a race between mutex_trylock() and mutex_unlock(). More > specifically, the race is between the functions > __mutex_trylock_slowpath and __mutex_unlock_common_slowpath (both > defined in kernel/locking/mutex.c). > > Consider the following sequence of events: > > 0. Suppose a mutex is locked by task A and has no waiters. > > 1. Task B calls mutex_trylock(). > > 2. mutex_trylock() calls the architecture-specific > __mutex_fastpath_trylock(), with __mutex_trylock_slowpath() as > fail_fn. > > 3. According to the description of __mutex_fastpath_trylock() (for > example in include/asm-generic/mutex-dec.h), "if the architecture > has no effective trylock variant, it should call the fail_fn > spinlock-based trylock variant unconditionally". So > __mutex_fastpath_trylock() may now call __mutex_trylock_slowpath(). > > 4. Task A releases the mutex. > > 5. Task B, in __mutex_trylock_slowpath, executes: > > /* No need to trylock if the mutex is locked. */ > if (mutex_is_locked(lock)) > return 0; > > Since the mutex is no longer locked, the function continues. > > 6. Task C, which runs on a different cpu than task B, locks the mutex > again. > > 7. Task B, in __mutex_trylock_slowpath(), continues: > > spin_lock_mutex(&lock->wait_lock, flags); > > prev = atomic_xchg(&lock->count, -1); > if (likely(prev == 1)) { > mutex_set_owner(lock); > mutex_acquire(&lock->dep_map, 0, 1, _RET_IP_); > } > > At this point task B holds mutex->wait_lock, prev is 0 (because there > are no waiters other than task B, so the count was 0) and the mutex > count is set to -1. > > 5. Task C calls mutex_unlock() to unlock the mutex. > > 6. mutex_unlock() calls the architecture-specific function > __mutex_fastpath_unlock(), which fails (because the mutex count is > -1), so it now calls __mutex_unlock_slowpath(), which calls > __mutex_unlock_common_slowpath(). > > 7. __mutex_unlock_common_slowpath() sets the mutex count to 1 > unconditionally, before spinning on mutex->wait_lock. > > 8. Task B, in __mutex_trylock_slowpath, continues: > > /* Set it back to 0 if there are no waiters: */ > if (likely(list_empty(&lock->wait_list))) > atomic_set(&lock->count, 0); > > spin_unlock_mutex(&lock->wait_lock, flags); > > return prev == 1; > > mutex->wait_list is still empty, so the code sets the mutex count to > zero (which means the mutex is locked), releases mutex->wait_lock, > and returns 0 (which means that the mutex is locked by someone else, > and cannot be acquired). > > 9. Task C, in __mutex_unlock_common_slowpath, acquires > mutex->wait_lock, unlocks it immediately (because there are no > waiters to wake up) and returns. > > The end result is that the mutex count is 0 (locked), although the > owner has just released it, and nobody else is holding the mutex. So it > can no longer be acquired by anyone. > > Am I missing something that prevents the above scenario from happening? > If not, should I post a patch that fixes it to LKML? Or is it > considered too "theoretical" and cannot happen in practice? > I haven't looked at your explanations, you should have come with a reproductible test case to demonstrate the issue (involving slowing down one CPU ?). Anyway, such deep knowledge on the mutex implementation has to be found on lkml. Regards. -- Yann Droneaud OPTEYA