Subject: Re: xfrm_state locking regression...

From: "Paul E. McKenney" <[email-masked]>
Date: Thu, 11 Sep 2008 16:22:12 -0700

On Thu, Sep 11, 2008 at 03:00:06PM -0700, David Miller wrote:
> From: "Paul E. McKenney" <[email-masked]>
> Date: Thu, 11 Sep 2008 14:24:59 -0700
> 
> > On Wed, Sep 10, 2008 at 12:33:12AM +1000, Herbert Xu wrote:
> > > On Mon, Sep 08, 2008 at 05:25:13PM -0700, David Miller wrote:
> > > > 
> > > > The only comment I would make is that maybe it's a bit excessive
> > > > to trigger the GC worker every time we walk the states.
> > > 
> > > Good point!
> > > 
> > > I've avoided the memory barrier by simply extending the mutexed
> > > section in the GC to cover the list splicing.  Here's the updated
> > > patch:
> > > 
> > > ipsec: Use RCU-like construct for saved state within a walk
> > > 
> > > Now that we save states within a walk we need synchronisation
> > > so that the list the saved state is on doesn't disappear from
> > > under us.
> > > 
> > > As it stands this is done by keeping the state on the list which
> > > is bad because it gets in the way of the management of the state
> > > life-cycle.
> > > 
> > > An alternative is to make our own pseudo-RCU system where we use
> > > counters to indicate which state can't be freed immediately as
> > > it may be referenced by an ongoing walk when that resumes.
> > 
> > There is only one reader at a time, right?  Otherwise, I don't see how
> > the increments and reads of xfrm_state_walk_completed line up.
> 
> The RTNL semaphore is held across the modifications of the counters.

Got it, thank you, sorry for the noise!

							Thanx, Paul