Re: [PATCH] rtmutex: ensure only the top waiter or higher priority task can take the lock and reduce unrelated boosting

Lai Jiangshan
2010-12-16T01:13:52Z

On 12/16/2010 05:53 AM, Steven Rostedt wrote:
> On Wed, 2010-12-15 at 16:09 +0800, Lai Jiangshan wrote:
>
>
>> /*
>> @@ -543,11 +491,13 @@ static void remove_waiter(struct rt_mutex *lock,
>>
>> raw_spin_lock_irqsave(&current->pi_lock, flags);
>> plist_del(&waiter->list_entry, &lock->wait_list);
>> - waiter->task = NULL;
>> current->pi_blocked_on = NULL;
>> raw_spin_unlock_irqrestore(&current->pi_lock, flags);
>>
>> - if (first && owner != current) {
>> + if (!owner)
>> + return;
>> +
>> + if (first) {
>
> This is a bug. There's a small chance that the mutex timed out, and at
> that same time, the owner gave up the lock and woke this task up. Which
> means this task is the new owner of the lock iff it was the
> rt_mutex_top_waiter().
>
> The fix is to do this:
>
>
> if (!owner) {
> if (first) {
> ret = try_to_take_rt_mutex();
> BUG_ON(!ret);
> }
> return first;
> }
>
> if (first) {
>
> We need to make remove_waiter return 1 if it took the lock and 0 if it
> did not, so it can pass this information back to the caller.
>
> if (unlikely(ret)) {
> if (remove_waiter(...))
> ret = 0;
> }
>

It has called try_to_take_rt_mutex() in __rt_mutex_slowlock(),
when timeout or got signal, it returns from __rt_mutex_slowlock()
with lock->wait_lock still held, and then calls remove_waiter(),

so we don't need to call try_to_take_rt_mutex() in remove_waiter().
It is strange that remove_waiter() do some "require lock" work.

Thanks,
Lai