Re: [BUG] TASK_DEAD task is able to be woken up in special condition

KOSAKI Motohiro
2011-12-22T20:03:05Z

>> I doubt it is not only TASK_DEAD issue, it is rwsem fundamental issue.
>> Because of, a lot of place assume "current->state = newstate" is safe
>> and don't need any synchronization. So, I'm worry about to lost
>> TASK_UNINTERRUPTIBLE can make catastrophe like TASK_DEAD.
>
> I don't understand why this is catastrophe.
> I suppose it is just waken up from TASK_UNINTERRUPTIBLE
> by try_to_wake_up() in race condition. It seems to be normal situation.....

First, yes, try_to_wake_up() has a race. caller must care for spurious wakeup.
But, mutex, rwsem and other synchronization primitives must hide it and ensure
locking semantics. That's one of the reason why we use lock primitive instead
of bare try_to_wake_up(). I don't think "hey, all drivers code must
know scheduler
race and scheduler internal deeply" is practical solution.

> But TASK_DEAD status is special. It must not return to TASK_RUNNING state.

O.K. agree. but your patch seems just hacky. (1) I'm worry about why
out of scheduler
code know pi_lock and take it (2) all of other task->state changing
place don't take
pi_lock. so, your proposed locking semantics is not clear.

So, to remove TASK_DEAD and to add new member task->is_dead is alternative idea.

>> @@ -208,9 +208,9 @@ rwsem_down_failed_common(struct rw_semaphore *sem,
>>
>>          /* wait to be given the lock */
>>          for (;;) {
>> +               schedule();
>>                  if (!waiter.task)
>>                          break;
>> -               schedule();
>>                  set_task_state(tsk, TASK_UNINTERRUPTIBLE);
>>          }
>>
>
> Hmmmmmmm.
> Are you sure there is no route which TASK_DEAD task is waken up like rwsem?

Not sure. but all of them are a bug if exist, I think.