| 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: <CAAObsKDtE_uj7iA53pzMaoT+EPctCgnHxypVhXL6GuVUQimn+w@mail.gmail.com>
Date: Mon, 5 Oct 2015 10:25:45 +0200
From: Tomeu Vizoso <tomeu.vizoso@...labora.com>
To: Pavel Machek <pavel@....cz>
Cc: "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
Alan Stern <stern@...land.harvard.edu>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
martyn.welch@...labora.co.uk,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Len Brown <len.brown@...el.com>,
Kevin Hilman <khilman@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v5 1/2] PM / sleep: Go direct_complete if driver has no callbacks
On 4 October 2015 at 17:16, Pavel Machek <pavel@....cz> wrote:
> On Tue 2015-09-29 14:29:19, Tomeu Vizoso wrote:
>> If a suitable prepare callback cannot be found for a given device and
>> its driver has no PM callbacks at all, assume that it can go direct to
>> complete when the system goes to sleep.
>>
>> The reason for this is that there's lots of devices in a system that do
>> no PM at all and there's no reason for them to prevent their ancestors
>> to do direct_complete if they can support it.
>
> Dunno. This sounds like asking for trouble. Even if most devices can handle
> this, is not this bound to introduce some bugs?
Hi Pavel,
in which situations do you think that this could be problematic?
Thanks,
Tomeu
> Thanks,
> Pavel
>
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@...labora.com>
>> ---
>>
>> Changes in v5:
>> - Check for all dev_pm_ops instances associated to a device, updating a
>> no_pm_callbacks flag at the times when that could change.
>>
>> drivers/base/dd.c | 3 ++
>> drivers/base/power/domain.c | 5 ++++
>> drivers/base/power/main.c | 69 ++++++++++++++++++++++++++++++++-------------
>> drivers/base/power/power.h | 2 ++
>> include/linux/pm.h | 1 +
>> 5 files changed, 61 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index be0eb4639128..fe0e9cb684b8 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -205,6 +205,8 @@ static void driver_bound(struct device *dev)
>>
>> klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
>>
>> + device_check_pm_callbacks(dev);
>> +
>> /*
>> * Make sure the device is no longer in one of the deferred lists and
>> * kick off retrying all pending devices
>> @@ -695,6 +697,7 @@ static void __device_release_driver(struct device *dev)
>> dev->pm_domain->dismiss(dev);
>>
>> klist_remove(&dev->p->knode_driver);
>> + device_check_pm_callbacks(dev);
>> if (dev->bus)
>> blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>> BUS_NOTIFY_UNBOUND_DRIVER,
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 16550c63d611..3cae1aa1885b 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -20,6 +20,8 @@
>> #include <linux/suspend.h>
>> #include <linux/export.h>
>>
>> +#include "power.h"
>> +
>> #define GENPD_RETRY_MAX_MS 250 /* Approximate */
>>
>> #define GENPD_DEV_CALLBACK(genpd, type, callback, dev) \
>> @@ -1305,6 +1307,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>>
>> list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
>>
>> + device_check_pm_callbacks(dev);
>> out:
>> mutex_unlock(&genpd->lock);
>>
>> @@ -1369,6 +1372,8 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
>>
>> genpd_free_dev_data(dev, gpd_data);
>>
>> + device_check_pm_callbacks(dev);
>> +
>> return 0;
>>
>> out:
>> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
>> index 1710c26ba097..b6eaa00b2540 100644
>> --- a/drivers/base/power/main.c
>> +++ b/drivers/base/power/main.c
>> @@ -117,6 +117,31 @@ void device_pm_unlock(void)
>> mutex_unlock(&dpm_list_mtx);
>> }
>>
>> +static bool pm_ops_is_empty(const struct dev_pm_ops *ops)
>> +{
>> + if (!ops)
>> + return true;
>> +
>> + return !ops->prepare &&
>> + !ops->suspend &&
>> + !ops->suspend_late &&
>> + !ops->suspend_noirq &&
>> + !ops->resume_noirq &&
>> + !ops->resume_early &&
>> + !ops->resume &&
>> + !ops->complete;
>> +}
>> +
>> +void device_check_pm_callbacks(struct device *dev)
>> +{
>> + dev->power.no_pm_callbacks =
>> + (!dev->bus || pm_ops_is_empty(dev->bus->pm)) &&
>> + (!dev->class || pm_ops_is_empty(dev->class->pm)) &&
>> + (!dev->type || pm_ops_is_empty(dev->type->pm)) &&
>> + (!dev->pm_domain || pm_ops_is_empty(&dev->pm_domain->ops)) &&
>> + (!dev->driver || pm_ops_is_empty(dev->driver->pm));
>> +}
>> +
>> /**
>> * device_pm_add - Add a device to the PM core's list of active devices.
>> * @dev: Device to add to the list.
>> @@ -131,6 +156,8 @@ void device_pm_add(struct device *dev)
>> dev_name(dev->parent));
>> list_add_tail(&dev->power.entry, &dpm_list);
>> mutex_unlock(&dpm_list_mtx);
>> +
>> + device_check_pm_callbacks(dev);
>> }
>>
>> /**
>> @@ -1569,27 +1596,31 @@ static int device_prepare(struct device *dev, pm_message_t state)
>>
>> dev->power.wakeup_path = device_may_wakeup(dev);
>>
>> - if (dev->pm_domain) {
>> - info = "preparing power domain ";
>> - callback = dev->pm_domain->ops.prepare;
>> - } else if (dev->type && dev->type->pm) {
>> - info = "preparing type ";
>> - callback = dev->type->pm->prepare;
>> - } else if (dev->class && dev->class->pm) {
>> - info = "preparing class ";
>> - callback = dev->class->pm->prepare;
>> - } else if (dev->bus && dev->bus->pm) {
>> - info = "preparing bus ";
>> - callback = dev->bus->pm->prepare;
>> - }
>> + if (dev->power.no_pm_callbacks)
>> + ret = 1; /* Let device go direct_complete */
>> + else {
>> + if (dev->pm_domain) {
>> + info = "preparing power domain ";
>> + callback = dev->pm_domain->ops.prepare;
>> + } else if (dev->type && dev->type->pm) {
>> + info = "preparing type ";
>> + callback = dev->type->pm->prepare;
>> + } else if (dev->class && dev->class->pm) {
>> + info = "preparing class ";
>> + callback = dev->class->pm->prepare;
>> + } else if (dev->bus && dev->bus->pm) {
>> + info = "preparing bus ";
>> + callback = dev->bus->pm->prepare;
>> + }
>>
>> - if (!callback && dev->driver && dev->driver->pm) {
>> - info = "preparing driver ";
>> - callback = dev->driver->pm->prepare;
>> - }
>> + if (!callback && dev->driver && dev->driver->pm) {
>> + info = "preparing driver ";
>> + callback = dev->driver->pm->prepare;
>> + }
>>
>> - if (callback)
>> - ret = callback(dev);
>> + if (callback)
>> + ret = callback(dev);
>> + }
>>
>> device_unlock(dev);
>>
>> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
>> index 998fa6b23084..518a8fc84e8d 100644
>> --- a/drivers/base/power/power.h
>> +++ b/drivers/base/power/power.h
>> @@ -9,6 +9,8 @@ static inline void device_pm_init_common(struct device *dev)
>> }
>> }
>>
>> +extern void device_check_pm_callbacks(struct device *dev);
>> +
>> #ifdef CONFIG_PM
>>
>> static inline void pm_runtime_early_init(struct device *dev)
>> diff --git a/include/linux/pm.h b/include/linux/pm.h
>> index 35d599e7250d..e334b9b8cd46 100644
>> --- a/include/linux/pm.h
>> +++ b/include/linux/pm.h
>> @@ -566,6 +566,7 @@ struct dev_pm_info {
>> bool ignore_children:1;
>> bool early_init:1; /* Owned by the PM core */
>> bool direct_complete:1; /* Owned by the PM core */
>> + bool no_pm_callbacks:1; /* Owned by the PM core */
>> spinlock_t lock;
>> #ifdef CONFIG_PM_SLEEP
>> struct list_head entry;
>> --
>> 2.4.3
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> --
> 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/
--
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