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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
