From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH] PERF(kernel): Cleanup power events V2 Date: Wed, 27 Oct 2010 00:22:55 +0200 Message-ID: <201010270022.56298.rjw@sisk.pl> References: <1287488171-25303-3-git-send-email-trenn@suse.de> <201010262104.21851.rjw@sisk.pl> <20101026213817.GB21495@Krystal> Mime-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: Received: from ogre.sisk.pl ([217.79.144.158]:56011 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753166Ab0JZWXp (ORCPT ); Tue, 26 Oct 2010 18:23:45 -0400 In-Reply-To: <20101026213817.GB21495@Krystal> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Mathieu Desnoyers Cc: Peter Zijlstra , Greg Kroah-Hartman , Pierre Tardy , Arjan van de Ven , Ingo Molnar , Thomas Renninger , Linus Torvalds , Andrew Morton , Thomas Gleixner , Masami Hiramatsu , Frank Eigler , Steven Rostedt , Kevin Hilman , linux-omap@vger.kernel.org, linux-pm@lists.linux-foundation.org, linux-trace-users@vger.kernel.org, Jean Pihet , Frederic Weisbecker , Tejun Heo On Tuesday, October 26, 2010, Mathieu Desnoyers wrote: > * Rafael J. Wysocki (rjw@sisk.pl) wrote: > > On Tuesday, October 26, 2010, Mathieu Desnoyers wrote: > > > * Peter Zijlstra (peterz@infradead.org) 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