lkml.org 
[lkml]   [2016]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/4] locking/rwsem: Drop superfluous waiter refcount
On Mon, 09 May 2016, Peter Zijlstra wrote:

>> >So I think you're wrong here; imagine this:
>> >
>> >
>> > rwsem_down_read_failed() rwsem_wake()
>> > get_task_struct();
>> > raw_spin_lock_irq(&wait_lock);
>> > list_add_tail(&waiter.list, &wait_list);
>> > raw_spin_unlock_irq(&wait_lock);
>> > raw_spin_lock_irqsave(&wait_lock)
>> > __rwsem_do_wake()
>> > while (true) {
>> > set_task_state(tsk, TASK_UNINTERRUPTIBLE);
>> > waiter->task = NULL
>> > if (!waiter.task) // true
>> > break;
>> >
>> > __set_task_state(tsk, TASK_RUNNING);
>> >
>> > do_exit();
>> > wake_up_process(tsk); /* BOOM */
>>
>> I may be missing something, but rwsem_down_read_failed() will not return until
>> after the wakeup is done by the rwsem_wake() thread.
>
>The above never gets to schedule(), and even if it did, a spurious
>wakeup could've happened, no?

Ah indeed, you are most certainly correct. For some reason I was always
considering schedule() in the picture. Hmm I'll have to think about this
some more, but given the small chance of a waiter actually seeing the nil
task at the first iteration I'm wondering if we could just invert the code
and call schedule() before the task check. Saving the refcounts will serve
_all_ reader waiters otoh, but this would obviously need numbers...

Thanks,
Davidlohr

\
 
 \ /
  Last update: 2016-05-09 21:21    [W:0.246 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site