| Message ID | 2054600.p9BxnU7SDF@aspire.rjw.lan (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 2017-11-02 at 00:03:54 +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The special value of 0 for device resume latency PM QoS means > "no restriction", but there are two problems with that. > > First, device resume latency PM QoS requests with 0 as the > value are always put in front of requests with positive > values in the priority lists used internally by the PM QoS > framework, causing 0 to be chosen as an effective constraint > value. However, that 0 is then interpreted as "no restriction" > effectively overriding the other requests with specific > restrictions which is incorrect. > > Second, the users of device resume latency PM QoS have no > way to specify that *any* resume latency at all should be > avoided, which is an artificial limitation in general. > > To address these issues, modify device resume latency PM QoS to > use S32_MAX as the "no constraint" value and 0 as the "no > latency at all" one and rework its users (the cpuidle menu > governor, the genpd QoS governor and the runtime PM framework) > to follow these changes. > > Also add a special "n/a" value to the corresponding user space I/F > to allow user space to indicate that it cannot accept any resume > latencies at all for the given device. > > Fixes: 85dc0b8a4019 (PM / QoS: Make it possible to expose PM QoS latency constraints) > Link: https://bugzilla.kernel.org/show_bug.cgi?id=197323 > Reported-by: Reinette Chatre <reinette.chatre@intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > Documentation/ABI/testing/sysfs-devices-power | 4 ++- > drivers/base/cpu.c | 3 +- > drivers/base/power/domain.c | 2 - > drivers/base/power/domain_governor.c | 27 ++++++++++++-------------- > drivers/base/power/qos.c | 5 +++- > drivers/base/power/runtime.c | 2 - > drivers/base/power/sysfs.c | 25 ++++++++++++++++++++---- > drivers/cpuidle/governors/menu.c | 4 +-- > include/linux/pm_qos.h | 24 +++++++++++++++-------- > 9 files changed, 63 insertions(+), 33 deletions(-) > > Index: linux-pm/drivers/base/power/sysfs.c > =================================================================== > --- linux-pm.orig/drivers/base/power/sysfs.c > +++ linux-pm/drivers/base/power/sysfs.c > @@ -218,7 +218,14 @@ static ssize_t pm_qos_resume_latency_sho > struct device_attribute *attr, > char *buf) > { > - return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev)); > + s32 value = dev_pm_qos_requested_resume_latency(dev); > + > + if (value == 0) > + return sprintf(buf, "n/a\n"); > + else if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) > + value = 0; > + > + return sprintf(buf, "%d\n", value); > } > > static ssize_t pm_qos_resume_latency_store(struct device *dev, > @@ -228,11 +235,21 @@ static ssize_t pm_qos_resume_latency_sto > s32 value; > int ret; > > - if (kstrtos32(buf, 0, &value)) > - return -EINVAL; > + if (!kstrtos32(buf, 0, &value)) { > + /* > + * Prevent users from writing negative or "no constraint" values > + * directly. > + */ > + if (value < 0 || value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) > + return -EINVAL; > > - if (value < 0) > + if (value == 0) > + value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; > + } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) { > + value = 0; > + } else { > return -EINVAL; > + } > > ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req, > value); > Index: linux-pm/include/linux/pm_qos.h > =================================================================== > --- linux-pm.orig/include/linux/pm_qos.h > +++ linux-pm/include/linux/pm_qos.h > @@ -27,16 +27,17 @@ enum pm_qos_flags_status { > PM_QOS_FLAGS_ALL, > }; > > -#define PM_QOS_DEFAULT_VALUE -1 > +#define PM_QOS_DEFAULT_VALUE (-1) > +#define PM_QOS_LATENCY_ANY S32_MAX > > #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) > #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) > #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE 0 > #define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE 0 > -#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE 0 > +#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE PM_QOS_LATENCY_ANY > +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY > #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0 > #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1) > -#define PM_QOS_LATENCY_ANY ((s32)(~(__u32)0 >> 1)) > > #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0) > > @@ -173,7 +174,8 @@ static inline s32 dev_pm_qos_requested_f > static inline s32 dev_pm_qos_raw_read_value(struct device *dev) > { > return IS_ERR_OR_NULL(dev->power.qos) ? > - 0 : pm_qos_read_value(&dev->power.qos->resume_latency); > + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT : > + pm_qos_read_value(&dev->power.qos->resume_latency); > } > #else > static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, > @@ -183,9 +185,9 @@ static inline enum pm_qos_flags_status d > s32 mask) > { return PM_QOS_FLAGS_UNDEFINED; } > static inline s32 __dev_pm_qos_read_value(struct device *dev) > - { return 0; } > + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } > static inline s32 dev_pm_qos_read_value(struct device *dev) > - { return 0; } > + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } > static inline int dev_pm_qos_add_request(struct device *dev, > struct dev_pm_qos_request *req, > enum dev_pm_qos_req_type type, > @@ -231,9 +233,15 @@ static inline int dev_pm_qos_expose_late > { return 0; } > static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {} > > -static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) { return 0; } > +static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) > +{ > + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; > +} > static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; } > -static inline s32 dev_pm_qos_raw_read_value(struct device *dev) { return 0; } > +static inline s32 dev_pm_qos_raw_read_value(struct device *dev) > +{ > + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; > +} > #endif > > #endif > Index: linux-pm/drivers/cpuidle/governors/menu.c > =================================================================== > --- linux-pm.orig/drivers/cpuidle/governors/menu.c > +++ linux-pm/drivers/cpuidle/governors/menu.c > @@ -298,8 +298,8 @@ static int menu_select(struct cpuidle_dr > data->needs_update = 0; > } > > - /* resume_latency is 0 means no restriction */ > - if (resume_latency && resume_latency < latency_req) > + if (resume_latency < latency_req && > + resume_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) > latency_req = resume_latency; > > /* Special case when user has set very strict latency requirement */ > Index: linux-pm/drivers/base/power/runtime.c > =================================================================== > --- linux-pm.orig/drivers/base/power/runtime.c > +++ linux-pm/drivers/base/power/runtime.c > @@ -253,7 +253,7 @@ static int rpm_check_suspend_allowed(str > || (dev->power.request_pending > && dev->power.request == RPM_REQ_RESUME)) > retval = -EAGAIN; > - else if (__dev_pm_qos_read_value(dev) < 0) > + else if (__dev_pm_qos_read_value(dev) == 0) > retval = -EPERM; > else if (dev->power.runtime_status == RPM_SUSPENDED) > retval = 1; > Index: linux-pm/drivers/base/cpu.c > =================================================================== > --- linux-pm.orig/drivers/base/cpu.c > +++ linux-pm/drivers/base/cpu.c > @@ -377,7 +377,8 @@ int register_cpu(struct cpu *cpu, int nu > > per_cpu(cpu_sys_devices, num) = &cpu->dev; > register_cpu_under_node(num, cpu_to_node(num)); > - dev_pm_qos_expose_latency_limit(&cpu->dev, 0); > + dev_pm_qos_expose_latency_limit(&cpu->dev, > + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT); > > return 0; > } > Index: linux-pm/drivers/base/power/qos.c > =================================================================== > --- linux-pm.orig/drivers/base/power/qos.c > +++ linux-pm/drivers/base/power/qos.c > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p > > switch(req->type) { > case DEV_PM_QOS_RESUME_LATENCY: > + if (WARN_ON(value < 0)) > + value = 0; > + > ret = pm_qos_update_target(&qos->resume_latency, > &req->data.pnode, action, value); > break; > @@ -189,7 +192,7 @@ static int dev_pm_qos_constraints_alloca > plist_head_init(&c->list); > c->target_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; > c->default_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; > - c->no_constraint_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; > + c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; > c->type = PM_QOS_MIN; > c->notifiers = n; > > Index: linux-pm/Documentation/ABI/testing/sysfs-devices-power > =================================================================== > --- linux-pm.orig/Documentation/ABI/testing/sysfs-devices-power > +++ linux-pm/Documentation/ABI/testing/sysfs-devices-power > @@ -211,7 +211,9 @@ Description: > device, after it has been suspended at run time, from a resume > request to the moment the device will be ready to process I/O, > in microseconds. If it is equal to 0, however, this means that > - the PM QoS resume latency may be arbitrary. > + the PM QoS resume latency may be arbitrary and the special value > + "n/a" means that user space cannot accept any resume latency at > + all for the given device. > > Not all drivers support this attribute. If it isn't supported, > it is not present. > Index: linux-pm/drivers/base/power/domain.c > =================================================================== > --- linux-pm.orig/drivers/base/power/domain.c > +++ linux-pm/drivers/base/power/domain.c > @@ -1331,7 +1331,7 @@ static struct generic_pm_domain_data *ge > > gpd_data->base.dev = dev; > gpd_data->td.constraint_changed = true; > - gpd_data->td.effective_constraint_ns = 0; > + gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; > gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; > > spin_lock_irq(&dev->power.lock); > Index: linux-pm/drivers/base/power/domain_governor.c > =================================================================== > --- linux-pm.orig/drivers/base/power/domain_governor.c > +++ linux-pm/drivers/base/power/domain_governor.c > @@ -26,11 +26,11 @@ static int dev_update_qos_constraint(str > * to take effect, the device has to be resumed and suspended again. > */ > constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns; > - /* 0 means "no constraint" */ > - if (constraint_ns == 0) > + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) > return 0; > > - if (constraint_ns < *constraint_ns_p || *constraint_ns_p == 0) > + if (constraint_ns < *constraint_ns_p || > + *constraint_ns_p == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) Check for constraint_ns < *constraint_ns_p should be enough because PM_QOS_RESUME_LATENCY_NO_CONSTRAINT = S32_MAX which is the largest possible value, if that can be assumed here. > *constraint_ns_p = constraint_ns; > > return 0; > @@ -58,12 +58,12 @@ static bool default_suspend_ok(struct de > } > td->constraint_changed = false; > td->cached_suspend_ok = false; > - td->effective_constraint_ns = -1; > + td->effective_constraint_ns = 0; > constraint_ns = __dev_pm_qos_read_value(dev); Why not initialize td->effective_constraint_ns with what is read by __dev_pm_qos_read_value()? Even though this value will stay only if false is returned, in which case it does not matter anyway, it looks like more meaningful value. Also PM_QOS_RESUME_LATENCY_NO_CONSTRAINT may be ok as it is initialized with at creation. > > spin_unlock_irqrestore(&dev->power.lock, flags); > > - if (constraint_ns < 0) > + if (constraint_ns == 0) > return false; > > constraint_ns *= NSEC_PER_USEC; This will go wrong if PM_QOS_RESUME_LATENCY_NO_CONSTRAINT was the value Code below and in dev_update_qos_constraint check for that special value. May be those places can check PM_QOS_RESUME_LATENCY_NO_CONSTRAINT * NSEC_PER_USEC There may be other places that this might be an issue > @@ -76,14 +76,14 @@ static bool default_suspend_ok(struct de > device_for_each_child(dev, &constraint_ns, > dev_update_qos_constraint); > > - if (constraint_ns == 0) { > + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) { > /* "No restriction", so the device is allowed to suspend. */ > - td->effective_constraint_ns = 0; > + td->effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; > td->cached_suspend_ok = true; > - } else if (WARN_ON(constraint_ns < 0)) { > + } else if (WARN_ON(constraint_ns == 0)) { This can never happen as this device will not be suspending if the child had failed to suspend (only case where this value is possible). > /* > - * effective_constraint_ns is negative already and > - * cached_suspend_ok is false, so return right away. > + * effective_constraint_ns is 0 already and cached_suspend_ok is > + * false, so return right away. > */ > return false; > } else { > @@ -156,12 +156,11 @@ static bool __default_power_down_ok(stru > */ > td = &to_gpd_data(pdd)->td; > constraint_ns = td->effective_constraint_ns; > - /* Negative values mean "no suspend at all" */ > - if (WARN_ON(constraint_ns < 0)) > + /* 0 mean "no suspend at all" */ > + if (WARN_ON(constraint_ns == 0)) > return false; If it did not suspend then it cannot powerdown. This check may not be very useful here. > > - /* 0 means "no constraint" */ > - if (constraint_ns == 0) > + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) > continue; > > if (constraint_ns <= off_on_time_ns) >
On Fri, Nov 3, 2017 at 8:43 AM, Ramesh Thomas <ramesh.thomas@intel.com> wrote: > On 2017-11-02 at 00:03:54 +0100, Rafael J. Wysocki wrote: >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> >> The special value of 0 for device resume latency PM QoS means >> "no restriction", but there are two problems with that. >> >> First, device resume latency PM QoS requests with 0 as the >> value are always put in front of requests with positive >> values in the priority lists used internally by the PM QoS >> framework, causing 0 to be chosen as an effective constraint >> value. However, that 0 is then interpreted as "no restriction" >> effectively overriding the other requests with specific >> restrictions which is incorrect. >> >> Second, the users of device resume latency PM QoS have no >> way to specify that *any* resume latency at all should be >> avoided, which is an artificial limitation in general. >> >> To address these issues, modify device resume latency PM QoS to >> use S32_MAX as the "no constraint" value and 0 as the "no >> latency at all" one and rework its users (the cpuidle menu >> governor, the genpd QoS governor and the runtime PM framework) >> to follow these changes. >> >> Also add a special "n/a" value to the corresponding user space I/F >> to allow user space to indicate that it cannot accept any resume >> latencies at all for the given device. >> >> Fixes: 85dc0b8a4019 (PM / QoS: Make it possible to expose PM QoS latency constraints) >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197323 >> Reported-by: Reinette Chatre <reinette.chatre@intel.com> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> --- >> Documentation/ABI/testing/sysfs-devices-power | 4 ++- >> drivers/base/cpu.c | 3 +- >> drivers/base/power/domain.c | 2 - >> drivers/base/power/domain_governor.c | 27 ++++++++++++-------------- >> drivers/base/power/qos.c | 5 +++- >> drivers/base/power/runtime.c | 2 - >> drivers/base/power/sysfs.c | 25 ++++++++++++++++++++---- >> drivers/cpuidle/governors/menu.c | 4 +-- >> include/linux/pm_qos.h | 24 +++++++++++++++-------- >> 9 files changed, 63 insertions(+), 33 deletions(-) >> >> Index: linux-pm/drivers/base/power/sysfs.c >> =================================================================== >> --- linux-pm.orig/drivers/base/power/sysfs.c >> +++ linux-pm/drivers/base/power/sysfs.c >> @@ -218,7 +218,14 @@ static ssize_t pm_qos_resume_latency_sho >> struct device_attribute *attr, >> char *buf) >> { >> - return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev)); >> + s32 value = dev_pm_qos_requested_resume_latency(dev); >> + >> + if (value == 0) >> + return sprintf(buf, "n/a\n"); >> + else if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >> + value = 0; >> + >> + return sprintf(buf, "%d\n", value); >> } >> >> static ssize_t pm_qos_resume_latency_store(struct device *dev, >> @@ -228,11 +235,21 @@ static ssize_t pm_qos_resume_latency_sto >> s32 value; >> int ret; >> >> - if (kstrtos32(buf, 0, &value)) >> - return -EINVAL; >> + if (!kstrtos32(buf, 0, &value)) { >> + /* >> + * Prevent users from writing negative or "no constraint" values >> + * directly. >> + */ >> + if (value < 0 || value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >> + return -EINVAL; >> >> - if (value < 0) >> + if (value == 0) >> + value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> + } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) { >> + value = 0; >> + } else { >> return -EINVAL; >> + } >> >> ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req, >> value); >> Index: linux-pm/include/linux/pm_qos.h >> =================================================================== >> --- linux-pm.orig/include/linux/pm_qos.h >> +++ linux-pm/include/linux/pm_qos.h >> @@ -27,16 +27,17 @@ enum pm_qos_flags_status { >> PM_QOS_FLAGS_ALL, >> }; >> >> -#define PM_QOS_DEFAULT_VALUE -1 >> +#define PM_QOS_DEFAULT_VALUE (-1) >> +#define PM_QOS_LATENCY_ANY S32_MAX >> >> #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) >> #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) >> #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE 0 >> #define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE 0 >> -#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE 0 >> +#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE PM_QOS_LATENCY_ANY >> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY >> #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0 >> #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1) >> -#define PM_QOS_LATENCY_ANY ((s32)(~(__u32)0 >> 1)) >> >> #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0) >> >> @@ -173,7 +174,8 @@ static inline s32 dev_pm_qos_requested_f >> static inline s32 dev_pm_qos_raw_read_value(struct device *dev) >> { >> return IS_ERR_OR_NULL(dev->power.qos) ? >> - 0 : pm_qos_read_value(&dev->power.qos->resume_latency); >> + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT : >> + pm_qos_read_value(&dev->power.qos->resume_latency); >> } >> #else >> static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, >> @@ -183,9 +185,9 @@ static inline enum pm_qos_flags_status d >> s32 mask) >> { return PM_QOS_FLAGS_UNDEFINED; } >> static inline s32 __dev_pm_qos_read_value(struct device *dev) >> - { return 0; } >> + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } >> static inline s32 dev_pm_qos_read_value(struct device *dev) >> - { return 0; } >> + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } >> static inline int dev_pm_qos_add_request(struct device *dev, >> struct dev_pm_qos_request *req, >> enum dev_pm_qos_req_type type, >> @@ -231,9 +233,15 @@ static inline int dev_pm_qos_expose_late >> { return 0; } >> static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {} >> >> -static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) { return 0; } >> +static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) >> +{ >> + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> +} >> static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; } >> -static inline s32 dev_pm_qos_raw_read_value(struct device *dev) { return 0; } >> +static inline s32 dev_pm_qos_raw_read_value(struct device *dev) >> +{ >> + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> +} >> #endif >> >> #endif >> Index: linux-pm/drivers/cpuidle/governors/menu.c >> =================================================================== >> --- linux-pm.orig/drivers/cpuidle/governors/menu.c >> +++ linux-pm/drivers/cpuidle/governors/menu.c >> @@ -298,8 +298,8 @@ static int menu_select(struct cpuidle_dr >> data->needs_update = 0; >> } >> >> - /* resume_latency is 0 means no restriction */ >> - if (resume_latency && resume_latency < latency_req) >> + if (resume_latency < latency_req && >> + resume_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >> latency_req = resume_latency; >> >> /* Special case when user has set very strict latency requirement */ >> Index: linux-pm/drivers/base/power/runtime.c >> =================================================================== >> --- linux-pm.orig/drivers/base/power/runtime.c >> +++ linux-pm/drivers/base/power/runtime.c >> @@ -253,7 +253,7 @@ static int rpm_check_suspend_allowed(str >> || (dev->power.request_pending >> && dev->power.request == RPM_REQ_RESUME)) >> retval = -EAGAIN; >> - else if (__dev_pm_qos_read_value(dev) < 0) >> + else if (__dev_pm_qos_read_value(dev) == 0) >> retval = -EPERM; >> else if (dev->power.runtime_status == RPM_SUSPENDED) >> retval = 1; >> Index: linux-pm/drivers/base/cpu.c >> =================================================================== >> --- linux-pm.orig/drivers/base/cpu.c >> +++ linux-pm/drivers/base/cpu.c >> @@ -377,7 +377,8 @@ int register_cpu(struct cpu *cpu, int nu >> >> per_cpu(cpu_sys_devices, num) = &cpu->dev; >> register_cpu_under_node(num, cpu_to_node(num)); >> - dev_pm_qos_expose_latency_limit(&cpu->dev, 0); >> + dev_pm_qos_expose_latency_limit(&cpu->dev, >> + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT); >> >> return 0; >> } >> Index: linux-pm/drivers/base/power/qos.c >> =================================================================== >> --- linux-pm.orig/drivers/base/power/qos.c >> +++ linux-pm/drivers/base/power/qos.c >> @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p >> >> switch(req->type) { >> case DEV_PM_QOS_RESUME_LATENCY: >> + if (WARN_ON(value < 0)) >> + value = 0; >> + >> ret = pm_qos_update_target(&qos->resume_latency, >> &req->data.pnode, action, value); >> break; >> @@ -189,7 +192,7 @@ static int dev_pm_qos_constraints_alloca >> plist_head_init(&c->list); >> c->target_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; >> c->default_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; >> - c->no_constraint_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; >> + c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> c->type = PM_QOS_MIN; >> c->notifiers = n; >> >> Index: linux-pm/Documentation/ABI/testing/sysfs-devices-power >> =================================================================== >> --- linux-pm.orig/Documentation/ABI/testing/sysfs-devices-power >> +++ linux-pm/Documentation/ABI/testing/sysfs-devices-power >> @@ -211,7 +211,9 @@ Description: >> device, after it has been suspended at run time, from a resume >> request to the moment the device will be ready to process I/O, >> in microseconds. If it is equal to 0, however, this means that >> - the PM QoS resume latency may be arbitrary. >> + the PM QoS resume latency may be arbitrary and the special value >> + "n/a" means that user space cannot accept any resume latency at >> + all for the given device. >> >> Not all drivers support this attribute. If it isn't supported, >> it is not present. >> Index: linux-pm/drivers/base/power/domain.c >> =================================================================== >> --- linux-pm.orig/drivers/base/power/domain.c >> +++ linux-pm/drivers/base/power/domain.c >> @@ -1331,7 +1331,7 @@ static struct generic_pm_domain_data *ge >> >> gpd_data->base.dev = dev; >> gpd_data->td.constraint_changed = true; >> - gpd_data->td.effective_constraint_ns = 0; >> + gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; >> >> spin_lock_irq(&dev->power.lock); >> Index: linux-pm/drivers/base/power/domain_governor.c >> =================================================================== >> --- linux-pm.orig/drivers/base/power/domain_governor.c >> +++ linux-pm/drivers/base/power/domain_governor.c >> @@ -26,11 +26,11 @@ static int dev_update_qos_constraint(str >> * to take effect, the device has to be resumed and suspended again. >> */ >> constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns; >> - /* 0 means "no constraint" */ >> - if (constraint_ns == 0) >> + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >> return 0; >> >> - if (constraint_ns < *constraint_ns_p || *constraint_ns_p == 0) >> + if (constraint_ns < *constraint_ns_p || >> + *constraint_ns_p == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) > > Check for constraint_ns < *constraint_ns_p should be enough because > PM_QOS_RESUME_LATENCY_NO_CONSTRAINT = S32_MAX which is the largest possible > value, if that can be assumed here. Well, not quite. effective_constraint_ns is s64 and it is the QoS value multiplied by NSEC_PER_USEC. Surely can be greater than PM_QOS_RESUME_LATENCY_NO_CONSTRAINT. Which reminds me about the possible collision in default_suspend_ok() which needs to be addressed. :-) >> *constraint_ns_p = constraint_ns; >> >> return 0; >> @@ -58,12 +58,12 @@ static bool default_suspend_ok(struct de >> } >> td->constraint_changed = false; >> td->cached_suspend_ok = false; >> - td->effective_constraint_ns = -1; >> + td->effective_constraint_ns = 0; >> constraint_ns = __dev_pm_qos_read_value(dev); > > Why not initialize td->effective_constraint_ns with what is read by > __dev_pm_qos_read_value()? Even though this value will stay only if false > is returned, in which case it does not matter anyway, it looks like more > meaningful value. Also PM_QOS_RESUME_LATENCY_NO_CONSTRAINT may be ok as it > is initialized with at creation. This looks like a leftover or similar, thanks. >> >> spin_unlock_irqrestore(&dev->power.lock, flags); >> >> - if (constraint_ns < 0) >> + if (constraint_ns == 0) >> return false; >> >> constraint_ns *= NSEC_PER_USEC; > > This will go wrong if PM_QOS_RESUME_LATENCY_NO_CONSTRAINT was the value > Code below and in dev_update_qos_constraint check for that special value. > > May be those places can check PM_QOS_RESUME_LATENCY_NO_CONSTRAINT * NSEC_PER_USEC > There may be other places that this might be an issue Right, I forgot about this one. >> @@ -76,14 +76,14 @@ static bool default_suspend_ok(struct de >> device_for_each_child(dev, &constraint_ns, >> dev_update_qos_constraint); >> >> - if (constraint_ns == 0) { >> + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) { >> /* "No restriction", so the device is allowed to suspend. */ >> - td->effective_constraint_ns = 0; >> + td->effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> td->cached_suspend_ok = true; >> - } else if (WARN_ON(constraint_ns < 0)) { >> + } else if (WARN_ON(constraint_ns == 0)) { > > This can never happen as this device will not be suspending if the child had > failed to suspend (only case where this value is possible). > >> /* >> - * effective_constraint_ns is negative already and >> - * cached_suspend_ok is false, so return right away. >> + * effective_constraint_ns is 0 already and cached_suspend_ok is >> + * false, so return right away. >> */ >> return false; >> } else { >> @@ -156,12 +156,11 @@ static bool __default_power_down_ok(stru >> */ >> td = &to_gpd_data(pdd)->td; >> constraint_ns = td->effective_constraint_ns; >> - /* Negative values mean "no suspend at all" */ >> - if (WARN_ON(constraint_ns < 0)) >> + /* 0 mean "no suspend at all" */ >> + if (WARN_ON(constraint_ns == 0)) >> return false; > > If it did not suspend then it cannot powerdown. This check may not be > very useful here. The checks with WARN_ON() are there to catch the unexpected. They aren't meant to trigger at all under normal conditions. Thanks, Rafael
On Fri, Nov 3, 2017 at 8:58 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Fri, Nov 3, 2017 at 8:43 AM, Ramesh Thomas <ramesh.thomas@intel.com> wrote: >> On 2017-11-02 at 00:03:54 +0100, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> The special value of 0 for device resume latency PM QoS means >>> "no restriction", but there are two problems with that. >>> >>> First, device resume latency PM QoS requests with 0 as the >>> value are always put in front of requests with positive >>> values in the priority lists used internally by the PM QoS >>> framework, causing 0 to be chosen as an effective constraint >>> value. However, that 0 is then interpreted as "no restriction" >>> effectively overriding the other requests with specific >>> restrictions which is incorrect. >>> >>> Second, the users of device resume latency PM QoS have no >>> way to specify that *any* resume latency at all should be >>> avoided, which is an artificial limitation in general. >>> >>> To address these issues, modify device resume latency PM QoS to >>> use S32_MAX as the "no constraint" value and 0 as the "no >>> latency at all" one and rework its users (the cpuidle menu >>> governor, the genpd QoS governor and the runtime PM framework) >>> to follow these changes. >>> >>> Also add a special "n/a" value to the corresponding user space I/F >>> to allow user space to indicate that it cannot accept any resume >>> latencies at all for the given device. >>> >>> Fixes: 85dc0b8a4019 (PM / QoS: Make it possible to expose PM QoS latency constraints) >>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197323 >>> Reported-by: Reinette Chatre <reinette.chatre@intel.com> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> --- >>> Documentation/ABI/testing/sysfs-devices-power | 4 ++- >>> drivers/base/cpu.c | 3 +- >>> drivers/base/power/domain.c | 2 - >>> drivers/base/power/domain_governor.c | 27 ++++++++++++-------------- >>> drivers/base/power/qos.c | 5 +++- >>> drivers/base/power/runtime.c | 2 - >>> drivers/base/power/sysfs.c | 25 ++++++++++++++++++++---- >>> drivers/cpuidle/governors/menu.c | 4 +-- >>> include/linux/pm_qos.h | 24 +++++++++++++++-------- >>> 9 files changed, 63 insertions(+), 33 deletions(-) >>> >>> Index: linux-pm/drivers/base/power/sysfs.c >>> =================================================================== >>> --- linux-pm.orig/drivers/base/power/sysfs.c >>> +++ linux-pm/drivers/base/power/sysfs.c >>> @@ -218,7 +218,14 @@ static ssize_t pm_qos_resume_latency_sho >>> struct device_attribute *attr, >>> char *buf) >>> { >>> - return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev)); >>> + s32 value = dev_pm_qos_requested_resume_latency(dev); >>> + >>> + if (value == 0) >>> + return sprintf(buf, "n/a\n"); >>> + else if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >>> + value = 0; >>> + >>> + return sprintf(buf, "%d\n", value); >>> } >>> >>> static ssize_t pm_qos_resume_latency_store(struct device *dev, >>> @@ -228,11 +235,21 @@ static ssize_t pm_qos_resume_latency_sto >>> s32 value; >>> int ret; >>> >>> - if (kstrtos32(buf, 0, &value)) >>> - return -EINVAL; >>> + if (!kstrtos32(buf, 0, &value)) { >>> + /* >>> + * Prevent users from writing negative or "no constraint" values >>> + * directly. >>> + */ >>> + if (value < 0 || value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >>> + return -EINVAL; >>> >>> - if (value < 0) >>> + if (value == 0) >>> + value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >>> + } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) { >>> + value = 0; >>> + } else { >>> return -EINVAL; >>> + } >>> >>> ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req, >>> value); >>> Index: linux-pm/include/linux/pm_qos.h >>> =================================================================== >>> --- linux-pm.orig/include/linux/pm_qos.h >>> +++ linux-pm/include/linux/pm_qos.h >>> @@ -27,16 +27,17 @@ enum pm_qos_flags_status { >>> PM_QOS_FLAGS_ALL, >>> }; >>> >>> -#define PM_QOS_DEFAULT_VALUE -1 >>> +#define PM_QOS_DEFAULT_VALUE (-1) >>> +#define PM_QOS_LATENCY_ANY S32_MAX >>> >>> #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) >>> #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) >>> #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE 0 >>> #define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE 0 >>> -#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE 0 >>> +#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE PM_QOS_LATENCY_ANY >>> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY >>> #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0 >>> #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1) >>> -#define PM_QOS_LATENCY_ANY ((s32)(~(__u32)0 >> 1)) >>> >>> #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0) >>> >>> @@ -173,7 +174,8 @@ static inline s32 dev_pm_qos_requested_f >>> static inline s32 dev_pm_qos_raw_read_value(struct device *dev) >>> { >>> return IS_ERR_OR_NULL(dev->power.qos) ? >>> - 0 : pm_qos_read_value(&dev->power.qos->resume_latency); >>> + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT : >>> + pm_qos_read_value(&dev->power.qos->resume_latency); >>> } >>> #else >>> static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, >>> @@ -183,9 +185,9 @@ static inline enum pm_qos_flags_status d >>> s32 mask) >>> { return PM_QOS_FLAGS_UNDEFINED; } >>> static inline s32 __dev_pm_qos_read_value(struct device *dev) >>> - { return 0; } >>> + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } >>> static inline s32 dev_pm_qos_read_value(struct device *dev) >>> - { return 0; } >>> + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } >>> static inline int dev_pm_qos_add_request(struct device *dev, >>> struct dev_pm_qos_request *req, >>> enum dev_pm_qos_req_type type, >>> @@ -231,9 +233,15 @@ static inline int dev_pm_qos_expose_late >>> { return 0; } >>> static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {} >>> >>> -static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) { return 0; } >>> +static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) >>> +{ >>> + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >>> +} >>> static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; } >>> -static inline s32 dev_pm_qos_raw_read_value(struct device *dev) { return 0; } >>> +static inline s32 dev_pm_qos_raw_read_value(struct device *dev) >>> +{ >>> + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >>> +} >>> #endif >>> >>> #endif >>> Index: linux-pm/drivers/cpuidle/governors/menu.c >>> =================================================================== >>> --- linux-pm.orig/drivers/cpuidle/governors/menu.c >>> +++ linux-pm/drivers/cpuidle/governors/menu.c >>> @@ -298,8 +298,8 @@ static int menu_select(struct cpuidle_dr >>> data->needs_update = 0; >>> } >>> >>> - /* resume_latency is 0 means no restriction */ >>> - if (resume_latency && resume_latency < latency_req) >>> + if (resume_latency < latency_req && >>> + resume_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >>> latency_req = resume_latency; >>> >>> /* Special case when user has set very strict latency requirement */ >>> Index: linux-pm/drivers/base/power/runtime.c >>> =================================================================== >>> --- linux-pm.orig/drivers/base/power/runtime.c >>> +++ linux-pm/drivers/base/power/runtime.c >>> @@ -253,7 +253,7 @@ static int rpm_check_suspend_allowed(str >>> || (dev->power.request_pending >>> && dev->power.request == RPM_REQ_RESUME)) >>> retval = -EAGAIN; >>> - else if (__dev_pm_qos_read_value(dev) < 0) >>> + else if (__dev_pm_qos_read_value(dev) == 0) >>> retval = -EPERM; >>> else if (dev->power.runtime_status == RPM_SUSPENDED) >>> retval = 1; >>> Index: linux-pm/drivers/base/cpu.c >>> =================================================================== >>> --- linux-pm.orig/drivers/base/cpu.c >>> +++ linux-pm/drivers/base/cpu.c >>> @@ -377,7 +377,8 @@ int register_cpu(struct cpu *cpu, int nu >>> >>> per_cpu(cpu_sys_devices, num) = &cpu->dev; >>> register_cpu_under_node(num, cpu_to_node(num)); >>> - dev_pm_qos_expose_latency_limit(&cpu->dev, 0); >>> + dev_pm_qos_expose_latency_limit(&cpu->dev, >>> + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT); >>> >>> return 0; >>> } >>> Index: linux-pm/drivers/base/power/qos.c >>> =================================================================== >>> --- linux-pm.orig/drivers/base/power/qos.c >>> +++ linux-pm/drivers/base/power/qos.c >>> @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p >>> >>> switch(req->type) { >>> case DEV_PM_QOS_RESUME_LATENCY: >>> + if (WARN_ON(value < 0)) >>> + value = 0; >>> + >>> ret = pm_qos_update_target(&qos->resume_latency, >>> &req->data.pnode, action, value); >>> break; >>> @@ -189,7 +192,7 @@ static int dev_pm_qos_constraints_alloca >>> plist_head_init(&c->list); >>> c->target_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; >>> c->default_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; >>> - c->no_constraint_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; >>> + c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >>> c->type = PM_QOS_MIN; >>> c->notifiers = n; >>> >>> Index: linux-pm/Documentation/ABI/testing/sysfs-devices-power >>> =================================================================== >>> --- linux-pm.orig/Documentation/ABI/testing/sysfs-devices-power >>> +++ linux-pm/Documentation/ABI/testing/sysfs-devices-power >>> @@ -211,7 +211,9 @@ Description: >>> device, after it has been suspended at run time, from a resume >>> request to the moment the device will be ready to process I/O, >>> in microseconds. If it is equal to 0, however, this means that >>> - the PM QoS resume latency may be arbitrary. >>> + the PM QoS resume latency may be arbitrary and the special value >>> + "n/a" means that user space cannot accept any resume latency at >>> + all for the given device. >>> >>> Not all drivers support this attribute. If it isn't supported, >>> it is not present. >>> Index: linux-pm/drivers/base/power/domain.c >>> =================================================================== >>> --- linux-pm.orig/drivers/base/power/domain.c >>> +++ linux-pm/drivers/base/power/domain.c >>> @@ -1331,7 +1331,7 @@ static struct generic_pm_domain_data *ge >>> >>> gpd_data->base.dev = dev; >>> gpd_data->td.constraint_changed = true; >>> - gpd_data->td.effective_constraint_ns = 0; >>> + gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >>> gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; >>> >>> spin_lock_irq(&dev->power.lock); >>> Index: linux-pm/drivers/base/power/domain_governor.c >>> =================================================================== >>> --- linux-pm.orig/drivers/base/power/domain_governor.c >>> +++ linux-pm/drivers/base/power/domain_governor.c >>> @@ -26,11 +26,11 @@ static int dev_update_qos_constraint(str >>> * to take effect, the device has to be resumed and suspended again. >>> */ >>> constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns; >>> - /* 0 means "no constraint" */ >>> - if (constraint_ns == 0) >>> + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >>> return 0; >>> >>> - if (constraint_ns < *constraint_ns_p || *constraint_ns_p == 0) >>> + if (constraint_ns < *constraint_ns_p || >>> + *constraint_ns_p == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) >> >> Check for constraint_ns < *constraint_ns_p should be enough because >> PM_QOS_RESUME_LATENCY_NO_CONSTRAINT = S32_MAX which is the largest possible >> value, if that can be assumed here. > > Well, not quite. effective_constraint_ns is s64 and it is the QoS > value multiplied by NSEC_PER_USEC. > > Surely can be greater than PM_QOS_RESUME_LATENCY_NO_CONSTRAINT. > > Which reminds me about the possible collision in default_suspend_ok() > which needs to be addressed. :-) > >>> *constraint_ns_p = constraint_ns; >>> >>> return 0; >>> @@ -58,12 +58,12 @@ static bool default_suspend_ok(struct de >>> } >>> td->constraint_changed = false; >>> td->cached_suspend_ok = false; >>> - td->effective_constraint_ns = -1; >>> + td->effective_constraint_ns = 0; >>> constraint_ns = __dev_pm_qos_read_value(dev); >> >> Why not initialize td->effective_constraint_ns with what is read by >> __dev_pm_qos_read_value()? Because if that is 0, we can return without scribbling on it again in the following code and if it wasn't 0, it would need to be multiplied by NSEC_PER_USEC. Thanks, Rafael
Index: linux-pm/drivers/base/power/sysfs.c =================================================================== --- linux-pm.orig/drivers/base/power/sysfs.c +++ linux-pm/drivers/base/power/sysfs.c @@ -218,7 +218,14 @@ static ssize_t pm_qos_resume_latency_sho struct device_attribute *attr, char *buf) { - return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev)); + s32 value = dev_pm_qos_requested_resume_latency(dev); + + if (value == 0) + return sprintf(buf, "n/a\n"); + else if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) + value = 0; + + return sprintf(buf, "%d\n", value); } static ssize_t pm_qos_resume_latency_store(struct device *dev, @@ -228,11 +235,21 @@ static ssize_t pm_qos_resume_latency_sto s32 value; int ret; - if (kstrtos32(buf, 0, &value)) - return -EINVAL; + if (!kstrtos32(buf, 0, &value)) { + /* + * Prevent users from writing negative or "no constraint" values + * directly. + */ + if (value < 0 || value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) + return -EINVAL; - if (value < 0) + if (value == 0) + value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; + } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) { + value = 0; + } else { return -EINVAL; + } ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req, value); Index: linux-pm/include/linux/pm_qos.h =================================================================== --- linux-pm.orig/include/linux/pm_qos.h +++ linux-pm/include/linux/pm_qos.h @@ -27,16 +27,17 @@ enum pm_qos_flags_status { PM_QOS_FLAGS_ALL, }; -#define PM_QOS_DEFAULT_VALUE -1 +#define PM_QOS_DEFAULT_VALUE (-1) +#define PM_QOS_LATENCY_ANY S32_MAX #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC) #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE 0 #define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE 0 -#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE 0 +#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE PM_QOS_LATENCY_ANY +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 0 #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT (-1) -#define PM_QOS_LATENCY_ANY ((s32)(~(__u32)0 >> 1)) #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0) @@ -173,7 +174,8 @@ static inline s32 dev_pm_qos_requested_f static inline s32 dev_pm_qos_raw_read_value(struct device *dev) { return IS_ERR_OR_NULL(dev->power.qos) ? - 0 : pm_qos_read_value(&dev->power.qos->resume_latency); + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT : + pm_qos_read_value(&dev->power.qos->resume_latency); } #else static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev, @@ -183,9 +185,9 @@ static inline enum pm_qos_flags_status d s32 mask) { return PM_QOS_FLAGS_UNDEFINED; } static inline s32 __dev_pm_qos_read_value(struct device *dev) - { return 0; } + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } static inline s32 dev_pm_qos_read_value(struct device *dev) - { return 0; } + { return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; } static inline int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, enum dev_pm_qos_req_type type, @@ -231,9 +233,15 @@ static inline int dev_pm_qos_expose_late { return 0; } static inline void dev_pm_qos_hide_latency_tolerance(struct device *dev) {} -static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) { return 0; } +static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev) +{ + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; +} static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; } -static inline s32 dev_pm_qos_raw_read_value(struct device *dev) { return 0; } +static inline s32 dev_pm_qos_raw_read_value(struct device *dev) +{ + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; +} #endif #endif Index: linux-pm/drivers/cpuidle/governors/menu.c =================================================================== --- linux-pm.orig/drivers/cpuidle/governors/menu.c +++ linux-pm/drivers/cpuidle/governors/menu.c @@ -298,8 +298,8 @@ static int menu_select(struct cpuidle_dr data->needs_update = 0; } - /* resume_latency is 0 means no restriction */ - if (resume_latency && resume_latency < latency_req) + if (resume_latency < latency_req && + resume_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) latency_req = resume_latency; /* Special case when user has set very strict latency requirement */ Index: linux-pm/drivers/base/power/runtime.c =================================================================== --- linux-pm.orig/drivers/base/power/runtime.c +++ linux-pm/drivers/base/power/runtime.c @@ -253,7 +253,7 @@ static int rpm_check_suspend_allowed(str || (dev->power.request_pending && dev->power.request == RPM_REQ_RESUME)) retval = -EAGAIN; - else if (__dev_pm_qos_read_value(dev) < 0) + else if (__dev_pm_qos_read_value(dev) == 0) retval = -EPERM; else if (dev->power.runtime_status == RPM_SUSPENDED) retval = 1; Index: linux-pm/drivers/base/cpu.c =================================================================== --- linux-pm.orig/drivers/base/cpu.c +++ linux-pm/drivers/base/cpu.c @@ -377,7 +377,8 @@ int register_cpu(struct cpu *cpu, int nu per_cpu(cpu_sys_devices, num) = &cpu->dev; register_cpu_under_node(num, cpu_to_node(num)); - dev_pm_qos_expose_latency_limit(&cpu->dev, 0); + dev_pm_qos_expose_latency_limit(&cpu->dev, + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT); return 0; } Index: linux-pm/drivers/base/power/qos.c =================================================================== --- linux-pm.orig/drivers/base/power/qos.c +++ linux-pm/drivers/base/power/qos.c @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p switch(req->type) { case DEV_PM_QOS_RESUME_LATENCY: + if (WARN_ON(value < 0)) + value = 0; + ret = pm_qos_update_target(&qos->resume_latency, &req->data.pnode, action, value); break; @@ -189,7 +192,7 @@ static int dev_pm_qos_constraints_alloca plist_head_init(&c->list); c->target_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; c->default_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; - c->no_constraint_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE; + c->no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; c->type = PM_QOS_MIN; c->notifiers = n; Index: linux-pm/Documentation/ABI/testing/sysfs-devices-power =================================================================== --- linux-pm.orig/Documentation/ABI/testing/sysfs-devices-power +++ linux-pm/Documentation/ABI/testing/sysfs-devices-power @@ -211,7 +211,9 @@ Description: device, after it has been suspended at run time, from a resume request to the moment the device will be ready to process I/O, in microseconds. If it is equal to 0, however, this means that - the PM QoS resume latency may be arbitrary. + the PM QoS resume latency may be arbitrary and the special value + "n/a" means that user space cannot accept any resume latency at + all for the given device. Not all drivers support this attribute. If it isn't supported, it is not present. Index: linux-pm/drivers/base/power/domain.c =================================================================== --- linux-pm.orig/drivers/base/power/domain.c +++ linux-pm/drivers/base/power/domain.c @@ -1331,7 +1331,7 @@ static struct generic_pm_domain_data *ge gpd_data->base.dev = dev; gpd_data->td.constraint_changed = true; - gpd_data->td.effective_constraint_ns = 0; + gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; spin_lock_irq(&dev->power.lock); Index: linux-pm/drivers/base/power/domain_governor.c =================================================================== --- linux-pm.orig/drivers/base/power/domain_governor.c +++ linux-pm/drivers/base/power/domain_governor.c @@ -26,11 +26,11 @@ static int dev_update_qos_constraint(str * to take effect, the device has to be resumed and suspended again. */ constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns; - /* 0 means "no constraint" */ - if (constraint_ns == 0) + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) return 0; - if (constraint_ns < *constraint_ns_p || *constraint_ns_p == 0) + if (constraint_ns < *constraint_ns_p || + *constraint_ns_p == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) *constraint_ns_p = constraint_ns; return 0; @@ -58,12 +58,12 @@ static bool default_suspend_ok(struct de } td->constraint_changed = false; td->cached_suspend_ok = false; - td->effective_constraint_ns = -1; + td->effective_constraint_ns = 0; constraint_ns = __dev_pm_qos_read_value(dev); spin_unlock_irqrestore(&dev->power.lock, flags); - if (constraint_ns < 0) + if (constraint_ns == 0) return false; constraint_ns *= NSEC_PER_USEC; @@ -76,14 +76,14 @@ static bool default_suspend_ok(struct de device_for_each_child(dev, &constraint_ns, dev_update_qos_constraint); - if (constraint_ns == 0) { + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) { /* "No restriction", so the device is allowed to suspend. */ - td->effective_constraint_ns = 0; + td->effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; td->cached_suspend_ok = true; - } else if (WARN_ON(constraint_ns < 0)) { + } else if (WARN_ON(constraint_ns == 0)) { /* - * effective_constraint_ns is negative already and - * cached_suspend_ok is false, so return right away. + * effective_constraint_ns is 0 already and cached_suspend_ok is + * false, so return right away. */ return false; } else { @@ -156,12 +156,11 @@ static bool __default_power_down_ok(stru */ td = &to_gpd_data(pdd)->td; constraint_ns = td->effective_constraint_ns; - /* Negative values mean "no suspend at all" */ - if (WARN_ON(constraint_ns < 0)) + /* 0 mean "no suspend at all" */ + if (WARN_ON(constraint_ns == 0)) return false; - /* 0 means "no constraint" */ - if (constraint_ns == 0) + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) continue; if (constraint_ns <= off_on_time_ns)