Messages in this thread | | | Subject | Re: query: [PATCH 2/2] cgroup: Remove call to synchronize_rcu in cgroup_attach_task | From | Mike Galbraith <> | Date | Mon, 02 May 2011 16:29:14 +0200 |
| |
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
| |