Re: [PATCH -rt] catch put_task_struct RCU handling up to mainline

[Esben Nielsen <nielsen.esben@googlemail.com>]

On Mon, 10 Jul 2006, Paul E. McKenney wrote:

> On Sat, Jul 08, 2006 at 02:59:37PM +0100, Esben Nielsen wrote:
>> On Fri, 7 Jul 2006, Paul E. McKenney wrote:
>>
>>> On Fri, Jul 07, 2006 at 11:56:00PM +0100, Esben Nielsen wrote:
>>>> On Fri, 7 Jul 2006, Paul E. McKenney wrote:
>>>>
>>>>> Hello!
>>>>>
>>>>> Due to the separate -rt and mainline evolution of RCU signal handling,
>>>>> the -rt patchset now makes each task struct go through two RCU grace
>>>>> periods, with one call_rcu() in release_task() and with another
>>>>> in put_task_struct().  Only the call_rcu() in release_task() is
>>>>> required, since this is the one that is associated with tearing down
>>>>> the task structure.
>>>>>
>>>>> This patch removes the extra call_rcu() in put_task_struct(), synching
>>>>> this up with mainline.  Tested lightly on i386.
>>>>>
>>>>
>>>> The extra call_rcu() has an advantage:
>>>> It defers work away from the task doing the last put_task_struct().
>>>> It could be a priority 99 task with hard latency requirements doing
>>>> some PI boosting, forinstance. The extra call_rcu() defers non-RT work to
>>>> a low priority task. This is in generally a very good idea in a real-time
>>>> system.
>>>> So unless you can argue that the work defered is as small as the work of
>>>> doing a call_rcu() I would prefer the extra call_rcu().
>>>
>>> I would instead argue that the only way that the last put_task_struct()
>>> is an unrelated high-priority task is if it manipulating an already-exited
>>> task.  In particular, I believe that the sys_exit() path prohibits your
>>> example of priority-boosting an already-exited task by removing the
>>> exiting task from the various lists before doing the release_task()
>>> on itself.
>>>
>>> Please let me know what I am missing here!
>>
>> You could very well be right (I don't know the details that well). But in
>> that case the get/put_task_struct() in the PI code is not needed?
>> I think, however, it is needed because the task doing the (de)boosting
>> gets a pointer to a task, enables preemption and drops all locks. It then
>> uses the pointer. The task could have been deleted a long time ago if it
>> wasn't used protected by get/put_task_struct().
>>
>> This is an examble of why using reference counting in a RT system is a bad
>> idea: Suddenly a highpriority task can end up doing the cleanup for low
>> priority tasks.
>
> My belief is that the scenarios that lead to this situation involve error
> situations -- in which case the dying task will be missing whatever
> deadlines it had anyway, because it died before it could complete
> its work.  So, I agree that we need the get/put_task_struct() calls,
> but I believe that their job is to keep the system running in face of
> application errors.  Otherwise, it would be really hard to debug the
> application, right?
>
> That said, it is entirely possible that I am missing a code path where
> a correctly written application could legitimately force a high-priority
> task to do cleanup work on behalf of a low-priority path.
>
>> The work should be defered to a low priority task. Using rcu is
>> probably overkill because it also introduces other delays. A tasklet
>> or a dedicated task would be better.
>
> Agreed -- if there is in fact a legitimate non-error code path, then
> a patch that used some deferral mechanism would be good.  But RCU is
> overkill, and misleading overkill at that!
>

I think this is a legitimate situation. lock 1 is owned by B which is
blocked on lock 2 which is owned by C

  CPU1:                                      CPU2
     RT task A locks lock 1                C runs something
     A boosts B to RT
     A does get_task_struct B
     A enables interrupts                  C unlocks lock 2
     An very long interrupt is running     B unlocks lock 2
                                           B unlocks lock 1
                                           B is deboosted
                                           B exits
     A gets CPU1 again
     A does put_task_struct B

I don't know if the timing is realistic, but theoretically it is possible.
It might also be possible the B exits on another CPU even without the long
interrupt handler. If A has cpu affinity to CPU1 it is enough if a higher 
priority task preempts it on CPU1.

Esben

> 							Thanx, Paul
>