linux-kernel - Re: [PATCH 1/1] PM / Runtime: let rpm_resume fail if rpm disabled and device suspended.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 16 Jun 2014 23:29:17 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Allen Yu <alleny@...dia.com>, Pavel Machek <pavel@....cz>,
	Len Brown <len.brown@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] PM / Runtime: let rpm_resume fail if rpm disabled and device suspended.

On Monday, June 16, 2014 01:40:05 PM Alan Stern wrote:
> On Sat, 14 Jun 2014, Allen Yu wrote:
> 
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -608,7 +608,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
> >   repeat:
> >  	if (dev->power.runtime_error)
> >  		retval = -EINVAL;
> > -	else if (dev->power.disable_depth == 1 && dev->power.is_suspended
> > +	else if (dev->power.disable_depth == 1 && !dev->power.is_suspended
> >  	    && dev->power.runtime_status == RPM_ACTIVE)
> >  		retval = 1;
> 
> For reasons having nothing to do with Allen's suggested change, I
> wonder if we shouldn't replace this line with something like:
> 
> -	else if (dev->power.disable_depth == 1 && dev->power.is_suspended
> +	else if (dev->power.disable > 0 && !dev->power.is_suspended
> 	    && dev->power.runtime_status == RPM_ACTIVE)
> 		retval = 1;
> 
> It seems that I've been bitten by this several times in the past.  
> When a device is disabled for runtime PM, and more or less permanently
> stuck in the RPM_ACTIVE state, calls to pm_runtime_resume() or
> pm_runtime_get_sync() shouldn't fail.
> 
> For example, suppose some devices of a certain type support runtime 
> power management but others don't.  We naturally want to call 
> pm_runtime_disable() for the ones that don't.  But we also want the 
> same driver to work for all the devices, which means that 
> pm_runtime_get_sync() should return success -- otherwise the driver 
> will think that something has gone wrong.
> 
> Rafael, what do you think?

That condition is there specifically to take care of the system suspend
code path.  It means that if runtime PM is disabled, but it only has been
disabled by the system suspend code path, we should treat the device as
"active" (ie. return 1).  That won't work after the proposed change.

I guess drivers that want to work with devices where runtime PM may be
disabled can just check the return value of rpm_resume() for -EACCES?

Rafael

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