From patchwork Sat Jun 18 11:08:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 893002 Received: from smtp1.linux-foundation.org (smtp1.linux-foundation.org [140.211.169.13]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p5IBAplB032363 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL) for ; Sat, 18 Jun 2011 11:11:11 GMT Received: from daredevil.linux-foundation.org (localhost [127.0.0.1]) by smtp1.linux-foundation.org (8.14.2/8.13.5/Debian-3ubuntu1.1) with ESMTP id p5IB8Ovb006563; Sat, 18 Jun 2011 04:08:50 -0700 Received: from ogre.sisk.pl (ogre.sisk.pl [217.79.144.158]) by smtp1.linux-foundation.org (8.14.2/8.13.5/Debian-3ubuntu1.1) with ESMTP id p5IB7mgJ006535 for ; Sat, 18 Jun 2011 04:07:50 -0700 Received: from localhost (localhost.localdomain [127.0.0.1]) by ogre.sisk.pl (Postfix) with ESMTP id D1AB91AC6DF; Sat, 18 Jun 2011 12:48:33 +0200 (CEST) Received: from ogre.sisk.pl ([127.0.0.1]) by localhost (ogre.sisk.pl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 24145-04; Sat, 18 Jun 2011 12:48:14 +0200 (CEST) Received: from ferrari.rjw.lan (220-bem-13.acn.waw.pl [82.210.184.220]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ogre.sisk.pl (Postfix) with ESMTP id 8D7441AC682; Sat, 18 Jun 2011 12:48:14 +0200 (CEST) From: "Rafael J. Wysocki" To: Alan Stern Date: Sat, 18 Jun 2011 13:08:42 +0200 User-Agent: KMail/1.13.6 (Linux/3.0.0-rc3+; KDE/4.6.0; x86_64; ; ) References: <201106172329.37087.rjw@sisk.pl> In-Reply-To: <201106172329.37087.rjw@sisk.pl> MIME-Version: 1.0 Message-Id: <201106181308.42298.rjw@sisk.pl> X-Virus-Scanned: amavisd-new at ogre.sisk.pl using MkS_Vir for Linux Received-SPF: pass (localhost is always allowed.) X-Spam-Status: No, hits=-3.938 required=5 tests=AWL, BAYES_00, OSDL_HEADER_SUBJECT_BRACKETED X-Spam-Checker-Version: SpamAssassin 3.2.4-osdl_revision__1.47__ X-MIMEDefang-Filter: lf$Revision: 1.188 $ X-Scanned-By: MIMEDefang 2.63 on 140.211.169.21 Cc: linux-pm@lists.linux-foundation.org, linux-omap@vger.kernel.org Subject: Re: [linux-pm] calling runtime PM from system PM methods X-BeenThere: linux-pm@lists.linux-foundation.org X-Mailman-Version: 2.1.9 Precedence: list List-Id: Linux power management List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 18 Jun 2011 11:11:11 +0000 (UTC) On Friday, June 17, 2011, Rafael J. Wysocki wrote: > On Friday, June 17, 2011, Alan Stern wrote: > > On Fri, 17 Jun 2011, Rafael J. Wysocki wrote: > > > > > Having considered that a bit more I see that, in fact, commit > > > e8665002477f0278f84f898145b1f141ba26ee26 (PM: Allow pm_runtime_suspend() to > > > succeed during system suspend) has introduced at least one regression. > > > Namely, the PCI bus type runs pm_runtime_resume() in its .prepare() > > > callback to guarantee that devices will be in a well known state before > > > the PCI .suspend() and .suspend_noirq() callbacks are executed. > > > Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26 this > > > isn't valid any more, because devices can be runtime-suspend after the > > > pm_runtime_resume() in .prepare() has run. > > > > > > USB seems to do something similar in choose_wakeup(). > > > > > > So, either the both of these subsystems should be modified to use > > > pm_runtime_get_sync() and then pm_runtime_put_() some time > > > during resume, or we should revert commit e8665002477f0278f84f898145b1f141ba26ee26. > > > > pm_runtime_put_noidle would be appropriate. > > > > > Quite frankly, which shouldn't be a surprise to anyone at this point, I'd > > > prefer to revert that commit for 3.0. > > > > Maybe we can compromise. Instead of reverting that commit outright, > > put the get_noresume just before the suspend callback and put the > > put_sync just after the resume callback. > > That wouldn't fix the PCI problem, though, because it would leave a small > window in which the device could be suspended after the pm_runtime_resume() > in pci_pm_prepare() had run. That said, the PCI case can be solved with a separate patch and if the other subsystems are not affected, perhaps that's the best approach. Still, I'd like to make sure that there won't be any races between runtime PM and .suspend_noirq() and .resume_noirq() callbacks, so I'd like to apply the patch below. Thanks, Rafael --- drivers/base/power/main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) Index: linux-2.6/drivers/base/power/main.c =================================================================== --- linux-2.6.orig/drivers/base/power/main.c +++ linux-2.6/drivers/base/power/main.c @@ -591,6 +591,8 @@ void dpm_resume(pm_message_t state) async_error = 0; list_for_each_entry(dev, &dpm_suspended_list, power.entry) { + pm_runtime_get_noresume(dev); + pm_runtime_enable(dev); INIT_COMPLETION(dev->power.completion); if (is_async(dev)) { get_device(dev); @@ -614,6 +616,7 @@ void dpm_resume(pm_message_t state) } if (!list_empty(&dev->power.entry)) list_move_tail(&dev->power.entry, &dpm_prepared_list); + pm_runtime_put_noidle(dev); put_device(dev); } mutex_unlock(&dpm_list_mtx); @@ -939,8 +942,10 @@ int dpm_suspend(pm_message_t state) put_device(dev); break; } - if (!list_empty(&dev->power.entry)) + if (!list_empty(&dev->power.entry)) { list_move(&dev->power.entry, &dpm_suspended_list); + pm_runtime_disable(dev); + } put_device(dev); if (async_error) break;