| 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
| ||
|
Message-ID: <1748065.Php9fUYGsh@vostro.rjw.lan>
Date: Wed, 07 Nov 2012 23:51:15 +0100
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Huang Ying <ying.huang@...el.com>, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org
Subject: Re: [BUGFIX] PM: Fix active child counting when disabled and forbidden
On Wednesday, November 07, 2012 04:56:49 PM Alan Stern wrote:
> On Wed, 7 Nov 2012, Rafael J. Wysocki wrote:
>
> > > Right. The reasoning behind my proposal goes like this: When there's
> > > no driver, the subsystem can let userspace directly control the
> > > device's power level through the power/control attribute.
> >
> > Well, we might as well just leave the runtime PM of PCI devices enabled, even
> > if they have no drivers, but modify the PCI bus type's runtime PM callbacks
> > to ignore devices with no drivers.
> >
> > IIRC the reason why we decided to disable runtime PM for PCI device with no
> > drivers was that some of them refused to work again after being put by the
> > core into D3. By making the PCI bus type's runtime PM callbacks ignore them
> > we'd avoid this problem without modifying the core's behavior.
>
> It comes down to a question of the parent. If a driverless PCI device
> isn't being used, shouldn't its parent be allowed to go into runtime
> suspend? As things stand now, we do allow it.
>
> The problem is that we don't disallow it when the driverless device
> _is_ being used.
We can make it depend on what's there in the control file. Let's say if that's
"on" (ie. the devices usage counter is not zero), we won't allow the parent
to be suspended.
So, as I said, why don't we keep the runtime PM of PCI devices always enabled,
regardless of whether or not there is a driver, and arrange things in such a
way that the device is automatically "suspended" if user space writes "auto"
to the control file. IOW, I suppose we can do something like this:
---
drivers/pci/pci-driver.c | 34 ++++++++++++----------------------
drivers/pci/pci.c | 2 ++
2 files changed, 14 insertions(+), 22 deletions(-)
Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -263,22 +263,17 @@ static long local_pci_probe(void *_ddi)
/* The parent bridge must be in active state when probing */
if (parent)
pm_runtime_get_sync(parent);
- /* Unbound PCI devices are always set to disabled and suspended.
- * During probe, the device is set to enabled and active and the
- * usage count is incremented. If the driver supports runtime PM,
- * it should call pm_runtime_put_noidle() in its probe routine and
+ /*
+ * During probe, the device is set to active and the usage count is
+ * incremented. If the driver supports runtime PM, it should call
+ * pm_runtime_put_noidle() in its probe routine and
* pm_runtime_get_noresume() in its remove routine.
*/
- pm_runtime_get_noresume(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
+ pm_runtime_get_sync(dev);
rc = ddi->drv->probe(ddi->dev, ddi->id);
- if (rc) {
- pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
- pm_runtime_put_noidle(dev);
- }
+ if (rc)
+ pm_runtime_put_sync(dev);
+
if (parent)
pm_runtime_put(parent);
return rc;
@@ -369,9 +364,7 @@ static int pci_device_remove(struct devi
}
/* Undo the runtime PM settings in local_pci_probe() */
- pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
- pm_runtime_put_noidle(dev);
+ pm_runtime_put_sync(dev);
/*
* If the device is still on, set the power state as "unknown",
@@ -1003,7 +996,7 @@ static int pci_pm_runtime_suspend(struct
int error;
if (!pm || !pm->runtime_suspend)
- return -ENOSYS;
+ return 0;
pci_dev->no_d3cold = false;
error = pm->runtime_suspend(dev);
@@ -1038,7 +1031,7 @@ static int pci_pm_runtime_resume(struct
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
if (!pm || !pm->runtime_resume)
- return -ENOSYS;
+ return 0;
pci_restore_standard_config(pci_dev);
pci_fixup_device(pci_fixup_resume_early, pci_dev);
@@ -1056,10 +1049,7 @@ static int pci_pm_runtime_idle(struct de
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
- if (!pm)
- return -ENOSYS;
-
- if (pm->runtime_idle) {
+ if (pm && pm->runtime_idle) {
int ret = pm->runtime_idle(dev);
if (ret)
return ret;
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1868,6 +1868,8 @@ void pci_pm_init(struct pci_dev *dev)
u16 pmc;
pm_runtime_forbid(&dev->dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(&dev->dev);
device_enable_async_suspend(&dev->dev);
dev->wakeup_prepared = false;
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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/
Powered by blists - more mailing lists