Re: [PATCH percpu/for-3.16 2/2] percpu-refcount: implement percpu_ref_tryget()

[Tejun Heo <tj@kernel.org>]

Hey,

On Sun, May 11, 2014 at 05:06:19PM -0700, Kent Overstreet wrote:
> On Fri, May 09, 2014 at 03:55:22PM -0400, Tejun Heo wrote:
> > Hello,
> > 
> > On Fri, May 09, 2014 at 12:51:09PM -0700, Kent Overstreet wrote:
> > > Well not so much deprecated as "bad, avoid" - IMO using tryget() almost always
> > > (I haven't seen a convincing counterexample) means you screwed up your
> > > refcounting somewhere, if you need to take a ref on something whatever made that
> > > object visible to you should have its own ref.
> > > 
> > > (I think we had this debate, but that was awhile ago...)
> > 
> > Oh sure, tryget can definitely be misunderstood but RCU protected
> > iteration is one valid use case.
> > 
> > 	rcu_read_lock();
> > 	locate the object of interest;
> > 	tryget[_live]() depending on the use case;
> > 	rcu_read_unlock();
> > 
> > 	access the object.
> 
> No, it's not needed with RCU... look at the aio code for an example (or don't,
> save your eyes instead).
> 
> Conceptually the RCU data structure should own a refcount on the things that are
> accessible via it; that ref shouldn't be dropped until after it's removed and an
> RCU barrier has happened.

You can do that if the object can disappear from iteration when its
ref gets killed.  IOW, the list visiblity is not tied to the reference
count.  cgroup_subsys_states can't do that.  They have to remain
visible to iteration until the object's last ref is dropped because
controllers need to be able to walk them while the object is being
drained.

Thanks.

-- 
tejun