Re: query: [PATCH 2/2] cgroup: Remove call to synchronize_rcu in cgroup_attach_task

[Mike Galbraith <efault@gmx.de>]

On Mon, 2011-05-02 at 06:46 -0700, Paul E. McKenney wrote:
> On Fri, Apr 29, 2011 at 02:34:47PM +0200, Mike Galbraith wrote:

> > Makes one wonder what these things do for a living.
> 
> If you are adding something to an RCU-protected data structure, then you do
> not need synchronize_rcu().  But if you are removing something from
> an RCU-protected data structure, then you really do need them.  If you
> leave them out, you can see the following type of failure:
> 
> 1.	CPU 0, running in an RCU read-side critical section, obtains
> 	a pointer to data item A.
> 
> 2.	CPU 1 removes data item A from the structure.
> 
> 3.	CPU 1 does not do a synchronize_rcu().  If CPU 1 had done a
> 	synchronize_rcu(), then it would have waited until CPU 0 had
> 	left its RCU read-side critical section, and thus until
> 	CPU 0 stopped using its pointer to data item A.  But there was
> 	no synchronize_rcu(), so CPU 0 is still looking at data item A.
> 
> 4.	CPU 1 frees data item A.
> 
> This is very bad.  CPU 0 has a pointer into the freelist.  Worse yet,
> some other CPU might allocate memory and get a pointer to data item A.
> That CPU and CPU 0 would then have an interesting but short lived
> disagreement about that memory's type.  Crash goes the kernel.
> 
> So please do not remove synchronize_rcu() calls unless you can prove
> that it is safe to do so!

In these instances are a little different.

We have..
    start teardown
    synchronize_rcu()
    finish teardown
    call_rcu(kfree_it) 
..so removal wouldn't trigger the standard "let's rummage around in
freed memory" kind of excitement.

But yeah, removing them without proof is out.

My box was telling me that they _are_ safe to remove, by not exploding
with list/slub debug enabled while I beat the snot out of it.. which is
evidence, but not proof :)

	-Mike