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

From: Esben Nielsen
Date: Mon Jul 10 2006 - 13:08:50 EST


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

-
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/