Re: Using sparse to catch invalid RCU dereferences?

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

On Tue, Apr 08, 2008 at 06:09:15PM +0200, Johannes Berg wrote:
> 
> > So the address_space attribute says what the pointer points to rather
> > than where the pointer resides, correct?
> 
> Yeah. It's currently used for __user and __iomem. Using it for rcu
> wouldn't be quite the way it was intended, I think, but hey :)

;-)

> > It might be.  There are a number of places where it is legal to access
> > RCU-protected pointers directly, and all of these would need to be
> > changed.  For example, in the example above, one could do:
> > 
> > 	foo = NULL;
> 
> Yeah, all of those would lead to sparse warnings. Are we willing to
> change all that code?

If it found some bugs, I would certainly be in favor!

> > I recently tried to modify rcu_assign_pointer() to issue the memory
> > memory barrier only when the pointer was non-NULL, but this ended badly.
> > Probably because I am not the greatest gcc expert around...  We ended
> > up having to define an rcu_assign_index() to handle the possibility of
> > assigning a zero-value array index, but my attempts to do type-checking
> > backfired, and I eventually gave it up.  Again, someone a bit more clued
> > in to gcc than I am could probably pull it off.
> 
> I don't think I would be that person :)

When it comes to gcc extensions and type-casting trickery, I am not all
that clued in, to be honest...

> > In addition, it is legal to omit rcu_dereference() and rcu_assign_pointer()
> > when holding the update-side lock.
> 
> Right. Those too would lead to problems, unless we change that code to
> use those (or other) macros.

Yep.

> > So I very much like this approach in general, but it will require some
> > care to implement.  I would be very happy to review and comment!!!
> 
> I'll play with it a bit if I get around, was just reviewing some RCU
> usage and had the feeling that it should be possible to automate.

If it finds a few bugs, it would be worth it!

							Thanx, Paul