From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Stern Subject: Re: [PATCH] PERF(kernel): Cleanup power events V2 Date: Thu, 28 Oct 2010 11:22:55 -0400 (EDT) Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: 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 To: Mathieu Desnoyers Cc: Linux-pm mailing list , "Paul E. McKenney" , Kernel development list List-Id: linux-pm@vger.kernel.org On Wed, 27 Oct 2010, Alan Stern wrote: > On Wed, 27 Oct 2010, Mathieu Desnoyers wrote: > > > grep -r pm_runtime_get_noresume drivers/ hands out very interesting info. > > > > e.g.: > > > > drivers/usb/core/drivers.c: usb_autopm_get_interface_async() > > > > pm_runtime_get_noresume(&intf->dev); > > s = ACCESS_ONCE(intf->dev.power.runtime_status); > > if (s == RPM_SUSPENDING || s == RPM_SUSPENDED) > > status = pm_request_resume(&intf->dev); > > > > How is this supposed to work ? > > If the ACCESS_ONCE can be reordered before the atomic_inc(), then I fear the > > device can be suspended even after the check. > > You are correct; the code as written may sometimes fail. It was a > hack from the beginning; the kind of test it performs should not be > done outside the PM core. However at the time it was the easiest way > to do what I wanted. I forgot to mention one other thing... The fact that this code will sometimes behave unexpectedly isn't a bug. That function is documented as requiring additional locking when a driver uses it. The need for extra locking is unavoidable because I/O requests can arrive at any time, even while a runtime suspend is in progress. Therefore the fact that usb_autopm_get_interface_async() can race with a runtime suspend doesn't matter. The driver making the call should have sufficient locking to know that the runtime suspend should fail because the driver is busy. Alan Stern