Revert "PM / QoS: Fix device resume latency PM QoS"
authorRafael J. Wysocki <rafael.j.wysocki@intel.com>
Tue, 31 Oct 2017 17:26:15 +0000 (18:26 +0100)
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>
Tue, 31 Oct 2017 17:35:40 +0000 (18:35 +0100)
This reverts commit 0cc2b4e5a020 (PM / QoS: Fix device resume latency PM
QoS) as it introduced regressions on multiple systems and the fix-up
in commit 2a9a86d5c813 (PM / QoS: Fix default runtime_pm device resume
latency) does not address all of them.

The original problem that commit 0cc2b4e5a020 was attempting to fix
will be addressed later.

Fixes: 0cc2b4e5a020 (PM / QoS: Fix device resume latency PM QoS)
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Documentation/ABI/testing/sysfs-devices-power
drivers/base/cpu.c
drivers/base/power/domain_governor.c
drivers/base/power/qos.c
drivers/base/power/runtime.c
drivers/base/power/sysfs.c
drivers/cpuidle/governors/menu.c
include/linux/pm_qos.h

index 5cbb6f0..676fdf5 100644 (file)
@@ -211,9 +211,7 @@ 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 and the special value
-               "n/a" means that user space cannot accept any resume latency at
-               all for the given device.
+               the PM QoS resume latency may be arbitrary.
 
                Not all drivers support this attribute.  If it isn't supported,
                it is not present.
index 227bac5..321cd7b 100644 (file)
@@ -377,8 +377,7 @@ int register_cpu(struct cpu *cpu, int num)
 
        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,
-                                       PM_QOS_RESUME_LATENCY_NO_CONSTRAINT);
+       dev_pm_qos_expose_latency_limit(&cpu->dev, 0);
 
        return 0;
 }
index 51751cc..281f949 100644 (file)
 static int dev_update_qos_constraint(struct device *dev, void *data)
 {
        s64 *constraint_ns_p = data;
-       s64 constraint_ns = -1;
+       s32 constraint_ns = -1;
 
        if (dev->power.subsys_data && dev->power.subsys_data->domain_data)
                constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns;
 
-       if (constraint_ns < 0)
+       if (constraint_ns < 0) {
                constraint_ns = dev_pm_qos_read_value(dev);
-
-       if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
+               constraint_ns *= NSEC_PER_USEC;
+       }
+       if (constraint_ns == 0)
                return 0;
 
-       constraint_ns *= NSEC_PER_USEC;
-
-       if (constraint_ns < *constraint_ns_p || *constraint_ns_p < 0)
+       /*
+        * constraint_ns cannot be negative here, because the device has been
+        * suspended.
+        */
+       if (constraint_ns < *constraint_ns_p || *constraint_ns_p == 0)
                *constraint_ns_p = constraint_ns;
 
        return 0;
@@ -60,14 +63,10 @@ static bool default_suspend_ok(struct device *dev)
 
        spin_unlock_irqrestore(&dev->power.lock, flags);
 
-       if (constraint_ns == 0)
+       if (constraint_ns < 0)
                return false;
 
-       if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
-               constraint_ns = -1;
-       else
-               constraint_ns *= NSEC_PER_USEC;
-
+       constraint_ns *= NSEC_PER_USEC;
        /*
         * We can walk the children without any additional locking, because
         * they all have been suspended at this point and their
@@ -77,19 +76,14 @@ static bool default_suspend_ok(struct device *dev)
                device_for_each_child(dev, &constraint_ns,
                                      dev_update_qos_constraint);
 
-       if (constraint_ns < 0) {
-               /* The children have no constraints. */
-               td->effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
-               td->cached_suspend_ok = true;
-       } else {
-               constraint_ns -= td->suspend_latency_ns + td->resume_latency_ns;
-               if (constraint_ns > 0) {
-                       td->effective_constraint_ns = constraint_ns;
-                       td->cached_suspend_ok = true;
-               } else {
-                       td->effective_constraint_ns = 0;
-               }
+       if (constraint_ns > 0) {
+               constraint_ns -= td->suspend_latency_ns +
+                               td->resume_latency_ns;
+               if (constraint_ns == 0)
+                       return false;
        }
+       td->effective_constraint_ns = constraint_ns;
+       td->cached_suspend_ok = constraint_ns >= 0;
 
        /*
         * The children have been suspended already, so we don't need to take
@@ -151,14 +145,13 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd,
                td = &to_gpd_data(pdd)->td;
                constraint_ns = td->effective_constraint_ns;
                /* default_suspend_ok() need not be called before us. */
-               if (constraint_ns < 0)
+               if (constraint_ns < 0) {
                        constraint_ns = dev_pm_qos_read_value(pdd->dev);
-
-               if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
+                       constraint_ns *= NSEC_PER_USEC;
+               }
+               if (constraint_ns == 0)
                        continue;
 
-               constraint_ns *= NSEC_PER_USEC;
-
                /*
                 * constraint_ns cannot be negative here, because the device has
                 * been suspended.
index 7d29286..277d43a 100644 (file)
@@ -189,7 +189,7 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
        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_NO_CONSTRAINT;
+       c->no_constraint_value = PM_QOS_RESUME_LATENCY_DEFAULT_VALUE;
        c->type = PM_QOS_MIN;
        c->notifiers = n;
 
index 13e0159..7bcf80f 100644 (file)
@@ -253,7 +253,7 @@ static int rpm_check_suspend_allowed(struct device *dev)
            || (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 632077f..156ab57 100644 (file)
@@ -218,14 +218,7 @@ static ssize_t pm_qos_resume_latency_show(struct device *dev,
                                          struct device_attribute *attr,
                                          char *buf)
 {
-       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);
+       return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev));
 }
 
 static ssize_t pm_qos_resume_latency_store(struct device *dev,
@@ -235,21 +228,11 @@ static ssize_t pm_qos_resume_latency_store(struct device *dev,
        s32 value;
        int ret;
 
-       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 (kstrtos32(buf, 0, &value))
+               return -EINVAL;
 
-               if (value == 0)
-                       value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
-       } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) {
-               value = 0;
-       } else {
+       if (value < 0)
                return -EINVAL;
-       }
 
        ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req,
                                        value);
index aa39040..48eaf28 100644 (file)
@@ -298,8 +298,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
                data->needs_update = 0;
        }
 
-       if (resume_latency < latency_req &&
-           resume_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
+       /* resume_latency is 0 means no restriction */
+       if (resume_latency && resume_latency < latency_req)
                latency_req = resume_latency;
 
        /* Special case when user has set very strict latency requirement */
index 6737a8c..032b559 100644 (file)
@@ -27,17 +27,16 @@ enum pm_qos_flags_status {
        PM_QOS_FLAGS_ALL,
 };
 
-#define PM_QOS_DEFAULT_VALUE   (-1)
-#define PM_QOS_LATENCY_ANY     S32_MAX
+#define PM_QOS_DEFAULT_VALUE -1
 
 #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_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)
 #define PM_QOS_FLAG_REMOTE_WAKEUP      (1 << 1)