Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Wednesday, November 14, 2012 09:08:28 AM Huang Ying wrote:
> On Tue, 2012-11-13 at 11:10 -0500, Alan Stern wrote:
> > On Tue, 13 Nov 2012, Huang Ying wrote:
> > 
> > > > This is not quite right.  Consider a device that is in runtime suspend 
> > > > when a system sleep starts.  When the system sleep ends, the device 
> > > > will be resumed but the PM core will still think its state is 
> > > > SUSPENDED.  The subsystem has to tell the PM core that the device is 
> > > > now ACTIVE.  Currently, subsystems do this by calling 
> > > > pm_runtime_disable, pm_runtime_set_active, pm_runtime_enable.  Under 
> > > > your scheme this wouldn't work; the pm_runtime_set_active call would 
> > > > fail because the device was !forbidden.
> > > 
> > > Thanks for your information.  For this specific situation, is it
> > > possible to call pm_runtime_resume() or pm_request_resume() for the
> > > device?
> > 
> > No, because the device already is at full power.  The subsystem just
> > needs to tell the PM core that it is.
> > 
> > > > > PM.  Device can always work with full power.
> > > > 
> > > > It can't if the parent is in SUSPEND.  If necessary, the user can write 
> > > > "on" to the parent's power/control attribute first.
> > > 
> > > Is it possible to call pm_runtime_set_active() for the parent if the
> > > parent is disabled and SUSPENDED.
> > 
> > Doing that is possible, but it might not work.  The parent might
> > actually be at low power; calling pm_runtime_set_active wouldn't change
> > the physical power level.  Basically, it's not safe to assume anything
> > about devices that are disabled for runtime PM.
> > 
> > > It appears that there is race condition between this and the
> > > pm_runtime_disable, pm_runtime_set_active, pm_runtime_enable sequence
> > > you mentioned ealier.
> > > 
> > > thread 1			thread 2
> > > pm_runtime_disable
> > > pm_runtime_set_active
> > > 				pm_runtime_allow
> > > 				  pm_runtime_set_suspended
> > > pm_runtime_enable
> > 
> > This can't happen in the situation I described earlier because during
> > system sleep transitions, no other user threads are allowed to run.  
> > All of them except the one actually carrying out the transition are
> > frozen.
> 
> Thanks for your kind explanation.
> 
> After talking with you, my feeling is that the disabled state is obscure
> and error-prone.  So I suggest not to use it if possible.  Maybe we can
> 
>  - make changes suggested by Alan to make disabled state better.

What changes specifically do you mean to be precise?

>  - use Rafael's solution to solve this specific issue, and avoid the
> usage of disabled state here.

Well, I think that the PCI subsystem should just enable runtime PM for
all devices upfront and keep it enabled going forward.

My patch is incomplete, however, because it doesn't deal with probe/remove
correctly at this point (which Alan pointed out earlier in the thread).

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.