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

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

[Paul E. McKenney]

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.

CC: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Paul E. McKenney <paulmck@us.ibm.com>
---

 include/linux/sched.h |   10 ----------
 kernel/fork.c         |   21 ---------------------
 2 files changed, 31 deletions(-)

diff -urpNa -X dontdiff linux-2.6.17-rt5/include/linux/sched.h linux-2.6.17-rt5-2RCU/include/linux/sched.h
--- linux-2.6.17-rt5/include/linux/sched.h	2006-07-02 12:37:14.000000000 -0700
+++ linux-2.6.17-rt5-2RCU/include/linux/sched.h	2006-07-06 18:11:49.000000000 -0700
@@ -1105,15 +1105,6 @@ static inline int pid_alive(struct task_
 extern void free_task(struct task_struct *tsk);
 #define get_task_struct(tsk) do { atomic_inc(&(tsk)->usage); } while(0)
 
-#ifdef CONFIG_PREEMPT_RT
-extern void __put_task_struct_cb(struct rcu_head *rhp);
-
-static inline void put_task_struct(struct task_struct *t)
-{
-	if (atomic_dec_and_test(&t->usage))
-		call_rcu(&t->rcu, __put_task_struct_cb);
-}
-#else
 extern void __put_task_struct(struct task_struct *t);
 
 static inline void put_task_struct(struct task_struct *t)
@@ -1121,7 +1112,6 @@ static inline void put_task_struct(struc
 	if (atomic_dec_and_test(&t->usage))
 		__put_task_struct(t);
 }
-#endif
 
 /*
  * Per process flags
diff -urpNa -X dontdiff linux-2.6.17-rt5/kernel/fork.c linux-2.6.17-rt5-2RCU/kernel/fork.c
--- linux-2.6.17-rt5/kernel/fork.c	2006-07-02 12:37:15.000000000 -0700
+++ linux-2.6.17-rt5-2RCU/kernel/fork.c	2006-07-07 07:44:36.000000000 -0700
@@ -120,26 +120,6 @@ void free_task(struct task_struct *tsk)
 }
 EXPORT_SYMBOL(free_task);
 
-#ifdef CONFIG_PREEMPT_RT
-void __put_task_struct_cb(struct rcu_head *rhp)
-{
-	struct task_struct *tsk = container_of(rhp, struct task_struct, rcu);
-
-	BUG_ON(atomic_read(&tsk->usage));
-	WARN_ON(!(tsk->flags & PF_DEAD));
-	WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE)));
-	WARN_ON(tsk == current);
-
-	security_task_free(tsk);
-	free_uid(tsk->user);
-	put_group_info(tsk->group_info);
-
-	if (!profile_handoff_task(tsk))
-		free_task(tsk);
-}
-
-#else
-
 void __put_task_struct(struct task_struct *tsk)
 {
 	WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE)));
@@ -154,7 +134,6 @@ void __put_task_struct(struct task_struc
 	if (!profile_handoff_task(tsk))
 		free_task(tsk);
 }
-#endif
 
 void __init fork_init(unsigned long mempages)
 {

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

[Esben Nielsen]

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().

Esben

> CC: Oleg Nesterov <oleg@tv-sign.ru>
> Signed-off-by: Paul E. McKenney <paulmck@us.ibm.com>
> ---
>
> include/linux/sched.h |   10 ----------
> kernel/fork.c         |   21 ---------------------
> 2 files changed, 31 deletions(-)
>
> diff -urpNa -X dontdiff linux-2.6.17-rt5/include/linux/sched.h linux-2.6.17-rt5-2RCU/include/linux/sched.h
> --- linux-2.6.17-rt5/include/linux/sched.h	2006-07-02 12:37:14.000000000 -0700
> +++ linux-2.6.17-rt5-2RCU/include/linux/sched.h	2006-07-06 18:11:49.000000000 -0700
> @@ -1105,15 +1105,6 @@ static inline int pid_alive(struct task_
> extern void free_task(struct task_struct *tsk);
> #define get_task_struct(tsk) do { atomic_inc(&(tsk)->usage); } while(0)
>
> -#ifdef CONFIG_PREEMPT_RT
> -extern void __put_task_struct_cb(struct rcu_head *rhp);
> -
> -static inline void put_task_struct(struct task_struct *t)
> -{
> -	if (atomic_dec_and_test(&t->usage))
> -		call_rcu(&t->rcu, __put_task_struct_cb);
> -}
> -#else
> extern void __put_task_struct(struct task_struct *t);
>
> static inline void put_task_struct(struct task_struct *t)
> @@ -1121,7 +1112,6 @@ static inline void put_task_struct(struc
> 	if (atomic_dec_and_test(&t->usage))
> 		__put_task_struct(t);
> }
> -#endif
>
> /*
>  * Per process flags
> diff -urpNa -X dontdiff linux-2.6.17-rt5/kernel/fork.c linux-2.6.17-rt5-2RCU/kernel/fork.c
> --- linux-2.6.17-rt5/kernel/fork.c	2006-07-02 12:37:15.000000000 -0700
> +++ linux-2.6.17-rt5-2RCU/kernel/fork.c	2006-07-07 07:44:36.000000000 -0700
> @@ -120,26 +120,6 @@ void free_task(struct task_struct *tsk)
> }
> EXPORT_SYMBOL(free_task);
>
> -#ifdef CONFIG_PREEMPT_RT
> -void __put_task_struct_cb(struct rcu_head *rhp)
> -{
> -	struct task_struct *tsk = container_of(rhp, struct task_struct, rcu);
> -
> -	BUG_ON(atomic_read(&tsk->usage));
> -	WARN_ON(!(tsk->flags & PF_DEAD));
> -	WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE)));
> -	WARN_ON(tsk == current);
> -
> -	security_task_free(tsk);
> -	free_uid(tsk->user);
> -	put_group_info(tsk->group_info);
> -
> -	if (!profile_handoff_task(tsk))
> -		free_task(tsk);
> -}
> -
> -#else
> -
> void __put_task_struct(struct task_struct *tsk)
> {
> 	WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE)));
> @@ -154,7 +134,6 @@ void __put_task_struct(struct task_struc
> 	if (!profile_handoff_task(tsk))
> 		free_task(tsk);
> }
> -#endif
>
> void __init fork_init(unsigned long mempages)
> {
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

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

[Paul E. McKenney]

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!

							Thanx, Paul

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

[Esben Nielsen]

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.

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.

Esben

>
> 							Thanx, Paul
>

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

[Paul E. McKenney]

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!

							Thanx, Paul

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

[Paul E. McKenney]

On Mon, Jul 10, 2006 at 07:10:49PM +0100, Esben Nielsen wrote:
> On Mon, 10 Jul 2006, Paul E. McKenney wrote:
> >On Sat, Jul 08, 2006 at 02:59:37PM +0100, Esben Nielsen wrote:

[ . . . ]

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

For this to happen, either A has to be at a lower priority than the irq
tasks or the interrupt has to be a hard irq (e.g., scheduling clock
interrupt).  In the first case, the added cleanup processing seems
inconsequential compared to (say) an interrupt doing network protocol
processing.  In the second case, B does not do its put_task_struct()
until after the hard irq returns (because the put_task_struct() is invoked
from a call_rcu() callback), which makes the above scenario unlikely,
though perhaps not impossible.

If the second scenario is in fact possible, would you be willing to
supply the appropriate deferral code?  I believe we both agree that RCU
is not really the right deferral mechanism in this situation.

							Thanx, Paul

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

[Esben Nielsen]

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
>

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

[Esben Nielsen]

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

> On Mon, Jul 10, 2006 at 07:10:49PM +0100, Esben Nielsen wrote:
>> On Mon, 10 Jul 2006, Paul E. McKenney wrote:
>>> On Sat, Jul 08, 2006 at 02:59:37PM +0100, Esben Nielsen wrote:
>
> [ . . . ]
>
>>>> 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.
>
> For this to happen, either A has to be at a lower priority than the irq
> tasks or the interrupt has to be a hard irq (e.g., scheduling clock
> interrupt).  In the first case, the added cleanup processing seems
> inconsequential compared to (say) an interrupt doing network protocol
> processing.  In the second case, B does not do its put_task_struct()
> until after the hard irq returns (because the put_task_struct() is invoked
> from a call_rcu() callback), which makes the above scenario unlikely,
> though perhaps not impossible.
>
> If the second scenario is in fact possible, would you be willing to
> supply the appropriate deferral code?  I believe we both agree that RCU
> is not really the right deferral mechanism in this situation.
>

Let your patch go through. I'll stop complaining :-)
Is there anywhere where we can make a list of known issues like this?
I can't promise I will get time to fix this one :-(

Esben


> 							Thanx, Paul
>

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

[Paul E. McKenney]

On Mon, Jul 10, 2006 at 09:09:35PM +0100, Esben Nielsen wrote:
> On Mon, 10 Jul 2006, Paul E. McKenney wrote:
> >On Mon, Jul 10, 2006 at 07:10:49PM +0100, Esben Nielsen wrote:
> >>On Mon, 10 Jul 2006, Paul E. McKenney wrote:
> >>>On Sat, Jul 08, 2006 at 02:59:37PM +0100, Esben Nielsen wrote:
> >
> >[ . . . ]
> >
> >>>>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.
> >
> >For this to happen, either A has to be at a lower priority than the irq
> >tasks or the interrupt has to be a hard irq (e.g., scheduling clock
> >interrupt).  In the first case, the added cleanup processing seems
> >inconsequential compared to (say) an interrupt doing network protocol
> >processing.  In the second case, B does not do its put_task_struct()
> >until after the hard irq returns (because the put_task_struct() is invoked
> >from a call_rcu() callback), which makes the above scenario unlikely,
> >though perhaps not impossible.
> >
> >If the second scenario is in fact possible, would you be willing to
> >supply the appropriate deferral code?  I believe we both agree that RCU
> >is not really the right deferral mechanism in this situation.
> >
> 
> Let your patch go through. I'll stop complaining :-)
> Is there anywhere where we can make a list of known issues like this?
> I can't promise I will get time to fix this one :-(

;-)

One possibility would be a file in the Documentation directory.
Another would be something on the web, but a file in the Documentation
directory would have the advantage of being synched with the -rt version.

							Thanx, Paul

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

[Ingo Molnar]

* Paul E. McKenney <paulmck@us.ibm.com> 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.

i've applied your patch to the -rt tree - we'll see how it goes.

	Ingo