Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933163AbdGKOhv (ORCPT ); Tue, 11 Jul 2017 10:37:51 -0400 Received: from mail-pg0-f47.google.com ([74.125.83.47]:36438 "EHLO mail-pg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933146AbdGKOho (ORCPT ); Tue, 11 Jul 2017 10:37:44 -0400 Subject: Re: [PATCH v2 2/3] rtmutex: update rt-mutex To: peterz@infradead.org, mingo@redhat.com, corbet@lwn.net, "open list:LOCKING PRIMITIVES" , "open list:DOCUMENTATION" Cc: Steven Rostedt , Sebastian Siewior , Mathieu Poirier , Juri Lelli , Thomas Gleixner References: <1499395922-542-1-git-send-email-alex.shi@linaro.org> <1499395922-542-2-git-send-email-alex.shi@linaro.org> From: Alex Shi Message-ID: <3771f154-c6cd-215a-3e59-2a94b10bb074@linaro.org> Date: Tue, 11 Jul 2017 22:37:39 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1499395922-542-2-git-send-email-alex.shi@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5600 Lines: 112 Any comments for this patch? or time to pick it up? Thanks Alex On 07/07/2017 10:52 AM, Alex Shi wrote: > The rtmutex remove a pending owner bit in in rt_mutex::owner, in > commit 8161239a8bcc ("rtmutex: Simplify PI algorithm and make highest prio task get lock") > But the document was changed accordingly. Updating it to a meaningful > state. > > BTW, as 'Steven Rostedt' mentioned: > There is still technically a "Pending Owner", it's just not called > that anymore. The pending owner happens to be the top_waiter of a lock > that has no owner and has been woken up to grab the lock. > > Signed-off-by: Alex Shi > Cc: Steven Rostedt > Cc: Sebastian Siewior > Cc: Mathieu Poirier > Cc: Juri Lelli > Cc: Thomas Gleixner > To: linux-doc@vger.kernel.org > To: linux-kernel@vger.kernel.org > To: Jonathan Corbet > To: Ingo Molnar > To: Peter Zijlstra > --- > Documentation/locking/rt-mutex.txt | 58 +++++++++++++++++--------------------- > 1 file changed, 26 insertions(+), 32 deletions(-) > > diff --git a/Documentation/locking/rt-mutex.txt b/Documentation/locking/rt-mutex.txt > index 243393d..35793e0 100644 > --- a/Documentation/locking/rt-mutex.txt > +++ b/Documentation/locking/rt-mutex.txt > @@ -28,14 +28,13 @@ magic bullet for poorly designed applications, but it allows > well-designed applications to use userspace locks in critical parts of > an high priority thread, without losing determinism. > > -The enqueueing of the waiters into the rtmutex waiter list is done in > +The enqueueing of the waiters into the rtmutex waiter tree is done in > priority order. For same priorities FIFO order is chosen. For each > rtmutex, only the top priority waiter is enqueued into the owner's > -priority waiters list. This list too queues in priority order. Whenever > +priority waiters tree. This tree too queues in priority order. Whenever > the top priority waiter of a task changes (for example it timed out or > -got a signal), the priority of the owner task is readjusted. [The > -priority enqueueing is handled by "plists", see include/linux/plist.h > -for more details.] > +got a signal), the priority of the owner task is readjusted. The > +priority enqueueing is handled by "pi_waiters". > > RT-mutexes are optimized for fastpath operations and have no internal > locking overhead when locking an uncontended mutex or unlocking a mutex > @@ -46,34 +45,29 @@ is used] > The state of the rt-mutex is tracked via the owner field of the rt-mutex > structure: > > -rt_mutex->owner holds the task_struct pointer of the owner. Bit 0 and 1 > -are used to keep track of the "owner is pending" and "rtmutex has > -waiters" state. > +lock->owner holds the task_struct pointer of the owner. Bit 0 is used to > +keep track of the "lock has waiters" state. > > - owner bit1 bit0 > - NULL 0 0 mutex is free (fast acquire possible) > - NULL 0 1 invalid state > - NULL 1 0 Transitional state* > - NULL 1 1 invalid state > - taskpointer 0 0 mutex is held (fast release possible) > - taskpointer 0 1 task is pending owner > - taskpointer 1 0 mutex is held and has waiters > - taskpointer 1 1 task is pending owner and mutex has waiters > + owner bit0 > + NULL 0 lock is free (fast acquire possible) > + NULL 1 lock is free and has waiters and the top waiter > + is going to take the lock* > + taskpointer 0 lock is held (fast release possible) > + taskpointer 1 lock is held and has waiters** > > -Pending-ownership handling is a performance optimization: > -pending-ownership is assigned to the first (highest priority) waiter of > -the mutex, when the mutex is released. The thread is woken up and once > -it starts executing it can acquire the mutex. Until the mutex is taken > -by it (bit 0 is cleared) a competing higher priority thread can "steal" > -the mutex which puts the woken up thread back on the waiters list. > +The fast atomic compare exchange based acquire and release is only > +possible when bit 0 of lock->owner is 0. > > -The pending-ownership optimization is especially important for the > -uninterrupted workflow of high-prio tasks which repeatedly > -takes/releases locks that have lower-prio waiters. Without this > -optimization the higher-prio thread would ping-pong to the lower-prio > -task [because at unlock time we always assign a new owner]. > +(*) It also can be a transitional state when grabbing the lock > +with ->wait_lock is held. To prevent any fast path cmpxchg to the lock, > +we need to set the bit0 before looking at the lock, and the owner may be > +NULL in this small time, hence this can be a transitional state. > > -(*) The "mutex has waiters" bit gets set to take the lock. If the lock > -doesn't already have an owner, this bit is quickly cleared if there are > -no waiters. So this is a transitional state to synchronize with looking > -at the owner field of the mutex and the mutex owner releasing the lock. > +(**) There is a small time when bit 0 is set but there are no > +waiters. This can happen when grabbing the lock in the slow path. > +To prevent a cmpxchg of the owner releasing the lock, we need to > +set this bit before looking at the lock. > + > +BTW, there is still technically a "Pending Owner", it's just not called > +that anymore. The pending owner happens to be the top_waiter of a lock > +that has no owner and has been woken up to grab the lock. >