* [PATCH 1/2] cpufreq: governor: CPUFREQ_GOV_POLICY_EXIT never fails
2016-05-12 13:12 [PATCH 0/2] cpufreq: Avoid checking governor return values that are always 0 Rafael J. Wysocki
@ 2016-05-12 13:13 ` Rafael J. Wysocki
2016-05-13 3:13 ` Viresh Kumar
2016-05-12 13:14 ` [PATCH 2/2] cpufreq: governor: CPUFREQ_GOV_STOP " Rafael J. Wysocki
1 sibling, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2016-05-12 13:13 UTC (permalink / raw)
To: Linux PM list
Cc: Srinivas Pandruvada, Viresh Kumar, Linux Kernel Mailing List
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
None of the cpufreq governors currently in the tree will ever fail
an invocation of the ->governor() callback with the event argument
equal to CPUFREQ_GOV_POLICY_EXIT (unless invoked with incorrect
arguments which doesn't matter anyway) and it wouldn't really
make sense to fail it, because the caller won't be able to handle
that failure in a meaningful way.
Accordingly, rearrange the code in the core to make it clear that
this call never fails.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -78,9 +78,9 @@ static int cpufreq_governor(struct cpufr
static unsigned int __cpufreq_get(struct cpufreq_policy *policy);
static int cpufreq_start_governor(struct cpufreq_policy *policy);
-static inline int cpufreq_exit_governor(struct cpufreq_policy *policy)
+static inline void cpufreq_exit_governor(struct cpufreq_policy *policy)
{
- return cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
+ (void)cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
}
/**
@@ -1387,12 +1387,8 @@ static void cpufreq_offline(unsigned int
if (cpufreq_driver->stop_cpu)
cpufreq_driver->stop_cpu(policy);
- /* If cpu is last user of policy, free policy */
- if (has_target()) {
- ret = cpufreq_exit_governor(policy);
- if (ret)
- pr_err("%s: Failed to exit governor\n", __func__);
- }
+ if (has_target())
+ cpufreq_exit_governor(policy);
/*
* Perform the ->exit() even during light-weight tear-down,
@@ -2049,16 +2045,15 @@ static int cpufreq_governor(struct cpufr
ret = policy->governor->governor(policy, event);
- if (!ret) {
- if (event == CPUFREQ_GOV_POLICY_INIT)
+ if (event == CPUFREQ_GOV_POLICY_INIT) {
+ if (ret)
+ module_put(policy->governor->owner);
+ else
policy->governor->initialized++;
- else if (event == CPUFREQ_GOV_POLICY_EXIT)
- policy->governor->initialized--;
- }
-
- if (((event == CPUFREQ_GOV_POLICY_INIT) && ret) ||
- ((event == CPUFREQ_GOV_POLICY_EXIT) && !ret))
+ } else if (event == CPUFREQ_GOV_POLICY_EXIT) {
+ policy->governor->initialized--;
module_put(policy->governor->owner);
+ }
return ret;
}
@@ -2228,13 +2223,7 @@ static int cpufreq_set_policy(struct cpu
__func__, old_gov->name, ret);
return ret;
}
-
- ret = cpufreq_exit_governor(policy);
- if (ret) {
- pr_err("%s: Failed to Exit Governor: %s (%d)\n",
- __func__, old_gov->name, ret);
- return ret;
- }
+ cpufreq_exit_governor(policy);
}
/* start new governor */
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] cpufreq: governor: CPUFREQ_GOV_STOP never fails
2016-05-12 13:12 [PATCH 0/2] cpufreq: Avoid checking governor return values that are always 0 Rafael J. Wysocki
2016-05-12 13:13 ` [PATCH 1/2] cpufreq: governor: CPUFREQ_GOV_POLICY_EXIT never fails Rafael J. Wysocki
@ 2016-05-12 13:14 ` Rafael J. Wysocki
2016-05-13 3:14 ` Viresh Kumar
1 sibling, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2016-05-12 13:14 UTC (permalink / raw)
To: Linux PM list
Cc: Srinivas Pandruvada, Viresh Kumar, Linux Kernel Mailing List
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
None of the cpufreq governors currently in the tree will ever fail
an invocation of the ->governor() callback with the event argument
equal to CPUFREQ_GOV_STOP (unless invoked with incorrect arguments
which doesn't matter anyway) and it is rather difficult to imagine
a valid reason for such a failure.
Accordingly, rearrange the code in the core to make it clear that
this call never fails.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq.c | 40 +++++++++++-----------------------------
1 file changed, 11 insertions(+), 29 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -83,6 +83,11 @@ static inline void cpufreq_exit_governor
(void)cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
}
+static inline void cpufreq_stop_governor(struct cpufreq_policy *policy)
+{
+ (void)cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+}
+
/**
* Two notifier lists: the "policy" list is involved in the
* validation process for a new CPU frequency policy; the
@@ -1026,13 +1031,8 @@ static int cpufreq_add_policy_cpu(struct
return 0;
down_write(&policy->rwsem);
- if (has_target()) {
- ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
- if (ret) {
- pr_err("%s: Failed to stop governor\n", __func__);
- goto unlock;
- }
- }
+ if (has_target())
+ cpufreq_stop_governor(policy);
cpumask_set_cpu(cpu, policy->cpus);
@@ -1041,8 +1041,6 @@ static int cpufreq_add_policy_cpu(struct
if (ret)
pr_err("%s: Failed to start governor\n", __func__);
}
-
-unlock:
up_write(&policy->rwsem);
return ret;
}
@@ -1354,11 +1352,8 @@ static void cpufreq_offline(unsigned int
}
down_write(&policy->rwsem);
- if (has_target()) {
- ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
- if (ret)
- pr_err("%s: Failed to stop governor\n", __func__);
- }
+ if (has_target())
+ cpufreq_stop_governor(policy);
cpumask_clear_cpu(cpu, policy->cpus);
@@ -1622,7 +1617,6 @@ EXPORT_SYMBOL(cpufreq_generic_suspend);
void cpufreq_suspend(void)
{
struct cpufreq_policy *policy;
- int ret;
if (!cpufreq_driver)
return;
@@ -1635,14 +1629,8 @@ void cpufreq_suspend(void)
for_each_active_policy(policy) {
if (has_target()) {
down_write(&policy->rwsem);
- ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+ cpufreq_stop_governor(policy);
up_write(&policy->rwsem);
-
- if (ret) {
- pr_err("%s: Failed to stop governor for policy: %p\n",
- __func__, policy);
- continue;
- }
}
if (cpufreq_driver->suspend && cpufreq_driver->suspend(policy))
@@ -2216,13 +2204,7 @@ static int cpufreq_set_policy(struct cpu
old_gov = policy->governor;
/* end old governor */
if (old_gov) {
- ret = cpufreq_governor(policy, CPUFREQ_GOV_STOP);
- if (ret) {
- /* This can happen due to race with other operations */
- pr_debug("%s: Failed to Stop Governor: %s (%d)\n",
- __func__, old_gov->name, ret);
- return ret;
- }
+ cpufreq_stop_governor(policy);
cpufreq_exit_governor(policy);
}
^ permalink raw reply [flat|nested] 5+ messages in thread