Re: [GIT PULL rcu/urgent] yet more lockdep-RCU splat fixes

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Wed, Jun 16, 2010 at 08:23:51AM +0200, Peter Zijlstra wrote:
> On Wed, 2010-06-16 at 07:53 +0200, Ingo Molnar wrote:
> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > Hello, Ingo,
> > > 
> > > Here are a few more fixes for RCU-lockdep splats.
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-2.6-rcu.git rcu/urgent
> > > 
> > > This is based off of v2.6.35-rc3.
> > > 
> > > 							Thanx, Paul
> > > 
> > > ------------------>
> > > Daniel J Blueman (1):
> > >       rcu: fix lockdep splat in wake_affine()
> > > 
> > > Paul E. McKenney (2):
> > >       rcu: fix scope of wake_affine()'s new RCU read-side critical section
> > >       idr: fix RCU lockdep splat in idr_get_next()
> > > 
> > >  kernel/sched_fair.c |    2 ++
> > >  lib/idr.c           |    4 ++--
> > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > Pulled, thanks a lot Paul!
> 
> I'm not at all sure of those two wake_affine() ones..

Hello, Peter!

Here is the story as I understand it:

o	wake_affine() calls task_group() and uses the resulting
	pointer, for example, passing it to effective_load().

	This pointer is to a struct task_group, which contains
	a struct rcu_head, which is passed to call_rcu in
	sched_destroy_group().  So some protection really is
	needed -- or is it enough that wake_affine seems to be
	invoked on the current task?  If the latter, we would
	need to add a "task == current" check to task_subsys_state().

o	task_group() calls task_subsys_state(), returning a pointer to
	the enclosing task_group structure.

o	task_subsys_state() returns an rcu_dereference_check()ed
	pointer.  The caller must either be in an RCU read-side
	critical section, hold the ->alloc_lock, or hold the
	cgroup lock.

Now wake_affine() appears to be doing load calculations, so it does not
seem reasonable to acquire the lock.  Hence the use of RCU.

So, what should we be doing instead?  ;-)

							Thanx, Paul