On Tuesday, October 26, 2010, Mathieu Desnoyers wrote: > * Rafael J. Wysocki ([email protected]) wrote: > > On Tuesday, October 26, 2010, Mathieu Desnoyers wrote: > > > * Peter Zijlstra ([email protected]) wrote: > > > > On Tue, 2010-10-26 at 11:56 -0500, Pierre Tardy wrote: > > > > > > > > > > + trace_runtime_pm_usage(dev, > > > > > atomic_read(&dev->power.usage_count)+1); > > > > > atomic_inc(&dev->power.usage_count); > > > > > > > > That's terribly racy.. > > > > > > Looking at the original code, it looks racy even without considering the > > > tracepoint: > > > > > > int __pm_runtime_get(struct device *dev, bool sync) > > > { > > > int retval; > > > > > > + trace_runtime_pm_usage(dev, > > > atomic_read(&dev->power.usage_count)+1); > > > atomic_inc(&dev->power.usage_count); > > > retval = sync ? pm_runtime_resume(dev) : pm_request_resume(dev); > > > > > > There is no implied memory barrier after "atomic_inc". So either all these > > > inc/dec are protected with mutexes or spinlocks, in which case one might > > > wonder > > > why atomic operations are used at all, or it's a racy mess. (I vote for > > > the > > > second option) > > > > No, it isn't. > > > > > kref should certainly be used there. > > > > No, it shouldn't. > > > > Please try to understand the code you're commenting on first. > > Please see my reply to Alan Stern: > > http://www.spinics.net/lists/linux-omap/msg39382.html
I have and I'm still unimpressed. :-) Please see my reply to that message. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
