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

[Lai Jiangshan]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/