Re: [linux-pm] [PATCH] PERF(kernel): Cleanup power events V2

[Alan Stern]

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

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