pmqos: Replace spinlock with mutex for pm_qos_lock
Sai Gurrappadi [Tue, 1 Oct 2013 17:01:27 +0000 (10:01 -0700)]
Using a spinlock (taken with irqsave) meant that pm_qos_lock couldn't be
used to synchronize on the notifiers in order to ensure proper order of
the notifications. This is needed in case where there might be two near
simultaneous pmqos client requests for a bound on the same constraint;
the notifiers in pm_qos_update_target for the two clients could
potentially engage in a race.

Example:

Assume two requests are made (A, B with A coming first) for max cpufreq
and these are the only requests currently available.

Current behavior can result in:

notify(max_cpu_freq, minof(A, B))
notify(max_cpu_freq, minof(LONG_MAX, A))

Expected behavior:

notify(max_cpu_freq, minof(LONG_MAX, A))
notify(max_cpu_freq, minof(A, B))

Most of the PM QoS and Dev PM QoS requester clients were reviewed and
none of them were found to be calling pm_qos_add/update/remove request
from interrupt or atomic context since those calls include the blocking
notifier call which cannot be done in atomic context.

Change-Id: I2fb43cc38da4c701e4872b937dd82cd38f1a1c1e
Signed-off-by: Sai Gurrappadi <sgurrappadi@nvidia.com>
Reviewed-on: http://git-master/r/299036
Reviewed-by: Automatic_Commit_Validation_User
Reviewed-by: Diwakar Tundlam <dtundlam@nvidia.com>

kernel/power/qos.c

index 1dcb9fd..6f3b1f9 100644 (file)
  * pointer or exits the pm_qos_object will get an opportunity to clean up.
  *
  * Mark Gross <mgross@linux.intel.com>
+ *
+ * Conversion of pm_qos_lock from spinlock to mutex
+ * Sai Gurrappadi <sgurrappadi@nvidia.com>
+ * Copyright (c) 2013, NVIDIA CORPORATION.  All rights reserved.
  */
 
 /*#define DEBUG*/
@@ -48,7 +52,7 @@
 /*
  * locking rule: all changes to constraints or notifiers lists
  * or pm_qos_object list and pm_qos_objects need to happen with pm_qos_lock
- * held, taken with _irqsave.  One lock to rule them all
+ * One lock to rule them all
  */
 struct pm_qos_object {
        struct pm_qos_constraints *constraints;
@@ -56,7 +60,7 @@ struct pm_qos_object {
        char *name;
 };
 
-static DEFINE_SPINLOCK(pm_qos_lock);
+static DEFINE_MUTEX(pm_qos_lock);
 
 static struct pm_qos_object null_pm_qos;
 
@@ -257,10 +261,9 @@ static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
 int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node,
                         enum pm_qos_req_action action, int value)
 {
-       unsigned long flags;
-       int prev_value, curr_value, new_value;
+       int prev_value, curr_value, new_value, ret;
 
-       spin_lock_irqsave(&pm_qos_lock, flags);
+       mutex_lock(&pm_qos_lock);
        prev_value = pm_qos_get_value(c);
        if (value == PM_QOS_DEFAULT_VALUE)
                new_value = c->default_value;
@@ -294,16 +297,18 @@ int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node,
                curr_value = c->default_value;
        }
 
-       spin_unlock_irqrestore(&pm_qos_lock, flags);
-
        if (prev_value != curr_value) {
                blocking_notifier_call_chain(c->notifiers,
                                             (unsigned long)curr_value,
                                             NULL);
-               return 1;
+               ret = 1;
        } else {
-               return 0;
+               ret = 0;
        }
+
+       mutex_unlock(&pm_qos_lock);
+
+       return ret;
 }
 
 /**
@@ -338,10 +343,9 @@ bool pm_qos_update_flags(struct pm_qos_flags *pqf,
                         struct pm_qos_flags_request *req,
                         enum pm_qos_req_action action, s32 val)
 {
-       unsigned long irqflags;
        s32 prev_value, curr_value;
 
-       spin_lock_irqsave(&pm_qos_lock, irqflags);
+       mutex_lock(&pm_qos_lock);
 
        prev_value = list_empty(&pqf->list) ? 0 : pqf->effective_flags;
 
@@ -364,7 +368,7 @@ bool pm_qos_update_flags(struct pm_qos_flags *pqf,
 
        curr_value = list_empty(&pqf->list) ? 0 : pqf->effective_flags;
 
-       spin_unlock_irqrestore(&pm_qos_lock, irqflags);
+       mutex_unlock(&pm_qos_lock);
 
        return prev_value != curr_value;
 }
@@ -531,7 +535,6 @@ EXPORT_SYMBOL_GPL(pm_qos_remove_request);
 
 static int pm_qos_enabled_set(const char *arg, const struct kernel_param *kp)
 {
-       unsigned long flags;
        bool old;
        s32 prev[PM_QOS_NUM_CLASSES], curr[PM_QOS_NUM_CLASSES];
        int ret, i;
@@ -543,7 +546,7 @@ static int pm_qos_enabled_set(const char *arg, const struct kernel_param *kp)
                        __FUNCTION__, arg);
                return ret;
        }
-       spin_lock_irqsave(&pm_qos_lock, flags);
+       mutex_lock(&pm_qos_lock);
        for (i = 1; i < PM_QOS_NUM_CLASSES; i++)
                prev[i] = pm_qos_read_value(pm_qos_array[i]->constraints);
        if (old && !pm_qos_enabled) {
@@ -559,14 +562,13 @@ static int pm_qos_enabled_set(const char *arg, const struct kernel_param *kp)
                        pm_qos_set_value(pm_qos_array[i]->constraints, curr[i]);
                }
        }
-       spin_unlock_irqrestore(&pm_qos_lock, flags);
        for (i = 1; i < PM_QOS_NUM_CLASSES; i++)
                if (prev[i] != curr[i])
                        blocking_notifier_call_chain(
                                pm_qos_array[i]->constraints->notifiers,
                                (unsigned long)curr[i],
                                NULL);
-
+       mutex_unlock(&pm_qos_lock);
        return ret;
 }
 
@@ -679,7 +681,6 @@ static ssize_t pm_qos_power_read(struct file *filp, char __user *buf,
                size_t count, loff_t *f_pos)
 {
        s32 value;
-       unsigned long flags;
        struct pm_qos_request *req = filp->private_data;
 
        if (!req)
@@ -687,9 +688,9 @@ static ssize_t pm_qos_power_read(struct file *filp, char __user *buf,
        if (!pm_qos_request_active(req))
                return -EINVAL;
 
-       spin_lock_irqsave(&pm_qos_lock, flags);
+       mutex_lock(&pm_qos_lock);
        value = pm_qos_get_value(pm_qos_array[req->pm_qos_class]->constraints);
-       spin_unlock_irqrestore(&pm_qos_lock, flags);
+       mutex_unlock(&pm_qos_lock);
 
        return simple_read_from_buffer(buf, count, f_pos, &value, sizeof(s32));
 }