Re: [PATCH] generic dynamic per cpu refcounting

[Tejun Heo <tj@kernel.org>]

Hello, guys.

On Mon, Jan 28, 2013 at 10:15:28AM -0800, Kent Overstreet wrote:
> > 	percpu_ref_kill();
> > 	put_and_dsetroy();
> > 
> > And this can race with another holder which drops the last reference,
> > its put_and_dsetroy() can see PCPU_REF_DYING and return false.
> > 
> > Or I misunderstood the code/interface?
> 
> Nope, nailed it :) That should _definitely_ be in the documentation.

Can we just combine kill initiation and base ref put and make that the
responsibility of the owner?  Extra features on basic constructs may
seem good for certain use cases but tend to bring more confusion than
good in the long run.  If a user needs to synchronize among multiple
killers, let the user deal with the issue.

> Actually - I think it'd be better to have the default percpu_ref_kill()
> do the second synchronize_rcu(), and have an unsafe version that skips
> it.

Note that synchronize_rcu/sched() can be very slow and cause problems
in paths which are frequently traveled and visible to userland.  It's
fine for things like module destruction but can be a problem even
during device destruction - blkcg had synchronize_rcu() in
request_queue destruction which led to huge latencies during boot
because SCSI wants to create and then destroy request_queues for all
possible LUNs on certain configurations.  So, if you put
synchronize_rcu/sched() in percpu_ref_kill(), that better not be used
from e.g. close(2).

Thanks.

-- 
tejun