Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1326257 > unrolled thread
| Started by | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| First post | 2016-02-04 00:50 +0100 |
| Last post | 2016-02-07 17:00 +0100 |
| Articles | 20 on this page of 48 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-04 00:50 +0100
[PATCH 5/11] cpufreq: governor: Put governor structure into common_dbs_data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-04 00:50 +0100
Re: [PATCH 5/11] cpufreq: governor: Put governor structure into common_dbs_data Saravana Kannan <skannan@codeaurora.org> - 2016-02-04 03:00 +0100
[PATCH 11/11] cpufreq: governor: Drop cpu argument from dbs_check_cpu() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-04 00:50 +0100
Re: [PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-04 06:50 +0100
Re: [PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-04 18:30 +0100
[PATCH v2 7/10] cpufreq: governor: Drop the gov pointer from struct dbs_data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 7/10] cpufreq: governor: Drop the gov pointer from struct dbs_data Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 09:30 +0100
[PATCH v2 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 09:40 +0100
Re: [PATCH v2 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 23:50 +0100
[PATCH v3 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-06 13:50 +0100
Re: [PATCH v3 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 10:40 +0100
[PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 08:00 +0100
Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-06 00:00 +0100
Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 10:40 +0100
Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-07 15:40 +0100
[PATCH v2 6/10] cpufreq: governor: Rework cpufreq_governor_dbs() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 6/10] cpufreq: governor: Rework cpufreq_governor_dbs() Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 09:20 +0100
[PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 10:20 +0100
Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 23:50 +0100
Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 10:30 +0100
Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-07 15:40 +0100
[PATCH v2 1/10] cpufreq: Clean up default and fallback governor setup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
[PATCH v2 0/10] cpufreq: governor: ondemand/conservative data structures rework "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
[PATCH v2 3/10] cpufreq: governor: Avoid passing dbs_data pointers around unnecessarily "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 3/10] cpufreq: governor: Avoid passing dbs_data pointers around unnecessarily Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 08:10 +0100
[PATCH v2 5/10] cpufreq: governor: Rename some data types and variables "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 5/10] cpufreq: governor: Rename some data types and variables Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 08:20 +0100
[PATCH v2 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu() Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 10:20 +0100
[PATCH v3 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-06 13:50 +0100
Re: [PATCH v3 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu() Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 10:40 +0100
[PATCH v2 4/10] cpufreq: governor: Put governor structure into common_dbs_data "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-05 03:30 +0100
Re: [PATCH v2 4/10] cpufreq: governor: Put governor structure into common_dbs_data Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 08:20 +0100
Re: [PATCH v2 0/10] cpufreq: governor: ondemand/conservative data structures rework "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-06 13:50 +0100
[PATCH 1/3] cpufreq: governor: Simplify cpufreq_governor_limits() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-07 16:30 +0100
Re: [PATCH 1/3] cpufreq: governor: Simplify cpufreq_governor_limits() Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 16:50 +0100
Re: [PATCH 1/3] cpufreq: governor: Simplify cpufreq_governor_limits() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-08 02:00 +0100
[PATCH 0/3] cpufreq: governor: Data structure rearrangement "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-07 16:30 +0100
[PATCH 3/3] cpufreq: governor: Symmetrize cpu_dbs_info initialization and cleanup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-07 16:30 +0100
Re: [PATCH 3/3] cpufreq: governor: Symmetrize cpu_dbs_info initialization and cleanup Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 17:00 +0100
[PATCH 2/3] cpufreq: governor: Rearrange governor data structures "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-07 16:30 +0100
Re: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 16:50 +0100
Re: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 17:00 +0100
Re: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-07 17:00 +0100
Page 1 of 3 [1] 2 3 Next page →
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-04 00:50 +0100 |
| Subject | [PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework |
| Message-ID | <qYfbJ-uu-39@gated-at.bofh.it> |
Hi, A few days ago I looked at the common code used by the ondemand and conservative governors because of the deadlock issue that Viresh has addressed recently (http://marc.info/?l=linux-pm&m=145450832814058&w=4) and it occurred to me that the whole thing was really too tangled and might be made easier to follow at least. I started to work on this and ended up with the following series. I'm not really going to stop here, but first, I'd like to let everybody know that this is happening and second, I'll need to rebase these patches on the ones from Viresh (in the series linked above), but that may take some time and I don't want to sit on them for all that long. Overall, I'd like the governor code to be cleaner and easier to follow, so we can move at least some parts of governor work to utilization update callbacks (invoked by the scheduler) or to at least to irq_work so as to reduce the usage of process context in cpufreq to absolute minimum. That's the plan for the future, but for now this is just a major cleanup. [1/11] Clean up the way in which the default and fallback governors are set up. [2/11] Use a common global mutex for dbs_data protection. [3/11] Use common global pointer to dbs_data for system-wide governors. [4/11] Avoid passing dbs_data pointers to functions that can get them by themselves. [5/11] Make struct governor be a member of struct common_dbs_data. [6/11] Rename struct common_dbs_data to struct dbs_governor. [7/11] Rework cpufreq_governor_dbs() so it can be used as a governor callback directly. [8/11] Drop the dbs_governor (former cdata) pointer from struct dbs_data. [9/11] Rename struct cpu_common_dbs_info to struct policy_dbs_info. [10/11] Rearrange data structures so policy->governor_data points to struct policy_dbs_info. [11/11] Drop the second argument of dbs_check_cpu(). The patches are on top of 4.5-rc2 with my earlier series replacing timers with utilization update callbacks (http://marc.info/?l=linux-kernel&m=145410842801883&w=4) applied. Thanks, Rafael
[toc] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-04 00:50 +0100 |
| Subject | [PATCH 5/11] cpufreq: governor: Put governor structure into common_dbs_data |
| Message-ID | <qYflq-yl-55@gated-at.bofh.it> |
| In reply to | #1326257 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
For the ondemand and conservative governors (generally, governors
that use the common code in cpufreq_governor.c), there are two static
data structures representing the governor, the struct governor
structure (the interface to the cpufreq core) and the struct
common_dbs_data one (the interface to the cpufreq_governor.c code).
There's no fundamental reason why those two structures have to be
separate. Moreover, if the struct governor one is included into
struct common_dbs_data, it will be possible to reach the latter from
the policy via its policy->governor pointer, so it won't be necessary
to pass a separate pointer to it around. For this reason, embed
struct governor in struct common_dbs_data.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq_conservative.c | 78 +++++++++++++++++----------------
drivers/cpufreq/cpufreq_ondemand.c | 26 ++++++-----
2 files changed, 56 insertions(+), 48 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -539,7 +539,16 @@ static struct od_ops od_ops = {
.freq_increase = dbs_freq_increase,
};
+static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
+ unsigned int event);
+
static struct common_dbs_data od_dbs_cdata = {
+ .gov = {
+ .name = "ondemand",
+ .governor = od_cpufreq_governor_dbs,
+ .max_transition_latency = TRANSITION_LATENCY_LIMIT,
+ .owner = THIS_MODULE,
+ },
.governor = GOV_ONDEMAND,
.attr_group_gov_sys = &od_attr_group_gov_sys,
.attr_group_gov_pol = &od_attr_group_gov_pol,
@@ -552,19 +561,14 @@ static struct common_dbs_data od_dbs_cda
.exit = od_exit,
};
+#define CPU_FREQ_GOV_ONDEMAND (&od_dbs_cdata.gov)
+
static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
unsigned int event)
{
return cpufreq_governor_dbs(policy, &od_dbs_cdata, event);
}
-static struct cpufreq_governor cpufreq_gov_ondemand = {
- .name = "ondemand",
- .governor = od_cpufreq_governor_dbs,
- .max_transition_latency = TRANSITION_LATENCY_LIMIT,
- .owner = THIS_MODULE,
-};
-
static void od_set_powersave_bias(unsigned int powersave_bias)
{
struct cpufreq_policy *policy;
@@ -590,7 +594,7 @@ static void od_set_powersave_bias(unsign
policy = shared->policy;
cpumask_or(&done, &done, policy->cpus);
- if (policy->governor != &cpufreq_gov_ondemand)
+ if (policy->governor != CPU_FREQ_GOV_ONDEMAND)
continue;
dbs_data = policy->governor_data;
@@ -618,12 +622,12 @@ EXPORT_SYMBOL_GPL(od_unregister_powersav
static int __init cpufreq_gov_dbs_init(void)
{
- return cpufreq_register_governor(&cpufreq_gov_ondemand);
+ return cpufreq_register_governor(CPU_FREQ_GOV_ONDEMAND);
}
static void __exit cpufreq_gov_dbs_exit(void)
{
- cpufreq_unregister_governor(&cpufreq_gov_ondemand);
+ cpufreq_unregister_governor(CPU_FREQ_GOV_ONDEMAND);
}
MODULE_AUTHOR("Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>");
@@ -635,7 +639,7 @@ MODULE_LICENSE("GPL");
#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
struct cpufreq_governor *cpufreq_default_governor(void)
{
- return &cpufreq_gov_ondemand;
+ return CPU_FREQ_GOV_ONDEMAND;
}
fs_initcall(cpufreq_gov_dbs_init);
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -23,16 +23,6 @@
static DEFINE_PER_CPU(struct cs_cpu_dbs_info_s, cs_cpu_dbs_info);
-static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
- unsigned int event);
-
-static struct cpufreq_governor cpufreq_gov_conservative = {
- .name = "conservative",
- .governor = cs_cpufreq_governor_dbs,
- .max_transition_latency = TRANSITION_LATENCY_LIMIT,
- .owner = THIS_MODULE,
-};
-
static inline unsigned int get_freq_target(struct cs_dbs_tuners *cs_tuners,
struct cpufreq_policy *policy)
{
@@ -122,30 +112,7 @@ static unsigned int cs_dbs_timer(struct
}
static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
- void *data)
-{
- struct cpufreq_freqs *freq = data;
- struct cs_cpu_dbs_info_s *dbs_info =
- &per_cpu(cs_cpu_dbs_info, freq->cpu);
- struct cpufreq_policy *policy = cpufreq_cpu_get_raw(freq->cpu);
-
- if (!policy)
- return 0;
-
- /* policy isn't governed by conservative governor */
- if (policy->governor != &cpufreq_gov_conservative)
- return 0;
-
- /*
- * we only care if our internally tracked freq moves outside the 'valid'
- * ranges of frequency available to us otherwise we do not change it
- */
- if (dbs_info->requested_freq > policy->max
- || dbs_info->requested_freq < policy->min)
- dbs_info->requested_freq = freq->new;
-
- return 0;
-}
+ void *data);
static struct notifier_block cs_cpufreq_notifier_block = {
.notifier_call = dbs_cpufreq_notifier,
@@ -358,7 +325,16 @@ static void cs_exit(struct dbs_data *dbs
define_get_cpu_dbs_routines(cs_cpu_dbs_info);
+static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
+ unsigned int event);
+
static struct common_dbs_data cs_dbs_cdata = {
+ .gov = {
+ .name = "conservative",
+ .governor = cs_cpufreq_governor_dbs,
+ .max_transition_latency = TRANSITION_LATENCY_LIMIT,
+ .owner = THIS_MODULE,
+ },
.governor = GOV_CONSERVATIVE,
.attr_group_gov_sys = &cs_attr_group_gov_sys,
.attr_group_gov_pol = &cs_attr_group_gov_pol,
@@ -370,20 +346,48 @@ static struct common_dbs_data cs_dbs_cda
.exit = cs_exit,
};
+#define CPU_FREQ_GOV_CONSERVATIVE (&cs_dbs_cdata.gov)
+
static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
unsigned int event)
{
return cpufreq_governor_dbs(policy, &cs_dbs_cdata, event);
}
+static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
+ void *data)
+{
+ struct cpufreq_freqs *freq = data;
+ struct cs_cpu_dbs_info_s *dbs_info =
+ &per_cpu(cs_cpu_dbs_info, freq->cpu);
+ struct cpufreq_policy *policy = cpufreq_cpu_get_raw(freq->cpu);
+
+ if (!policy)
+ return 0;
+
+ /* policy isn't governed by conservative governor */
+ if (policy->governor != CPU_FREQ_GOV_CONSERVATIVE)
+ return 0;
+
+ /*
+ * we only care if our internally tracked freq moves outside the 'valid'
+ * ranges of frequency available to us otherwise we do not change it
+ */
+ if (dbs_info->requested_freq > policy->max
+ || dbs_info->requested_freq < policy->min)
+ dbs_info->requested_freq = freq->new;
+
+ return 0;
+}
+
static int __init cpufreq_gov_dbs_init(void)
{
- return cpufreq_register_governor(&cpufreq_gov_conservative);
+ return cpufreq_register_governor(CPU_FREQ_GOV_CONSERVATIVE);
}
static void __exit cpufreq_gov_dbs_exit(void)
{
- cpufreq_unregister_governor(&cpufreq_gov_conservative);
+ cpufreq_unregister_governor(CPU_FREQ_GOV_CONSERVATIVE);
}
MODULE_AUTHOR("Alexander Clouter <alex@digriz.org.uk>");
@@ -395,7 +399,7 @@ MODULE_LICENSE("GPL");
#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
struct cpufreq_governor *cpufreq_default_governor(void)
{
- return &cpufreq_gov_conservative;
+ return CPU_FREQ_GOV_CONSERVATIVE;
}
fs_initcall(cpufreq_gov_dbs_init);
[toc] | [prev] | [next] | [standalone]
| From | Saravana Kannan <skannan@codeaurora.org> |
|---|---|
| Date | 2016-02-04 03:00 +0100 |
| Subject | Re: [PATCH 5/11] cpufreq: governor: Put governor structure into common_dbs_data |
| Message-ID | <qYhnc-1Yt-5@gated-at.bofh.it> |
| In reply to | #1326260 |
On 02/03/2016 03:31 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> For the ondemand and conservative governors (generally, governors
> that use the common code in cpufreq_governor.c), there are two static
> data structures representing the governor, the struct governor
> structure (the interface to the cpufreq core) and the struct
> common_dbs_data one (the interface to the cpufreq_governor.c code).
>
> There's no fundamental reason why those two structures have to be
> separate. Moreover, if the struct governor one is included into
> struct common_dbs_data, it will be possible to reach the latter from
> the policy via its policy->governor pointer, so it won't be necessary
> to pass a separate pointer to it around. For this reason, embed
> struct governor in struct common_dbs_data.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/cpufreq_conservative.c | 78 +++++++++++++++++----------------
> drivers/cpufreq/cpufreq_ondemand.c | 26 ++++++-----
> 2 files changed, 56 insertions(+), 48 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
> +++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
> @@ -539,7 +539,16 @@ static struct od_ops od_ops = {
> .freq_increase = dbs_freq_increase,
> };
>
> +static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
> + unsigned int event);
> +
> static struct common_dbs_data od_dbs_cdata = {
> + .gov = {
> + .name = "ondemand",
> + .governor = od_cpufreq_governor_dbs,
> + .max_transition_latency = TRANSITION_LATENCY_LIMIT,
> + .owner = THIS_MODULE,
> + },
> .governor = GOV_ONDEMAND,
> .attr_group_gov_sys = &od_attr_group_gov_sys,
> .attr_group_gov_pol = &od_attr_group_gov_pol,
> @@ -552,19 +561,14 @@ static struct common_dbs_data od_dbs_cda
> .exit = od_exit,
> };
>
> +#define CPU_FREQ_GOV_ONDEMAND (&od_dbs_cdata.gov)
> +
> static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
> unsigned int event)
> {
> return cpufreq_governor_dbs(policy, &od_dbs_cdata, event);
> }
>
> -static struct cpufreq_governor cpufreq_gov_ondemand = {
> - .name = "ondemand",
> - .governor = od_cpufreq_governor_dbs,
> - .max_transition_latency = TRANSITION_LATENCY_LIMIT,
> - .owner = THIS_MODULE,
> -};
> -
> static void od_set_powersave_bias(unsigned int powersave_bias)
> {
> struct cpufreq_policy *policy;
> @@ -590,7 +594,7 @@ static void od_set_powersave_bias(unsign
> policy = shared->policy;
> cpumask_or(&done, &done, policy->cpus);
>
> - if (policy->governor != &cpufreq_gov_ondemand)
> + if (policy->governor != CPU_FREQ_GOV_ONDEMAND)
> continue;
>
> dbs_data = policy->governor_data;
> @@ -618,12 +622,12 @@ EXPORT_SYMBOL_GPL(od_unregister_powersav
>
> static int __init cpufreq_gov_dbs_init(void)
> {
> - return cpufreq_register_governor(&cpufreq_gov_ondemand);
> + return cpufreq_register_governor(CPU_FREQ_GOV_ONDEMAND);
> }
>
> static void __exit cpufreq_gov_dbs_exit(void)
> {
> - cpufreq_unregister_governor(&cpufreq_gov_ondemand);
> + cpufreq_unregister_governor(CPU_FREQ_GOV_ONDEMAND);
> }
>
> MODULE_AUTHOR("Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>");
> @@ -635,7 +639,7 @@ MODULE_LICENSE("GPL");
> #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
> struct cpufreq_governor *cpufreq_default_governor(void)
> {
> - return &cpufreq_gov_ondemand;
> + return CPU_FREQ_GOV_ONDEMAND;
> }
>
> fs_initcall(cpufreq_gov_dbs_init);
> Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
> +++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
> @@ -23,16 +23,6 @@
>
> static DEFINE_PER_CPU(struct cs_cpu_dbs_info_s, cs_cpu_dbs_info);
>
> -static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
> - unsigned int event);
> -
> -static struct cpufreq_governor cpufreq_gov_conservative = {
> - .name = "conservative",
> - .governor = cs_cpufreq_governor_dbs,
> - .max_transition_latency = TRANSITION_LATENCY_LIMIT,
> - .owner = THIS_MODULE,
> -};
> -
> static inline unsigned int get_freq_target(struct cs_dbs_tuners *cs_tuners,
> struct cpufreq_policy *policy)
> {
> @@ -122,30 +112,7 @@ static unsigned int cs_dbs_timer(struct
> }
>
> static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
> - void *data)
> -{
> - struct cpufreq_freqs *freq = data;
> - struct cs_cpu_dbs_info_s *dbs_info =
> - &per_cpu(cs_cpu_dbs_info, freq->cpu);
> - struct cpufreq_policy *policy = cpufreq_cpu_get_raw(freq->cpu);
> -
> - if (!policy)
> - return 0;
> -
> - /* policy isn't governed by conservative governor */
> - if (policy->governor != &cpufreq_gov_conservative)
> - return 0;
> -
> - /*
> - * we only care if our internally tracked freq moves outside the 'valid'
> - * ranges of frequency available to us otherwise we do not change it
> - */
> - if (dbs_info->requested_freq > policy->max
> - || dbs_info->requested_freq < policy->min)
> - dbs_info->requested_freq = freq->new;
> -
> - return 0;
> -}
> + void *data);
>
> static struct notifier_block cs_cpufreq_notifier_block = {
> .notifier_call = dbs_cpufreq_notifier,
> @@ -358,7 +325,16 @@ static void cs_exit(struct dbs_data *dbs
>
> define_get_cpu_dbs_routines(cs_cpu_dbs_info);
>
> +static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
> + unsigned int event);
> +
> static struct common_dbs_data cs_dbs_cdata = {
> + .gov = {
> + .name = "conservative",
> + .governor = cs_cpufreq_governor_dbs,
> + .max_transition_latency = TRANSITION_LATENCY_LIMIT,
> + .owner = THIS_MODULE,
> + },
> .governor = GOV_CONSERVATIVE,
> .attr_group_gov_sys = &cs_attr_group_gov_sys,
> .attr_group_gov_pol = &cs_attr_group_gov_pol,
> @@ -370,20 +346,48 @@ static struct common_dbs_data cs_dbs_cda
> .exit = cs_exit,
> };
>
> +#define CPU_FREQ_GOV_CONSERVATIVE (&cs_dbs_cdata.gov)
> +
> static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
> unsigned int event)
> {
> return cpufreq_governor_dbs(policy, &cs_dbs_cdata, event);
> }
>
> +static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
> + void *data)
> +{
> + struct cpufreq_freqs *freq = data;
> + struct cs_cpu_dbs_info_s *dbs_info =
> + &per_cpu(cs_cpu_dbs_info, freq->cpu);
> + struct cpufreq_policy *policy = cpufreq_cpu_get_raw(freq->cpu);
> +
> + if (!policy)
> + return 0;
> +
> + /* policy isn't governed by conservative governor */
> + if (policy->governor != CPU_FREQ_GOV_CONSERVATIVE)
> + return 0;
> +
> + /*
> + * we only care if our internally tracked freq moves outside the 'valid'
> + * ranges of frequency available to us otherwise we do not change it
> + */
> + if (dbs_info->requested_freq > policy->max
> + || dbs_info->requested_freq < policy->min)
> + dbs_info->requested_freq = freq->new;
> +
> + return 0;
> +}
> +
> static int __init cpufreq_gov_dbs_init(void)
> {
> - return cpufreq_register_governor(&cpufreq_gov_conservative);
> + return cpufreq_register_governor(CPU_FREQ_GOV_CONSERVATIVE);
> }
>
> static void __exit cpufreq_gov_dbs_exit(void)
> {
> - cpufreq_unregister_governor(&cpufreq_gov_conservative);
> + cpufreq_unregister_governor(CPU_FREQ_GOV_CONSERVATIVE);
> }
>
> MODULE_AUTHOR("Alexander Clouter <alex@digriz.org.uk>");
> @@ -395,7 +399,7 @@ MODULE_LICENSE("GPL");
> #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
> struct cpufreq_governor *cpufreq_default_governor(void)
> {
> - return &cpufreq_gov_conservative;
> + return CPU_FREQ_GOV_CONSERVATIVE;
> }
>
> fs_initcall(cpufreq_gov_dbs_init);
>
I'm not sold on the macros/#defines for the &blah.gov, but not a strong
opinion.
Acked-by: Saravana Kannan <skannan@codeaurora.org>
-Saravana
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-04 00:50 +0100 |
| Subject | [PATCH 11/11] cpufreq: governor: Drop cpu argument from dbs_check_cpu() |
| Message-ID | <qYflr-yl-61@gated-at.bofh.it> |
| In reply to | #1326257 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Since policy->cpu is always passed as the second argument to
dbs_check_cpu(), it is not really necessary to pass it, because
the function can obtain that value via its first argument just fine.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq_conservative.c | 2 +-
drivers/cpufreq/cpufreq_governor.c | 5 +++--
drivers/cpufreq/cpufreq_governor.h | 2 +-
drivers/cpufreq/cpufreq_ondemand.c | 6 ++----
4 files changed, 7 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -279,7 +279,7 @@ extern struct mutex cpufreq_governor_loc
void gov_set_update_util(struct policy_dbs_info *policy_dbs,
unsigned int delay_us);
void gov_cancel_work(struct policy_dbs_info *policy_dbs);
-void dbs_check_cpu(struct cpufreq_policy *policy, int cpu);
+void dbs_check_cpu(struct cpufreq_policy *policy);
int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event);
void od_register_powersave_bias_handler(unsigned int (*f)
(struct cpufreq_policy *, unsigned int, unsigned int),
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -198,9 +198,7 @@ static unsigned int od_dbs_timer(struct
{
struct policy_dbs_info *policy_dbs = policy->governor_data;
struct dbs_data *dbs_data = policy_dbs->dbs_data;
- unsigned int cpu = policy->cpu;
- struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info,
- cpu);
+ struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, policy->cpu);
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
int delay = 0, sample_type = dbs_info->sample_type;
@@ -211,7 +209,7 @@ static unsigned int od_dbs_timer(struct
__cpufreq_driver_target(policy, dbs_info->freq_lo,
CPUFREQ_RELATION_H);
} else {
- dbs_check_cpu(policy, cpu);
+ dbs_check_cpu(policy);
if (dbs_info->freq_lo) {
/* Setup timer for SUB_SAMPLE */
dbs_info->sample_type = OD_SUB_SAMPLE;
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -109,7 +109,7 @@ static unsigned int cs_dbs_timer(struct
struct dbs_data *dbs_data = policy_dbs->dbs_data;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
- dbs_check_cpu(policy, policy->cpu);
+ dbs_check_cpu(policy);
return delay_for_sampling_rate(cs_tuners->sampling_rate);
}
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -34,13 +34,14 @@ static struct attribute_group *get_sysfs
gov->attr_group_gov_pol : gov->attr_group_gov_sys;
}
-void dbs_check_cpu(struct cpufreq_policy *policy, int cpu)
+void dbs_check_cpu(struct cpufreq_policy *policy)
{
struct dbs_governor *gov = dbs_governor_of(policy);
struct policy_dbs_info *policy_dbs = policy->governor_data;
struct dbs_data *dbs_data = policy_dbs->dbs_data;
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ int cpu = policy->cpu;
unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
@@ -519,7 +520,7 @@ static int cpufreq_governor_limits(struc
__cpufreq_driver_target(policy, policy->max, CPUFREQ_RELATION_H);
else if (policy->min > policy->cur)
__cpufreq_driver_target(policy, policy->min, CPUFREQ_RELATION_L);
- dbs_check_cpu(policy, policy->cpu);
+ dbs_check_cpu(policy);
mutex_unlock(&policy_dbs->timer_mutex);
return 0;
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-04 06:50 +0100 |
| Subject | Re: [PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework |
| Message-ID | <qYkXL-4pW-1@gated-at.bofh.it> |
| In reply to | #1326257 |
On 04-02-16, 00:12, Rafael J. Wysocki wrote: > Hi, > > A few days ago I looked at the common code used by the ondemand and conservative > governors because of the deadlock issue that Viresh has addressed recently > (http://marc.info/?l=linux-pm&m=145450832814058&w=4) and it occurred to me > that the whole thing was really too tangled and might be made easier to follow > at least. I started to work on this and ended up with the following series. > > I'm not really going to stop here, but first, I'd like to let everybody know > that this is happening and second, I'll need to rebase these patches on the > ones from Viresh (in the series linked above), but that may take some time > and I don't want to sit on them for all that long. > > Overall, I'd like the governor code to be cleaner and easier to follow, so we can > move at least some parts of governor work to utilization update callbacks (invoked > by the scheduler) or to at least to irq_work so as to reduce the usage of process > context in cpufreq to absolute minimum. That's the plan for the future, but for > now this is just a major cleanup. > > [1/11] Clean up the way in which the default and fallback governors are set up. > [2/11] Use a common global mutex for dbs_data protection. > [3/11] Use common global pointer to dbs_data for system-wide governors. Hi Rafael, I have some very basic doubts on 2nd and 3rd patch, and so have stopped reviewing after that because there is too much dependency I believe on these two. I will review the rest, if my concerns on the earlier ones are incorrect. -- viresh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2016-02-04 18:30 +0100 |
| Subject | Re: [PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework |
| Message-ID | <qYvTd-53o-23@gated-at.bofh.it> |
| In reply to | #1326412 |
On Thu, Feb 4, 2016 at 6:40 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 04-02-16, 00:12, Rafael J. Wysocki wrote: >> Hi, >> >> A few days ago I looked at the common code used by the ondemand and conservative >> governors because of the deadlock issue that Viresh has addressed recently >> (http://marc.info/?l=linux-pm&m=145450832814058&w=4) and it occurred to me >> that the whole thing was really too tangled and might be made easier to follow >> at least. I started to work on this and ended up with the following series. >> >> I'm not really going to stop here, but first, I'd like to let everybody know >> that this is happening and second, I'll need to rebase these patches on the >> ones from Viresh (in the series linked above), but that may take some time >> and I don't want to sit on them for all that long. >> >> Overall, I'd like the governor code to be cleaner and easier to follow, so we can >> move at least some parts of governor work to utilization update callbacks (invoked >> by the scheduler) or to at least to irq_work so as to reduce the usage of process >> context in cpufreq to absolute minimum. That's the plan for the future, but for >> now this is just a major cleanup. >> >> [1/11] Clean up the way in which the default and fallback governors are set up. >> [2/11] Use a common global mutex for dbs_data protection. >> [3/11] Use common global pointer to dbs_data for system-wide governors. > > Hi Rafael, > > I have some very basic doubts on 2nd and 3rd patch, and so have > stopped reviewing after that because there is too much dependency I > believe on these two. > > I will review the rest, if my concerns on the earlier ones are > incorrect. Patch [3/11] is really incorrect (thanks for pointing the problem out to me), but it also isn't essential, so I can drop it just fine I think. I don't see issues with the [2/11], though, apart from the changelog being inaccurate which should be easy to fix. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-05 03:30 +0100 |
| Subject | [PATCH v2 7/10] cpufreq: governor: Drop the gov pointer from struct dbs_data |
| Message-ID | <qYEjM-2cq-1@gated-at.bofh.it> |
| In reply to | #1326257 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Since it is possible to obtain a pointer to struct dbs_governor
from a pointer to the struct governor embedded in it with the help
of container_of(), the additional gov pointer in struct dbs_data
isn't really necessary.
Drop that pointer and make the code using it reach the dbs_governor
object via policy->governor.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/amd_freq_sensitivity.c | 2 -
drivers/cpufreq/cpufreq_conservative.c | 2 -
drivers/cpufreq/cpufreq_governor.c | 63 +++++++++++++++------------------
drivers/cpufreq/cpufreq_governor.h | 8 +++-
drivers/cpufreq/cpufreq_ondemand.c | 2 -
5 files changed, 39 insertions(+), 38 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -228,9 +228,13 @@ struct dbs_governor {
void *gov_ops;
};
+static inline struct dbs_governor *dbs_governor_of(struct cpufreq_policy *policy)
+{
+ return container_of(policy->governor, struct dbs_governor, gov);
+}
+
/* Governor Per policy data */
struct dbs_data {
- struct dbs_governor *gov;
unsigned int min_sampling_rate;
int usage_count;
void *tuners;
@@ -275,7 +279,7 @@ extern struct mutex cpufreq_governor_loc
void gov_set_update_util(struct cpu_common_dbs_info *shared,
unsigned int delay_us);
void gov_cancel_work(struct cpu_common_dbs_info *shared);
-void dbs_check_cpu(struct dbs_data *dbs_data, int cpu);
+void dbs_check_cpu(struct cpufreq_policy *policy, int cpu);
int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event);
void od_register_powersave_bias_handler(unsigned int (*f)
(struct cpufreq_policy *, unsigned int, unsigned int),
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -107,7 +107,7 @@ static unsigned int cs_dbs_timer(struct
struct dbs_data *dbs_data = policy->governor_data;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
- dbs_check_cpu(dbs_data, policy->cpu);
+ dbs_check_cpu(policy, policy->cpu);
return delay_for_sampling_rate(cs_tuners->sampling_rate);
}
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -25,28 +25,27 @@
DEFINE_MUTEX(dbs_data_mutex);
EXPORT_SYMBOL_GPL(dbs_data_mutex);
-static struct attribute_group *get_sysfs_attr(struct dbs_data *dbs_data)
+static struct attribute_group *get_sysfs_attr(struct dbs_governor *gov)
{
- if (have_governor_per_policy())
- return dbs_data->gov->attr_group_gov_pol;
- else
- return dbs_data->gov->attr_group_gov_sys;
+ return have_governor_per_policy() ?
+ gov->attr_group_gov_pol : gov->attr_group_gov_sys;
}
-void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
+void dbs_check_cpu(struct cpufreq_policy *policy, int cpu)
{
- struct cpu_dbs_info *cdbs = dbs_data->gov->get_cpu_cdbs(cpu);
+ struct dbs_governor *gov = dbs_governor_of(policy);
+ struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
+ struct dbs_data *dbs_data = policy->governor_data;
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
- struct cpufreq_policy *policy = cdbs->shared->policy;
unsigned int sampling_rate;
unsigned int max_load = 0;
unsigned int ignore_nice;
unsigned int j;
- if (dbs_data->gov->governor == GOV_ONDEMAND) {
+ if (gov->governor == GOV_ONDEMAND) {
struct od_cpu_dbs_info_s *od_dbs_info =
- dbs_data->gov->get_cpu_dbs_info_s(cpu);
+ gov->get_cpu_dbs_info_s(cpu);
/*
* Sometimes, the ondemand governor uses an additional
@@ -71,7 +70,7 @@ void dbs_check_cpu(struct dbs_data *dbs_
unsigned int load;
int io_busy = 0;
- j_cdbs = dbs_data->gov->get_cpu_cdbs(j);
+ j_cdbs = gov->get_cpu_cdbs(j);
/*
* For the purpose of ondemand, waiting for disk IO is
@@ -79,7 +78,7 @@ void dbs_check_cpu(struct dbs_data *dbs_
* not that the system is actually idle. So do not add
* the iowait time to the cpu idle time.
*/
- if (dbs_data->gov->governor == GOV_ONDEMAND)
+ if (gov->governor == GOV_ONDEMAND)
io_busy = od_tuners->io_is_busy;
cur_idle_time = get_cpu_idle_time(j, &cur_wall_time, io_busy);
@@ -160,7 +159,7 @@ void dbs_check_cpu(struct dbs_data *dbs_
max_load = load;
}
- dbs_data->gov->gov_check_cpu(cpu, max_load);
+ gov->gov_check_cpu(cpu, max_load);
}
EXPORT_SYMBOL_GPL(dbs_check_cpu);
@@ -168,7 +167,7 @@ void gov_set_update_util(struct cpu_comm
unsigned int delay_us)
{
struct cpufreq_policy *policy = shared->policy;
- struct dbs_data *dbs_data = policy->governor_data;
+ struct dbs_governor *gov = dbs_governor_of(policy);
int cpu;
shared->sample_delay_ns = delay_us * NSEC_PER_USEC;
@@ -176,7 +175,7 @@ void gov_set_update_util(struct cpu_comm
shared->last_sample_time = 0;
for_each_cpu(cpu, policy->cpus) {
- struct cpu_dbs_info *cdbs = dbs_data->gov->get_cpu_cdbs(cpu);
+ struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
cpufreq_set_update_util_data(cpu, &cdbs->update_util);
}
@@ -214,11 +213,11 @@ static void dbs_work_handler(struct work
struct cpu_common_dbs_info *shared = container_of(work, struct
cpu_common_dbs_info, work);
struct cpufreq_policy *policy;
- struct dbs_data *dbs_data;
+ struct dbs_governor *gov;
unsigned int delay;
policy = shared->policy;
- dbs_data = policy->governor_data;
+ gov = dbs_governor_of(policy);
/*
* Make sure cpufreq_governor_limits() isn't evaluating load or the
@@ -226,7 +225,7 @@ static void dbs_work_handler(struct work
* parallel.
*/
mutex_lock(&shared->timer_mutex);
- delay = dbs_data->gov->gov_dbs_timer(policy);
+ delay = gov->gov_dbs_timer(policy);
shared->sample_delay_ns = jiffies_to_nsecs(delay);
shared->time_stamp = ktime_get();
mutex_unlock(&shared->timer_mutex);
@@ -277,9 +276,10 @@ static void dbs_update_util_handler(stru
}
static void set_sampling_rate(struct dbs_data *dbs_data,
- unsigned int sampling_rate)
+ struct dbs_governor *gov,
+ unsigned int sampling_rate)
{
- if (dbs_data->gov->governor == GOV_CONSERVATIVE) {
+ if (gov->governor == GOV_CONSERVATIVE) {
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
cs_tuners->sampling_rate = sampling_rate;
} else {
@@ -326,8 +326,7 @@ static void free_common_dbs_info(struct
static int cpufreq_governor_init(struct cpufreq_policy *policy)
{
- struct dbs_governor *gov = container_of(policy->governor,
- struct dbs_governor, gov);
+ struct dbs_governor *gov = dbs_governor_of(policy);
struct dbs_data *dbs_data = gov->gdbs_data;
unsigned int latency;
int ret;
@@ -357,7 +356,6 @@ static int cpufreq_governor_init(struct
if (ret)
goto free_dbs_data;
- dbs_data->gov = gov;
dbs_data->usage_count = 1;
ret = gov->init(dbs_data, !policy->governor->initialized);
@@ -372,7 +370,7 @@ static int cpufreq_governor_init(struct
/* Bring kernel and HW constraints together */
dbs_data->min_sampling_rate = max(dbs_data->min_sampling_rate,
MIN_LATENCY_MULTIPLIER * latency);
- set_sampling_rate(dbs_data, max(dbs_data->min_sampling_rate,
+ set_sampling_rate(dbs_data, gov, max(dbs_data->min_sampling_rate,
latency * LATENCY_MULTIPLIER));
if (!have_governor_per_policy())
@@ -381,7 +379,7 @@ static int cpufreq_governor_init(struct
policy->governor_data = dbs_data;
ret = sysfs_create_group(get_governor_parent_kobj(policy),
- get_sysfs_attr(dbs_data));
+ get_sysfs_attr(gov));
if (ret)
goto reset_gdbs_data;
@@ -402,8 +400,8 @@ free_dbs_data:
static int cpufreq_governor_exit(struct cpufreq_policy *policy)
{
+ struct dbs_governor *gov = dbs_governor_of(policy);
struct dbs_data *dbs_data = policy->governor_data;
- struct dbs_governor *gov = dbs_data->gov;
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
/* State should be equivalent to INIT */
@@ -412,7 +410,7 @@ static int cpufreq_governor_exit(struct
if (!--dbs_data->usage_count) {
sysfs_remove_group(get_governor_parent_kobj(policy),
- get_sysfs_attr(dbs_data));
+ get_sysfs_attr(gov));
policy->governor_data = NULL;
@@ -431,8 +429,8 @@ static int cpufreq_governor_exit(struct
static int cpufreq_governor_start(struct cpufreq_policy *policy)
{
+ struct dbs_governor *gov = dbs_governor_of(policy);
struct dbs_data *dbs_data = policy->governor_data;
- struct dbs_governor *gov = dbs_data->gov;
unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
struct cpu_common_dbs_info *shared = cdbs->shared;
@@ -500,8 +498,8 @@ static int cpufreq_governor_start(struct
static int cpufreq_governor_stop(struct cpufreq_policy *policy)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cpu_dbs_info *cdbs = dbs_data->gov->get_cpu_cdbs(policy->cpu);
+ struct dbs_governor *gov = dbs_governor_of(policy);
+ struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
struct cpu_common_dbs_info *shared = cdbs->shared;
/* State should be equivalent to START */
@@ -516,8 +514,7 @@ static int cpufreq_governor_stop(struct
static int cpufreq_governor_limits(struct cpufreq_policy *policy)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct dbs_governor *gov = dbs_data->gov;
+ struct dbs_governor *gov = dbs_governor_of(policy);
unsigned int cpu = policy->cpu;
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
@@ -532,7 +529,7 @@ static int cpufreq_governor_limits(struc
else if (policy->min > cdbs->shared->policy->cur)
__cpufreq_driver_target(cdbs->shared->policy, policy->min,
CPUFREQ_RELATION_L);
- dbs_check_cpu(dbs_data, cpu);
+ dbs_check_cpu(policy, cpu);
mutex_unlock(&cdbs->shared->timer_mutex);
return 0;
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -203,7 +203,7 @@ static unsigned int od_dbs_timer(struct
__cpufreq_driver_target(policy, dbs_info->freq_lo,
CPUFREQ_RELATION_H);
} else {
- dbs_check_cpu(dbs_data, cpu);
+ dbs_check_cpu(policy, cpu);
if (dbs_info->freq_lo) {
/* Setup timer for SUB_SAMPLE */
dbs_info->sample_type = OD_SUB_SAMPLE;
Index: linux-pm/drivers/cpufreq/amd_freq_sensitivity.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/amd_freq_sensitivity.c
+++ linux-pm/drivers/cpufreq/amd_freq_sensitivity.c
@@ -48,7 +48,7 @@ static unsigned int amd_powersave_bias_t
struct dbs_data *od_data = policy->governor_data;
struct od_dbs_tuners *od_tuners = od_data->tuners;
struct od_cpu_dbs_info_s *od_info =
- od_data->gov->get_cpu_dbs_info_s(policy->cpu);
+ dbs_governor_of(policy)->get_cpu_dbs_info_s(policy->cpu);
if (!od_info->freq_table)
return freq_next;
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-05 09:30 +0100 |
| Subject | Re: [PATCH v2 7/10] cpufreq: governor: Drop the gov pointer from struct dbs_data |
| Message-ID | <qYJWa-68A-11@gated-at.bofh.it> |
| In reply to | #1327402 |
On 05-02-16, 03:19, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Since it is possible to obtain a pointer to struct dbs_governor > from a pointer to the struct governor embedded in it with the help > of container_of(), the additional gov pointer in struct dbs_data > isn't really necessary. > > Drop that pointer and make the code using it reach the dbs_governor > object via policy->governor. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/cpufreq/amd_freq_sensitivity.c | 2 - > drivers/cpufreq/cpufreq_conservative.c | 2 - > drivers/cpufreq/cpufreq_governor.c | 63 +++++++++++++++------------------ > drivers/cpufreq/cpufreq_governor.h | 8 +++- > drivers/cpufreq/cpufreq_ondemand.c | 2 - > 5 files changed, 39 insertions(+), 38 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-05 03:30 +0100 |
| Subject | [PATCH v2 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info |
| Message-ID | <qYEjM-2cq-5@gated-at.bofh.it> |
| In reply to | #1326257 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The struct cpu_common_dbs_info structure represents the per-policy
part of the governor data (for the ondemand and conservative
governors), but its name doesn't reflect its purpose.
Rename it to struct policy_dbs_info and rename variables related to
it accordingly.
No functional changes.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq_governor.c | 120 ++++++++++++++++++-------------------
drivers/cpufreq/cpufreq_governor.h | 8 +-
drivers/cpufreq/cpufreq_ondemand.c | 32 ++++-----
3 files changed, 80 insertions(+), 80 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -132,7 +132,7 @@ static void *get_cpu_dbs_info_s(int cpu)
*/
/* Common to all CPUs of a policy */
-struct cpu_common_dbs_info {
+struct policy_dbs_info {
struct cpufreq_policy *policy;
/*
* Per policy mutex that serializes load evaluation from limit-change
@@ -162,7 +162,7 @@ struct cpu_dbs_info {
*/
unsigned int prev_load;
struct update_util_data update_util;
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *shared;
};
struct od_cpu_dbs_info_s {
@@ -276,9 +276,9 @@ static ssize_t show_sampling_rate_min_go
extern struct mutex dbs_data_mutex;
extern struct mutex cpufreq_governor_lock;
-void gov_set_update_util(struct cpu_common_dbs_info *shared,
+void gov_set_update_util(struct policy_dbs_info *policy_dbs,
unsigned int delay_us);
-void gov_cancel_work(struct cpu_common_dbs_info *shared);
+void gov_cancel_work(struct policy_dbs_info *policy_dbs);
void dbs_check_cpu(struct cpufreq_policy *policy, int cpu);
int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event);
void od_register_powersave_bias_handler(unsigned int (*f)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -163,16 +163,16 @@ void dbs_check_cpu(struct cpufreq_policy
}
EXPORT_SYMBOL_GPL(dbs_check_cpu);
-void gov_set_update_util(struct cpu_common_dbs_info *shared,
+void gov_set_update_util(struct policy_dbs_info *policy_dbs,
unsigned int delay_us)
{
- struct cpufreq_policy *policy = shared->policy;
+ struct cpufreq_policy *policy = policy_dbs->policy;
struct dbs_governor *gov = dbs_governor_of(policy);
int cpu;
- shared->sample_delay_ns = delay_us * NSEC_PER_USEC;
- shared->time_stamp = ktime_get();
- shared->last_sample_time = 0;
+ policy_dbs->sample_delay_ns = delay_us * NSEC_PER_USEC;
+ policy_dbs->time_stamp = ktime_get();
+ policy_dbs->last_sample_time = 0;
for_each_cpu(cpu, policy->cpus) {
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
@@ -192,31 +192,31 @@ static inline void gov_clear_update_util
synchronize_rcu();
}
-void gov_cancel_work(struct cpu_common_dbs_info *shared)
+void gov_cancel_work(struct policy_dbs_info *policy_dbs)
{
/* Tell dbs_update_util_handler() to skip queuing up work items. */
- atomic_inc(&shared->skip_work);
+ atomic_inc(&policy_dbs->skip_work);
/*
* If dbs_update_util_handler() is already running, it may not notice
* the incremented skip_work, so wait for it to complete to prevent its
* work item from being queued up after the cancel_work_sync() below.
*/
- gov_clear_update_util(shared->policy);
- wait_for_completion(&shared->irq_work_done);
- cancel_work_sync(&shared->work);
- atomic_set(&shared->skip_work, 0);
+ gov_clear_update_util(policy_dbs->policy);
+ wait_for_completion(&policy_dbs->irq_work_done);
+ cancel_work_sync(&policy_dbs->work);
+ atomic_set(&policy_dbs->skip_work, 0);
}
EXPORT_SYMBOL_GPL(gov_cancel_work);
static void dbs_work_handler(struct work_struct *work)
{
- struct cpu_common_dbs_info *shared = container_of(work, struct
- cpu_common_dbs_info, work);
+ struct policy_dbs_info *policy_dbs;
struct cpufreq_policy *policy;
struct dbs_governor *gov;
unsigned int delay;
- policy = shared->policy;
+ policy_dbs = container_of(work, struct policy_dbs_info, work);
+ policy = policy_dbs->policy;
gov = dbs_governor_of(policy);
/*
@@ -224,11 +224,11 @@ static void dbs_work_handler(struct work
* ondemand governor isn't reading the time stamp and sampling rate in
* parallel.
*/
- mutex_lock(&shared->timer_mutex);
+ mutex_lock(&policy_dbs->timer_mutex);
delay = gov->gov_dbs_timer(policy);
- shared->sample_delay_ns = jiffies_to_nsecs(delay);
- shared->time_stamp = ktime_get();
- mutex_unlock(&shared->timer_mutex);
+ policy_dbs->sample_delay_ns = jiffies_to_nsecs(delay);
+ policy_dbs->time_stamp = ktime_get();
+ mutex_unlock(&policy_dbs->timer_mutex);
/*
* If the atomic operation below is reordered with respect to the
@@ -236,23 +236,23 @@ static void dbs_work_handler(struct work
* up using a stale sample delay value.
*/
smp_mb__before_atomic();
- atomic_dec(&shared->skip_work);
+ atomic_dec(&policy_dbs->skip_work);
}
static void dbs_irq_work(struct irq_work *irq_work)
{
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
- shared = container_of(irq_work, struct cpu_common_dbs_info, irq_work);
- schedule_work(&shared->work);
- complete(&shared->irq_work_done);
+ policy_dbs = container_of(irq_work, struct policy_dbs_info, irq_work);
+ schedule_work(&policy_dbs->work);
+ complete(&policy_dbs->irq_work_done);
}
static void dbs_update_util_handler(struct update_util_data *data, u64 time,
unsigned long util, unsigned long max)
{
struct cpu_dbs_info *cdbs = container_of(data, struct cpu_dbs_info, update_util);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->shared;
/*
* The work may not be allowed to be queued up right now.
@@ -261,18 +261,18 @@ static void dbs_update_util_handler(stru
* - The governor is being stopped.
* - It is too early (too little time from the previous sample).
*/
- if (atomic_inc_return(&shared->skip_work) == 1) {
+ if (atomic_inc_return(&policy_dbs->skip_work) == 1) {
u64 delta_ns;
- delta_ns = time - shared->last_sample_time;
- if ((s64)delta_ns >= shared->sample_delay_ns) {
- shared->last_sample_time = time;
- reinit_completion(&shared->irq_work_done);
- irq_work_queue_on(&shared->irq_work, smp_processor_id());
+ delta_ns = time - policy_dbs->last_sample_time;
+ if ((s64)delta_ns >= policy_dbs->sample_delay_ns) {
+ policy_dbs->last_sample_time = time;
+ reinit_completion(&policy_dbs->irq_work_done);
+ irq_work_queue_on(&policy_dbs->irq_work, smp_processor_id());
return;
}
}
- atomic_dec(&shared->skip_work);
+ atomic_dec(&policy_dbs->skip_work);
}
static void set_sampling_rate(struct dbs_data *dbs_data,
@@ -288,40 +288,40 @@ static void set_sampling_rate(struct dbs
}
}
-static int alloc_common_dbs_info(struct cpufreq_policy *policy,
+static int alloc_policy_dbs_info(struct cpufreq_policy *policy,
struct dbs_governor *gov)
{
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
int j;
/* Allocate memory for the common information for policy->cpus */
- shared = kzalloc(sizeof(*shared), GFP_KERNEL);
- if (!shared)
+ policy_dbs = kzalloc(sizeof(*policy_dbs), GFP_KERNEL);
+ if (!policy_dbs)
return -ENOMEM;
- /* Set shared for all CPUs, online+offline */
+ /* Set policy_dbs for all CPUs, online+offline */
for_each_cpu(j, policy->related_cpus)
- gov->get_cpu_cdbs(j)->shared = shared;
+ gov->get_cpu_cdbs(j)->shared = policy_dbs;
- mutex_init(&shared->timer_mutex);
- atomic_set(&shared->skip_work, 0);
- INIT_WORK(&shared->work, dbs_work_handler);
+ mutex_init(&policy_dbs->timer_mutex);
+ atomic_set(&policy_dbs->skip_work, 0);
+ INIT_WORK(&policy_dbs->work, dbs_work_handler);
return 0;
}
-static void free_common_dbs_info(struct cpufreq_policy *policy,
+static void free_policy_dbs_info(struct cpufreq_policy *policy,
struct dbs_governor *gov)
{
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->shared;
int j;
- mutex_destroy(&shared->timer_mutex);
+ mutex_destroy(&policy_dbs->timer_mutex);
for_each_cpu(j, policy->cpus)
gov->get_cpu_cdbs(j)->shared = NULL;
- kfree(shared);
+ kfree(policy_dbs);
}
static int cpufreq_governor_init(struct cpufreq_policy *policy)
@@ -339,7 +339,7 @@ static int cpufreq_governor_init(struct
if (WARN_ON(have_governor_per_policy()))
return -EINVAL;
- ret = alloc_common_dbs_info(policy, gov);
+ ret = alloc_policy_dbs_info(policy, gov);
if (ret)
return ret;
@@ -352,7 +352,7 @@ static int cpufreq_governor_init(struct
if (!dbs_data)
return -ENOMEM;
- ret = alloc_common_dbs_info(policy, gov);
+ ret = alloc_policy_dbs_info(policy, gov);
if (ret)
goto free_dbs_data;
@@ -360,7 +360,7 @@ static int cpufreq_governor_init(struct
ret = gov->init(dbs_data, !policy->governor->initialized);
if (ret)
- goto free_common_dbs_info;
+ goto free_policy_dbs_info;
/* policy latency is in ns. Convert it to us first */
latency = policy->cpuinfo.transition_latency / 1000;
@@ -391,8 +391,8 @@ reset_gdbs_data:
if (!have_governor_per_policy())
gov->gdbs_data = NULL;
gov->exit(dbs_data, !policy->governor->initialized);
-free_common_dbs_info:
- free_common_dbs_info(policy, gov);
+free_policy_dbs_info:
+ free_policy_dbs_info(policy, gov);
free_dbs_data:
kfree(dbs_data);
return ret;
@@ -423,7 +423,7 @@ static int cpufreq_governor_exit(struct
policy->governor_data = NULL;
}
- free_common_dbs_info(policy, gov);
+ free_policy_dbs_info(policy, gov);
return 0;
}
@@ -433,14 +433,14 @@ static int cpufreq_governor_start(struct
struct dbs_data *dbs_data = policy->governor_data;
unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->shared;
int io_busy = 0;
if (!policy->cur)
return -EINVAL;
/* State should be equivalent to INIT */
- if (!shared || shared->policy)
+ if (!policy_dbs || policy_dbs->policy)
return -EBUSY;
if (gov->governor == GOV_CONSERVATIVE) {
@@ -473,9 +473,9 @@ static int cpufreq_governor_start(struct
j_cdbs->update_util.func = dbs_update_util_handler;
}
- shared->policy = policy;
- init_irq_work(&shared->irq_work, dbs_irq_work);
- init_completion(&shared->irq_work_done);
+ policy_dbs->policy = policy;
+ init_irq_work(&policy_dbs->irq_work, dbs_irq_work);
+ init_completion(&policy_dbs->irq_work_done);
if (gov->governor == GOV_CONSERVATIVE) {
struct cs_cpu_dbs_info_s *cs_dbs_info =
@@ -492,7 +492,7 @@ static int cpufreq_governor_start(struct
od_ops->powersave_bias_init_cpu(cpu);
}
- gov_set_update_util(shared, sampling_rate);
+ gov_set_update_util(policy_dbs, sampling_rate);
return 0;
}
@@ -500,14 +500,14 @@ static int cpufreq_governor_stop(struct
{
struct dbs_governor *gov = dbs_governor_of(policy);
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->shared;
/* State should be equivalent to START */
- if (!shared || !shared->policy)
+ if (!policy_dbs || !policy_dbs->policy)
return -EBUSY;
- gov_cancel_work(shared);
- shared->policy = NULL;
+ gov_cancel_work(policy_dbs);
+ policy_dbs->policy = NULL;
return 0;
}
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -255,21 +255,21 @@ static void update_sampling_rate(struct
struct cpufreq_policy *policy;
struct od_cpu_dbs_info_s *dbs_info;
struct cpu_dbs_info *cdbs;
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
ktime_t next_sampling, appointed_at;
dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
cdbs = &dbs_info->cdbs;
- shared = cdbs->shared;
+ policy_dbs = cdbs->shared;
/*
- * A valid shared and shared->policy means governor hasn't
- * stopped or exited yet.
+ * A valid policy_dbs and policy_dbs->policy means governor
+ * hasn't stopped or exited yet.
*/
- if (!shared || !shared->policy)
+ if (!policy_dbs || !policy_dbs->policy)
continue;
- policy = shared->policy;
+ policy = policy_dbs->policy;
/* clear all CPUs of this policy */
cpumask_andnot(&cpumask, &cpumask, policy->cpus);
@@ -289,14 +289,14 @@ static void update_sampling_rate(struct
*/
next_sampling = ktime_add_us(ktime_get(), new_rate);
- mutex_lock(&shared->timer_mutex);
- appointed_at = ktime_add_ns(shared->time_stamp,
- shared->sample_delay_ns);
- mutex_unlock(&shared->timer_mutex);
+ mutex_lock(&policy_dbs->timer_mutex);
+ appointed_at = ktime_add_ns(policy_dbs->time_stamp,
+ policy_dbs->sample_delay_ns);
+ mutex_unlock(&policy_dbs->timer_mutex);
if (ktime_before(next_sampling, appointed_at)) {
- gov_cancel_work(shared);
- gov_set_update_util(shared, new_rate);
+ gov_cancel_work(policy_dbs);
+ gov_set_update_util(policy_dbs, new_rate);
}
}
@@ -569,16 +569,16 @@ static void od_set_powersave_bias(unsign
get_online_cpus();
for_each_online_cpu(cpu) {
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
if (cpumask_test_cpu(cpu, &done))
continue;
- shared = per_cpu(od_cpu_dbs_info, cpu).cdbs.shared;
- if (!shared)
+ policy_dbs = per_cpu(od_cpu_dbs_info, cpu).cdbs.shared;
+ if (!policy_dbs)
continue;
- policy = shared->policy;
+ policy = policy_dbs->policy;
cpumask_or(&done, &done, policy->cpus);
if (policy->governor != CPU_FREQ_GOV_ONDEMAND)
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-05 09:40 +0100 |
| Subject | Re: [PATCH v2 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info |
| Message-ID | <qYK5Q-6cq-9@gated-at.bofh.it> |
| In reply to | #1327403 |
On 05-02-16, 03:20, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The struct cpu_common_dbs_info structure represents the per-policy
> part of the governor data (for the ondemand and conservative
> governors), but its name doesn't reflect its purpose.
>
> Rename it to struct policy_dbs_info and rename variables related to
> it accordingly.
>
> No functional changes.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/cpufreq_governor.c | 120 ++++++++++++++++++-------------------
> drivers/cpufreq/cpufreq_governor.h | 8 +-
> drivers/cpufreq/cpufreq_ondemand.c | 32 ++++-----
> 3 files changed, 80 insertions(+), 80 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.h
> @@ -132,7 +132,7 @@ static void *get_cpu_dbs_info_s(int cpu)
> */
>
> /* Common to all CPUs of a policy */
> -struct cpu_common_dbs_info {
> +struct policy_dbs_info {
> struct cpufreq_policy *policy;
> /*
> * Per policy mutex that serializes load evaluation from limit-change
> @@ -162,7 +162,7 @@ struct cpu_dbs_info {
> */
> unsigned int prev_load;
> struct update_util_data update_util;
> - struct cpu_common_dbs_info *shared;
> + struct policy_dbs_info *shared;
You should replaced shared with policy_dbs here as well ? Apart from
that:
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-05 23:50 +0100 |
| Subject | Re: [PATCH v2 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info |
| Message-ID | <qYXmq-6ou-9@gated-at.bofh.it> |
| In reply to | #1327534 |
On Friday, February 05, 2016 02:04:06 PM Viresh Kumar wrote:
> On 05-02-16, 03:20, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The struct cpu_common_dbs_info structure represents the per-policy
> > part of the governor data (for the ondemand and conservative
> > governors), but its name doesn't reflect its purpose.
> >
> > Rename it to struct policy_dbs_info and rename variables related to
> > it accordingly.
> >
> > No functional changes.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > drivers/cpufreq/cpufreq_governor.c | 120 ++++++++++++++++++-------------------
> > drivers/cpufreq/cpufreq_governor.h | 8 +-
> > drivers/cpufreq/cpufreq_ondemand.c | 32 ++++-----
> > 3 files changed, 80 insertions(+), 80 deletions(-)
> >
> > Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
> > ===================================================================
> > --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
> > +++ linux-pm/drivers/cpufreq/cpufreq_governor.h
> > @@ -132,7 +132,7 @@ static void *get_cpu_dbs_info_s(int cpu)
> > */
> >
> > /* Common to all CPUs of a policy */
> > -struct cpu_common_dbs_info {
> > +struct policy_dbs_info {
> > struct cpufreq_policy *policy;
> > /*
> > * Per policy mutex that serializes load evaluation from limit-change
> > @@ -162,7 +162,7 @@ struct cpu_dbs_info {
> > */
> > unsigned int prev_load;
> > struct update_util_data update_util;
> > - struct cpu_common_dbs_info *shared;
> > + struct policy_dbs_info *shared;
>
> You should replaced shared with policy_dbs here as well ? Apart from
> that:
I wanted to keep the name as the object pointed to by it is technically
still "shared". I guess I can change it too, though.
Thanks,
Rafael
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-06 13:50 +0100 |
| Subject | [PATCH v3 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info |
| Message-ID | <qZatj-7lm-7@gated-at.bofh.it> |
| In reply to | #1327403 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info
The struct cpu_common_dbs_info structure represents the per-policy
part of the governor data (for the ondemand and conservative
governors), but its name doesn't reflect its purpose.
Rename it to struct policy_dbs_info and rename variables related to
it accordingly.
No functional changes.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Changes from v2:
- Rebase on top of https://patchwork.kernel.org/patch/8241231/
- Change all struct cpu_common_dbs_info pointers to be called policy_dbs.
---
drivers/cpufreq/cpufreq_conservative.c | 2
drivers/cpufreq/cpufreq_governor.c | 134 ++++++++++++++++-----------------
drivers/cpufreq/cpufreq_governor.h | 8 -
drivers/cpufreq/cpufreq_ondemand.c | 28 +++---
4 files changed, 86 insertions(+), 86 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -131,7 +131,7 @@ static void *get_cpu_dbs_info_s(int cpu)
*/
/* Common to all CPUs of a policy */
-struct cpu_common_dbs_info {
+struct policy_dbs_info {
struct cpufreq_policy *policy;
/*
* Per policy mutex that serializes load evaluation from limit-change
@@ -146,10 +146,10 @@ struct cpu_common_dbs_info {
struct work_struct work;
};
-static inline void gov_update_sample_delay(struct cpu_common_dbs_info *shared,
+static inline void gov_update_sample_delay(struct policy_dbs_info *policy_dbs,
unsigned int delay_us)
{
- shared->sample_delay_ns = delay_us * NSEC_PER_USEC;
+ policy_dbs->sample_delay_ns = delay_us * NSEC_PER_USEC;
}
/* Per cpu structures */
@@ -165,7 +165,7 @@ struct cpu_dbs_info {
*/
unsigned int prev_load;
struct update_util_data update_util;
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
};
struct od_cpu_dbs_info_s {
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -163,15 +163,15 @@ void dbs_check_cpu(struct cpufreq_policy
}
EXPORT_SYMBOL_GPL(dbs_check_cpu);
-void gov_set_update_util(struct cpu_common_dbs_info *shared,
+void gov_set_update_util(struct policy_dbs_info *policy_dbs,
unsigned int delay_us)
{
- struct cpufreq_policy *policy = shared->policy;
+ struct cpufreq_policy *policy = policy_dbs->policy;
struct dbs_governor *gov = dbs_governor_of(policy);
int cpu;
- gov_update_sample_delay(shared, delay_us);
- shared->last_sample_time = 0;
+ gov_update_sample_delay(policy_dbs, delay_us);
+ policy_dbs->last_sample_time = 0;
for_each_cpu(cpu, policy->cpus) {
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
@@ -191,40 +191,40 @@ static inline void gov_clear_update_util
synchronize_rcu();
}
-static void gov_cancel_work(struct cpu_common_dbs_info *shared)
+static void gov_cancel_work(struct policy_dbs_info *policy_dbs)
{
/* Tell dbs_update_util_handler() to skip queuing up work items. */
- atomic_inc(&shared->skip_work);
+ atomic_inc(&policy_dbs->skip_work);
/*
* If dbs_update_util_handler() is already running, it may not notice
* the incremented skip_work, so wait for it to complete to prevent its
* work item from being queued up after the cancel_work_sync() below.
*/
- gov_clear_update_util(shared->policy);
- irq_work_sync(&shared->irq_work);
- cancel_work_sync(&shared->work);
- atomic_set(&shared->skip_work, 0);
+ gov_clear_update_util(policy_dbs->policy);
+ irq_work_sync(&policy_dbs->irq_work);
+ cancel_work_sync(&policy_dbs->work);
+ atomic_set(&policy_dbs->skip_work, 0);
}
static void dbs_work_handler(struct work_struct *work)
{
- struct cpu_common_dbs_info *shared = container_of(work, struct
- cpu_common_dbs_info, work);
+ struct policy_dbs_info *policy_dbs;
struct cpufreq_policy *policy;
struct dbs_governor *gov;
unsigned int delay;
- policy = shared->policy;
+ policy_dbs = container_of(work, struct policy_dbs_info, work);
+ policy = policy_dbs->policy;
gov = dbs_governor_of(policy);
/*
* Make sure cpufreq_governor_limits() isn't evaluating load or the
* ondemand governor isn't updating the sampling rate in parallel.
*/
- mutex_lock(&shared->timer_mutex);
+ mutex_lock(&policy_dbs->timer_mutex);
delay = gov->gov_dbs_timer(policy);
- shared->sample_delay_ns = jiffies_to_nsecs(delay);
- mutex_unlock(&shared->timer_mutex);
+ policy_dbs->sample_delay_ns = jiffies_to_nsecs(delay);
+ mutex_unlock(&policy_dbs->timer_mutex);
/*
* If the atomic operation below is reordered with respect to the
@@ -232,30 +232,30 @@ static void dbs_work_handler(struct work
* up using a stale sample delay value.
*/
smp_mb__before_atomic();
- atomic_dec(&shared->skip_work);
+ atomic_dec(&policy_dbs->skip_work);
}
static void dbs_irq_work(struct irq_work *irq_work)
{
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
- shared = container_of(irq_work, struct cpu_common_dbs_info, irq_work);
- schedule_work(&shared->work);
+ policy_dbs = container_of(irq_work, struct policy_dbs_info, irq_work);
+ schedule_work(&policy_dbs->work);
}
-static inline void gov_queue_irq_work(struct cpu_common_dbs_info *shared)
+static inline void gov_queue_irq_work(struct policy_dbs_info *policy_dbs)
{
if (IS_ENABLED(CONFIG_SMP))
- irq_work_queue_on(&shared->irq_work, smp_processor_id());
+ irq_work_queue_on(&policy_dbs->irq_work, smp_processor_id());
else
- irq_work_queue(&shared->irq_work);
+ irq_work_queue(&policy_dbs->irq_work);
}
static void dbs_update_util_handler(struct update_util_data *data, u64 time,
unsigned long util, unsigned long max)
{
struct cpu_dbs_info *cdbs = container_of(data, struct cpu_dbs_info, update_util);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
/*
* The work may not be allowed to be queued up right now.
@@ -264,17 +264,17 @@ static void dbs_update_util_handler(stru
* - The governor is being stopped.
* - It is too early (too little time from the previous sample).
*/
- if (atomic_inc_return(&shared->skip_work) == 1) {
+ if (atomic_inc_return(&policy_dbs->skip_work) == 1) {
u64 delta_ns;
- delta_ns = time - shared->last_sample_time;
- if ((s64)delta_ns >= shared->sample_delay_ns) {
- shared->last_sample_time = time;
- gov_queue_irq_work(shared);
+ delta_ns = time - policy_dbs->last_sample_time;
+ if ((s64)delta_ns >= policy_dbs->sample_delay_ns) {
+ policy_dbs->last_sample_time = time;
+ gov_queue_irq_work(policy_dbs);
return;
}
}
- atomic_dec(&shared->skip_work);
+ atomic_dec(&policy_dbs->skip_work);
}
static void set_sampling_rate(struct dbs_data *dbs_data,
@@ -290,40 +290,40 @@ static void set_sampling_rate(struct dbs
}
}
-static int alloc_common_dbs_info(struct cpufreq_policy *policy,
+static int alloc_policy_dbs_info(struct cpufreq_policy *policy,
struct dbs_governor *gov)
{
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
int j;
/* Allocate memory for the common information for policy->cpus */
- shared = kzalloc(sizeof(*shared), GFP_KERNEL);
- if (!shared)
+ policy_dbs = kzalloc(sizeof(*policy_dbs), GFP_KERNEL);
+ if (!policy_dbs)
return -ENOMEM;
- /* Set shared for all CPUs, online+offline */
+ /* Set policy_dbs for all CPUs, online+offline */
for_each_cpu(j, policy->related_cpus)
- gov->get_cpu_cdbs(j)->shared = shared;
+ gov->get_cpu_cdbs(j)->policy_dbs = policy_dbs;
- mutex_init(&shared->timer_mutex);
- atomic_set(&shared->skip_work, 0);
- INIT_WORK(&shared->work, dbs_work_handler);
+ mutex_init(&policy_dbs->timer_mutex);
+ atomic_set(&policy_dbs->skip_work, 0);
+ INIT_WORK(&policy_dbs->work, dbs_work_handler);
return 0;
}
-static void free_common_dbs_info(struct cpufreq_policy *policy,
+static void free_policy_dbs_info(struct cpufreq_policy *policy,
struct dbs_governor *gov)
{
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
int j;
- mutex_destroy(&shared->timer_mutex);
+ mutex_destroy(&policy_dbs->timer_mutex);
for_each_cpu(j, policy->cpus)
- gov->get_cpu_cdbs(j)->shared = NULL;
+ gov->get_cpu_cdbs(j)->policy_dbs = NULL;
- kfree(shared);
+ kfree(policy_dbs);
}
static int cpufreq_governor_init(struct cpufreq_policy *policy)
@@ -341,7 +341,7 @@ static int cpufreq_governor_init(struct
if (WARN_ON(have_governor_per_policy()))
return -EINVAL;
- ret = alloc_common_dbs_info(policy, gov);
+ ret = alloc_policy_dbs_info(policy, gov);
if (ret)
return ret;
@@ -354,7 +354,7 @@ static int cpufreq_governor_init(struct
if (!dbs_data)
return -ENOMEM;
- ret = alloc_common_dbs_info(policy, gov);
+ ret = alloc_policy_dbs_info(policy, gov);
if (ret)
goto free_dbs_data;
@@ -362,7 +362,7 @@ static int cpufreq_governor_init(struct
ret = gov->init(dbs_data, !policy->governor->initialized);
if (ret)
- goto free_common_dbs_info;
+ goto free_policy_dbs_info;
/* policy latency is in ns. Convert it to us first */
latency = policy->cpuinfo.transition_latency / 1000;
@@ -393,8 +393,8 @@ reset_gdbs_data:
if (!have_governor_per_policy())
gov->gdbs_data = NULL;
gov->exit(dbs_data, !policy->governor->initialized);
-free_common_dbs_info:
- free_common_dbs_info(policy, gov);
+free_policy_dbs_info:
+ free_policy_dbs_info(policy, gov);
free_dbs_data:
kfree(dbs_data);
return ret;
@@ -407,7 +407,7 @@ static int cpufreq_governor_exit(struct
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
/* State should be equivalent to INIT */
- if (!cdbs->shared || cdbs->shared->policy)
+ if (!cdbs->policy_dbs || cdbs->policy_dbs->policy)
return -EBUSY;
if (!--dbs_data->usage_count) {
@@ -425,7 +425,7 @@ static int cpufreq_governor_exit(struct
policy->governor_data = NULL;
}
- free_common_dbs_info(policy, gov);
+ free_policy_dbs_info(policy, gov);
return 0;
}
@@ -435,14 +435,14 @@ static int cpufreq_governor_start(struct
struct dbs_data *dbs_data = policy->governor_data;
unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
int io_busy = 0;
if (!policy->cur)
return -EINVAL;
/* State should be equivalent to INIT */
- if (!shared || shared->policy)
+ if (!policy_dbs || policy_dbs->policy)
return -EBUSY;
if (gov->governor == GOV_CONSERVATIVE) {
@@ -475,8 +475,8 @@ static int cpufreq_governor_start(struct
j_cdbs->update_util.func = dbs_update_util_handler;
}
- shared->policy = policy;
- init_irq_work(&shared->irq_work, dbs_irq_work);
+ policy_dbs->policy = policy;
+ init_irq_work(&policy_dbs->irq_work, dbs_irq_work);
if (gov->governor == GOV_CONSERVATIVE) {
struct cs_cpu_dbs_info_s *cs_dbs_info =
@@ -493,7 +493,7 @@ static int cpufreq_governor_start(struct
od_ops->powersave_bias_init_cpu(cpu);
}
- gov_set_update_util(shared, sampling_rate);
+ gov_set_update_util(policy_dbs, sampling_rate);
return 0;
}
@@ -501,14 +501,14 @@ static int cpufreq_governor_stop(struct
{
struct dbs_governor *gov = dbs_governor_of(policy);
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
- struct cpu_common_dbs_info *shared = cdbs->shared;
+ struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
/* State should be equivalent to START */
- if (!shared || !shared->policy)
+ if (!policy_dbs || !policy_dbs->policy)
return -EBUSY;
- gov_cancel_work(shared);
- shared->policy = NULL;
+ gov_cancel_work(policy_dbs);
+ policy_dbs->policy = NULL;
return 0;
}
@@ -520,18 +520,18 @@ static int cpufreq_governor_limits(struc
struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
/* State should be equivalent to START */
- if (!cdbs->shared || !cdbs->shared->policy)
+ if (!cdbs->policy_dbs || !cdbs->policy_dbs->policy)
return -EBUSY;
- mutex_lock(&cdbs->shared->timer_mutex);
- if (policy->max < cdbs->shared->policy->cur)
- __cpufreq_driver_target(cdbs->shared->policy, policy->max,
+ mutex_lock(&cdbs->policy_dbs->timer_mutex);
+ if (policy->max < cdbs->policy_dbs->policy->cur)
+ __cpufreq_driver_target(cdbs->policy_dbs->policy, policy->max,
CPUFREQ_RELATION_H);
- else if (policy->min > cdbs->shared->policy->cur)
- __cpufreq_driver_target(cdbs->shared->policy, policy->min,
+ else if (policy->min > cdbs->policy_dbs->policy->cur)
+ __cpufreq_driver_target(cdbs->policy_dbs->policy, policy->min,
CPUFREQ_RELATION_L);
dbs_check_cpu(policy, cpu);
- mutex_unlock(&cdbs->shared->timer_mutex);
+ mutex_unlock(&cdbs->policy_dbs->timer_mutex);
return 0;
}
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -151,7 +151,7 @@ static void dbs_freq_increase(struct cpu
static void od_check_cpu(int cpu, unsigned int load)
{
struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
- struct cpufreq_policy *policy = dbs_info->cdbs.shared->policy;
+ struct cpufreq_policy *policy = dbs_info->cdbs.policy_dbs->policy;
struct dbs_data *dbs_data = policy->governor_data;
struct od_dbs_tuners *od_tuners = dbs_data->tuners;
@@ -255,20 +255,20 @@ static void update_sampling_rate(struct
struct cpufreq_policy *policy;
struct od_cpu_dbs_info_s *dbs_info;
struct cpu_dbs_info *cdbs;
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
dbs_info = &per_cpu(od_cpu_dbs_info, cpu);
cdbs = &dbs_info->cdbs;
- shared = cdbs->shared;
+ policy_dbs = cdbs->policy_dbs;
/*
- * A valid shared and shared->policy means governor hasn't
- * stopped or exited yet.
+ * A valid policy_dbs and policy_dbs->policy means governor
+ * hasn't stopped or exited yet.
*/
- if (!shared || !shared->policy)
+ if (!policy_dbs || !policy_dbs->policy)
continue;
- policy = shared->policy;
+ policy = policy_dbs->policy;
/* clear all CPUs of this policy */
cpumask_andnot(&cpumask, &cpumask, policy->cpus);
@@ -280,7 +280,7 @@ static void update_sampling_rate(struct
* multiple policies that are governed by the same dbs_data.
*/
if (dbs_data == policy->governor_data) {
- mutex_lock(&shared->timer_mutex);
+ mutex_lock(&policy_dbs->timer_mutex);
/*
* On 32-bit architectures this may race with the
* sample_delay_ns read in dbs_update_util_handler(),
@@ -299,8 +299,8 @@ static void update_sampling_rate(struct
* too big and it will be corrected next time a sample
* is taken, so it shouldn't be significant.
*/
- gov_update_sample_delay(shared, new_rate);
- mutex_unlock(&shared->timer_mutex);
+ gov_update_sample_delay(policy_dbs, new_rate);
+ mutex_unlock(&policy_dbs->timer_mutex);
}
}
@@ -573,16 +573,16 @@ static void od_set_powersave_bias(unsign
get_online_cpus();
for_each_online_cpu(cpu) {
- struct cpu_common_dbs_info *shared;
+ struct policy_dbs_info *policy_dbs;
if (cpumask_test_cpu(cpu, &done))
continue;
- shared = per_cpu(od_cpu_dbs_info, cpu).cdbs.shared;
- if (!shared)
+ policy_dbs = per_cpu(od_cpu_dbs_info, cpu).cdbs.policy_dbs;
+ if (!policy_dbs)
continue;
- policy = shared->policy;
+ policy = policy_dbs->policy;
cpumask_or(&done, &done, policy->cpus);
if (policy->governor != CPU_FREQ_GOV_ONDEMAND)
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -47,7 +47,7 @@ static inline unsigned int get_freq_targ
static void cs_check_cpu(int cpu, unsigned int load)
{
struct cs_cpu_dbs_info_s *dbs_info = &per_cpu(cs_cpu_dbs_info, cpu);
- struct cpufreq_policy *policy = dbs_info->cdbs.shared->policy;
+ struct cpufreq_policy *policy = dbs_info->cdbs.policy_dbs->policy;
struct dbs_data *dbs_data = policy->governor_data;
struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-07 10:40 +0100 |
| Subject | Re: [PATCH v3 8/10] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info |
| Message-ID | <qZtYZ-49J-3@gated-at.bofh.it> |
| In reply to | #1328295 |
On 06-02-16, 13:48, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: [PATCH] cpufreq: governor: Rename cpu_common_dbs_info to policy_dbs_info > > The struct cpu_common_dbs_info structure represents the per-policy > part of the governor data (for the ondemand and conservative > governors), but its name doesn't reflect its purpose. > > Rename it to struct policy_dbs_info and rename variables related to > it accordingly. > > No functional changes. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-05 03:30 +0100 |
| Subject | [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection |
| Message-ID | <qYEjM-2cq-7@gated-at.bofh.it> |
| In reply to | #1326257 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Every governor relying on the common code in cpufreq_governor.c
has to provide its own mutex in struct common_dbs_data. However,
there actually is no need to have a separate mutex per governor
for this purpose, they may be using the same global mutex just
fine. Accordingly, introduce a single common mutex for that and
drop the mutex field from struct common_dbs_data.
That at least will ensure that the mutex is always present and
initialized regardless of what the particular governors do.
Another benefit is that the common code does not need a pointer to
a governor-related structure to get to the mutex which sometimes
helps.
Finally, it makes the code generally easier to follow.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Saravana Kannan <skannan@codeaurora.org>
---
drivers/cpufreq/cpufreq_conservative.c | 1 -
drivers/cpufreq/cpufreq_governor.c | 7 +++++--
drivers/cpufreq/cpufreq_governor.h | 6 +-----
drivers/cpufreq/cpufreq_ondemand.c | 5 ++---
4 files changed, 8 insertions(+), 11 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -22,6 +22,9 @@
#include "cpufreq_governor.h"
+DEFINE_MUTEX(dbs_data_mutex);
+EXPORT_SYMBOL_GPL(dbs_data_mutex);
+
static struct attribute_group *get_sysfs_attr(struct dbs_data *dbs_data)
{
if (have_governor_per_policy())
@@ -541,7 +544,7 @@ int cpufreq_governor_dbs(struct cpufreq_
int ret;
/* Lock governor to block concurrent initialization of governor */
- mutex_lock(&cdata->mutex);
+ mutex_lock(&dbs_data_mutex);
if (have_governor_per_policy())
dbs_data = policy->governor_data;
@@ -574,7 +577,7 @@ int cpufreq_governor_dbs(struct cpufreq_
}
unlock:
- mutex_unlock(&cdata->mutex);
+ mutex_unlock(&dbs_data_mutex);
return ret;
}
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -225,11 +225,6 @@ struct common_dbs_data {
/* Governor specific ops, see below */
void *gov_ops;
-
- /*
- * Protects governor's data (struct dbs_data and struct common_dbs_data)
- */
- struct mutex mutex;
};
/* Governor Per policy data */
@@ -274,6 +269,7 @@ static ssize_t show_sampling_rate_min_go
return sprintf(buf, "%u\n", dbs_data->min_sampling_rate); \
}
+extern struct mutex dbs_data_mutex;
extern struct mutex cpufreq_governor_lock;
void gov_set_update_util(struct cpu_common_dbs_info *shared,
unsigned int delay_us);
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -249,7 +249,7 @@ static void update_sampling_rate(struct
/*
* Lock governor so that governor start/stop can't execute in parallel.
*/
- mutex_lock(&od_dbs_cdata.mutex);
+ mutex_lock(&dbs_data_mutex);
cpumask_copy(&cpumask, cpu_online_mask);
@@ -302,7 +302,7 @@ static void update_sampling_rate(struct
}
}
- mutex_unlock(&od_dbs_cdata.mutex);
+ mutex_unlock(&dbs_data_mutex);
}
static ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,
@@ -548,7 +548,6 @@ static struct common_dbs_data od_dbs_cda
.gov_ops = &od_ops,
.init = od_init,
.exit = od_exit,
- .mutex = __MUTEX_INITIALIZER(od_dbs_cdata.mutex),
};
static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -368,7 +368,6 @@ static struct common_dbs_data cs_dbs_cda
.gov_check_cpu = cs_check_cpu,
.init = cs_init,
.exit = cs_exit,
- .mutex = __MUTEX_INITIALIZER(cs_dbs_cdata.mutex),
};
static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-05 08:00 +0100 |
| Subject | Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection |
| Message-ID | <qYIx4-57t-11@gated-at.bofh.it> |
| In reply to | #1327404 |
On 05-02-16, 03:14, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Every governor relying on the common code in cpufreq_governor.c > has to provide its own mutex in struct common_dbs_data. However, > there actually is no need to have a separate mutex per governor > for this purpose, they may be using the same global mutex just > fine. Accordingly, introduce a single common mutex for that and > drop the mutex field from struct common_dbs_data. > > That at least will ensure that the mutex is always present and > initialized regardless of what the particular governors do. > > Another benefit is that the common code does not need a pointer to > a governor-related structure to get to the mutex which sometimes > helps. > > Finally, it makes the code generally easier to follow. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Acked-by: Saravana Kannan <skannan@codeaurora.org> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-06 00:00 +0100 |
| Subject | Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection |
| Message-ID | <qYXw6-6sJ-19@gated-at.bofh.it> |
| In reply to | #1327507 |
On Friday, February 05, 2016 12:23:41 PM Viresh Kumar wrote: > On 05-02-16, 03:14, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Every governor relying on the common code in cpufreq_governor.c > > has to provide its own mutex in struct common_dbs_data. However, > > there actually is no need to have a separate mutex per governor > > for this purpose, they may be using the same global mutex just > > fine. Accordingly, introduce a single common mutex for that and > > drop the mutex field from struct common_dbs_data. > > > > That at least will ensure that the mutex is always present and > > initialized regardless of what the particular governors do. > > > > Another benefit is that the common code does not need a pointer to > > a governor-related structure to get to the mutex which sometimes > > helps. > > > > Finally, it makes the code generally easier to follow. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Acked-by: Saravana Kannan <skannan@codeaurora.org> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> Thanks! One more observation here. If we are able to eliminate dbs_data_mutex from update_sampling_rate(), then cpufreq_governor_dbs() becomes the only user of that lock. Further, if we can guarantee that the governor's ->governor callback will always be invoked under policy->rwsem, dbs_data_mutex becomes unnecessary and may be dropped. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-07 10:40 +0100 |
| Subject | Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection |
| Message-ID | <qZtYZ-49J-1@gated-at.bofh.it> |
| In reply to | #1328117 |
On 05-02-16, 23:59, Rafael J. Wysocki wrote: > One more observation here. > > If we are able to eliminate dbs_data_mutex from update_sampling_rate(), > then cpufreq_governor_dbs() becomes the only user of that lock. Further, > if we can guarantee that the governor's ->governor callback will always > be invoked under policy->rwsem, dbs_data_mutex becomes unnecessary and > may be dropped. That will be guaranteed with my 7 patches, which I will rebase and send again. But there are cases where a single dbs_data is going to be used for multiple policies and so relying on policy->rwsem isn't going to be sufficient. But, yeah, we should be able to narrow down the locked area I believe. -- viresh
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-07 15:40 +0100 |
| Subject | Re: [PATCH v2 2/10] cpufreq: governor: Use common mutex for dbs_data protection |
| Message-ID | <qZyFj-7pJ-9@gated-at.bofh.it> |
| In reply to | #1328454 |
On Sunday, February 07, 2016 03:01:12 PM Viresh Kumar wrote: > On 05-02-16, 23:59, Rafael J. Wysocki wrote: > > One more observation here. > > > > If we are able to eliminate dbs_data_mutex from update_sampling_rate(), > > then cpufreq_governor_dbs() becomes the only user of that lock. Further, > > if we can guarantee that the governor's ->governor callback will always > > be invoked under policy->rwsem, dbs_data_mutex becomes unnecessary and > > may be dropped. > > That will be guaranteed with my 7 patches, which I will rebase and send again. > > But there are cases where a single dbs_data is going to be used for multiple > policies and so relying on policy->rwsem isn't going to be sufficient. > > But, yeah, we should be able to narrow down the locked area I believe. That should only be a matter of protecting the gov->gdbs_data object and its refcount then. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2016-02-05 03:30 +0100 |
| Subject | [PATCH v2 6/10] cpufreq: governor: Rework cpufreq_governor_dbs() |
| Message-ID | <qYEjM-2cq-9@gated-at.bofh.it> |
| In reply to | #1326257 |
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Since it is possible to obtain a pointer to struct dbs_governor
from a pointer to the struct governor embedded in it via
container_of(), the second argument of cpufreq_governor_init()
is not necessary. Accordingly, cpufreq_governor_dbs() doesn't
need its second argument either and the ->governor callbacks
for both the ondemand and conservative governors may be set
to cpufreq_governor_dbs() directly. Make that happen.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Saravana Kannan <skannan@codeaurora.org>
---
drivers/cpufreq/cpufreq_conservative.c | 11 +----------
drivers/cpufreq/cpufreq_governor.c | 10 +++++-----
drivers/cpufreq/cpufreq_governor.h | 3 +--
drivers/cpufreq/cpufreq_ondemand.c | 11 +----------
4 files changed, 8 insertions(+), 27 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -325,13 +325,10 @@ static void cs_exit(struct dbs_data *dbs
define_get_cpu_dbs_routines(cs_cpu_dbs_info);
-static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
- unsigned int event);
-
static struct dbs_governor cs_dbs_gov = {
.gov = {
.name = "conservative",
- .governor = cs_cpufreq_governor_dbs,
+ .governor = cpufreq_governor_dbs,
.max_transition_latency = TRANSITION_LATENCY_LIMIT,
.owner = THIS_MODULE,
},
@@ -348,12 +345,6 @@ static struct dbs_governor cs_dbs_gov =
#define CPU_FREQ_GOV_CONSERVATIVE (&cs_dbs_gov.gov)
-static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
- unsigned int event)
-{
- return cpufreq_governor_dbs(policy, &cs_dbs_gov, event);
-}
-
static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
void *data)
{
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -324,9 +324,10 @@ static void free_common_dbs_info(struct
kfree(shared);
}
-static int cpufreq_governor_init(struct cpufreq_policy *policy,
- struct dbs_governor *gov)
+static int cpufreq_governor_init(struct cpufreq_policy *policy)
{
+ struct dbs_governor *gov = container_of(policy->governor,
+ struct dbs_governor, gov);
struct dbs_data *dbs_data = gov->gdbs_data;
unsigned int latency;
int ret;
@@ -537,8 +538,7 @@ static int cpufreq_governor_limits(struc
return 0;
}
-int cpufreq_governor_dbs(struct cpufreq_policy *policy,
- struct dbs_governor *gov, unsigned int event)
+int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event)
{
int ret = -EINVAL;
@@ -546,7 +546,7 @@ int cpufreq_governor_dbs(struct cpufreq_
mutex_lock(&dbs_data_mutex);
if (event == CPUFREQ_GOV_POLICY_INIT) {
- ret = cpufreq_governor_init(policy, gov);
+ ret = cpufreq_governor_init(policy);
} else if (policy->governor_data) {
switch (event) {
case CPUFREQ_GOV_POLICY_EXIT:
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -276,8 +276,7 @@ void gov_set_update_util(struct cpu_comm
unsigned int delay_us);
void gov_cancel_work(struct cpu_common_dbs_info *shared);
void dbs_check_cpu(struct dbs_data *dbs_data, int cpu);
-int cpufreq_governor_dbs(struct cpufreq_policy *policy,
- struct dbs_governor *gov, unsigned int event);
+int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event);
void od_register_powersave_bias_handler(unsigned int (*f)
(struct cpufreq_policy *, unsigned int, unsigned int),
unsigned int powersave_bias);
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -535,13 +535,10 @@ static struct od_ops od_ops = {
.freq_increase = dbs_freq_increase,
};
-static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
- unsigned int event);
-
static struct dbs_governor od_dbs_gov = {
.gov = {
.name = "ondemand",
- .governor = od_cpufreq_governor_dbs,
+ .governor = cpufreq_governor_dbs,
.max_transition_latency = TRANSITION_LATENCY_LIMIT,
.owner = THIS_MODULE,
},
@@ -559,12 +556,6 @@ static struct dbs_governor od_dbs_gov =
#define CPU_FREQ_GOV_ONDEMAND (&od_dbs_gov.gov)
-static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
- unsigned int event)
-{
- return cpufreq_governor_dbs(policy, &od_dbs_gov, event);
-}
-
static void od_set_powersave_bias(unsigned int powersave_bias)
{
struct cpufreq_policy *policy;
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2016-02-05 09:20 +0100 |
| Subject | Re: [PATCH v2 6/10] cpufreq: governor: Rework cpufreq_governor_dbs() |
| Message-ID | <qYJMu-64X-7@gated-at.bofh.it> |
| In reply to | #1327405 |
On 05-02-16, 03:18, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Since it is possible to obtain a pointer to struct dbs_governor > from a pointer to the struct governor embedded in it via > container_of(), the second argument of cpufreq_governor_init() > is not necessary. Accordingly, cpufreq_governor_dbs() doesn't > need its second argument either and the ->governor callbacks > for both the ondemand and conservative governors may be set > to cpufreq_governor_dbs() directly. Make that happen. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Acked-by: Saravana Kannan <skannan@codeaurora.org> > --- > drivers/cpufreq/cpufreq_conservative.c | 11 +---------- > drivers/cpufreq/cpufreq_governor.c | 10 +++++----- > drivers/cpufreq/cpufreq_governor.h | 3 +-- > drivers/cpufreq/cpufreq_ondemand.c | 11 +---------- > 4 files changed, 8 insertions(+), 27 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh
[toc] | [prev] | [next] | [standalone]
Page 1 of 3 [1] 2 3 Next page →
Back to top | Article view | linux.kernel
csiph-web