Re: [PATCH] NETLABEL: Fix an RCU warning

[Paul Moore <paul.moore@hp.com>]

On Thursday 25 March 2010 09:32:20 am Eric Dumazet wrote:
> Le jeudi 25 mars 2010 à 11:37 +0000, David Howells a écrit :
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Sorry this is not the right fix.
> > > 
> > > Fix is to change the dereference check to take into account the lock
> > > owned here.
> > 
> > Then the comments on netlbl_unlhsh_hash(), netlbl_unlhsh_search_iface(),
> > netlbl_unlhsh_search_iface_def() and netlbl_unlhsh_add_iface() are all
> > wrong,
> > 
> > for all of them say:
> > 	* The caller is responsible for calling the rcu_read_[un]lock()
> > 	* functions.
> > 
> > Furthermore, netlabel_unlhsh_add() and netlabel_unlhsh_remove() _do_ wrap
> > the calls to those functions in rcu_read_lock'd sections.
> 
> Current code is probably fine.
> Comments are not up to date (as many other comments BTW)
> 
> Only the dereference check is bad, as it assumes the rcu_read lock is
> held.
> 
> Its not the case, we own a spinlock.
> 
> You suggest adding a surrounding rcu lock, but this surrounding lock
> adds overhead on normal kernels, to correct checker warnings only.

The rcu_dereference call, and the outdated comments for that matter, are 
likely artifacts from an early patch.  When the code was originally being 
developed I had assumed that you _always_ needed to hold the RCU read lock 
when doing RCU operations, regardless of if you were already holding the 
associated spinlock.  Thankfully, Paul McKenney was able to review the code 
and helped me to understand RCU a bit better, unfortunately, it looks like I 
still missed a few spots :/

Give me a day or two to go through the comments again and do some sanity 
checks and I'll send out a patch.  However, if you've already got something 
that is tested and ready to go don't let me stand in your way.

-- 
paul moore
linux @ hp