Messages in this thread | | | Subject | Re: [PATCH -tip v2 1/2] locking/rtmutex: Support spin on owner | From | Davidlohr Bueso <> | Date | Wed, 01 Jul 2015 17:16:38 -0700 |
| |
On Thu, 2015-07-02 at 00:27 +0200, Thomas Gleixner wrote: > On Wed, 1 Jul 2015, Davidlohr Bueso wrote: > > > Similar to what we have in other locks, particularly regular mutexes, the > > idea is that as long as the owner is running, there is a fair chance it'll > > release the lock soon, and thus a task trying to acquire the rtmutex will > > better off spinning instead of blocking immediately after the fastpath. > > Conditions to stop spinning and enter the slowpath are simple: > > > > (1) Upon need_resched() > > (2) Current lock owner blocks > > > > Because rtmutexes track the lock owner atomically, we can extend the fastpath > > to continue polling on the lock owner via cmpxchg(lock->owner, NULL, current). > > > > However, this is a conservative approach, such that if there are any waiters > > in-line, we stop spinning and immediately take the traditional slowpath. This > > allows priority boosting to take precedence over spinning, as otherwise we > > could starve a higher priority queued-up task (ie: top waiter) if spinners > > constantly steal the lock. > > I'm a bit wary about the whole approach. In the RT tree we spin AFTER > we've enqueued the waiter and run priority boosting. While I can see > the charm of your approach, i.e. avoiding the prio boost dance for the > simple case, this can introduce larger latencies. > > T1 (prio = 0) T2 (prio = 50) > lock(RTM); > lock(RTM); > spin() > -->preemption > T3 (prio = 10) leave spin, because owner is not on cpu > > enqueue(); > boost(); > schedule(); > -->preemption > T1 (prio = 50) > > So we trade two extra context switches in the worst case for an > enhancement of performance in the normal case. I cannot quantify the > impact of this, but we really need to evaluate that proper before > going there.
This is a very good point.
My first thought is that for a general purpose OS, the extra latency is probably ok -- if you _really_ rely on rt characteristics enough to care about this, you should be using the rt-patchset to begin with, methinks. Aside from the non-blocking performance benefits of spinning, avoiding the wait_lock (and the pi_lock in the case of doing the spinning AFTER the boosting) can ease a lot of the lock contention.
> Aside of that, if the lock is really contended, then you force all > spinners off the cpu, if one of the spinners starts blocking simply > because you have no idea which one is the top prio spinner. > > T1 (prio = 0) T2 (prio = 50) T3 (prio = 10) > lock(RTM); > lock(RTM); lock(RTM); > spin() spin(); > --> preemption > enqueue() > boost(); > schedule(); > sees waiter bit > enqueue(); > boost(); > schedule(); > > T2 could happily keep spinning despite T3 going to sleep. I'm not sure > if that's what we want to achieve.
Yeah this is one of the reasons why I was tempted of checking the top-waiter prio against current prio to possibly keep spinning. Now doing so without holding the wait_lock is obviously racy, however safe afaict. If the top-waiter changes after it is checked by the spinner, and we get it wrong, at worst we send that thread falsely to sleep, otherwise we bogus spin once more -- neither of which is the end of the world. Ie:
[top waiter prio 10] T1 (prio 0) T2 T3 (prio 0) lock(RTM) if (rt_mutex_has_waiters(lock) && lock(RTM) rt_mutex_top_waiter()->prio > current->prio){ spin ... [release lock] [wakeup top waiter]
[adds itself to the tree] }
But I could be overlooking something, which is why I chose to exclude it in this patch.
Thanks, Davidlohr
| |