Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1326257 > unrolled thread

[PATCH 0/11] cpufreq: governor: ondemand/conservative data structures rework

Back to article view | Back to linux.kernel

Contents

  [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 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1327406 — [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The struct policy_dbs_info objects representing per-policy governor
data are not accessible directly from the corresponding policy
objects.  To access them, one has to get a pointer to the
struct cpu_dbs_info of policy->cpu and use the "shared" field of
that which isn't really straightforward.

To address that rearrange the governor data structures so the
governor_data pointer in struct cpufreq_policy will point to
struct policy_dbs_info and that will contain a pointer to
struct dbs_data.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/amd_freq_sensitivity.c |    3 
 drivers/cpufreq/cpufreq_conservative.c |    6 +
 drivers/cpufreq/cpufreq_governor.c     |  120 ++++++++++++++-------------------
 drivers/cpufreq/cpufreq_governor.h     |   27 ++++---
 drivers/cpufreq/cpufreq_ondemand.c     |   18 +++-
 5 files changed, 85 insertions(+), 89 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -86,7 +86,8 @@ static ssize_t show_##file_name##_gov_sy
 static ssize_t show_##file_name##_gov_pol				\
 (struct cpufreq_policy *policy, char *buf)				\
 {									\
-	struct dbs_data *dbs_data = policy->governor_data;		\
+	struct policy_dbs_info *policy_dbs = policy->governor_data;	\
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;		\
 	struct _gov##_dbs_tuners *tuners = dbs_data->tuners;		\
 	return sprintf(buf, "%u\n", tuners->file_name);			\
 }
@@ -102,8 +103,8 @@ static ssize_t store_##file_name##_gov_s
 static ssize_t store_##file_name##_gov_pol				\
 (struct cpufreq_policy *policy, const char *buf, size_t count)		\
 {									\
-	struct dbs_data *dbs_data = policy->governor_data;		\
-	return store_##file_name(dbs_data, buf, count);			\
+	struct policy_dbs_info *policy_dbs = policy->governor_data;	\
+	return store_##file_name(policy_dbs->dbs_data, buf, count);			\
 }
 
 #define show_store_one(_gov, file_name)					\
@@ -131,6 +132,13 @@ static void *get_cpu_dbs_info_s(int cpu)
  * cs_*: Conservative governor
  */
 
+/* Governor demand based switching data (per-policy or global). */
+struct dbs_data {
+	unsigned int min_sampling_rate;
+	int usage_count;
+	void *tuners;
+};
+
 /* Common to all CPUs of a policy */
 struct policy_dbs_info {
 	struct cpufreq_policy *policy;
@@ -147,6 +155,8 @@ struct policy_dbs_info {
 	struct irq_work irq_work;
 	struct completion irq_work_done;
 	struct work_struct work;
+	/* dbs_data may be shared between multiple policy objects */
+	struct dbs_data *dbs_data;
 };
 
 /* Per cpu structures */
@@ -201,7 +211,6 @@ struct cs_dbs_tuners {
 };
 
 /* Common Governor data across policies */
-struct dbs_data;
 struct dbs_governor {
 	struct cpufreq_governor gov;
 
@@ -233,13 +242,6 @@ static inline struct dbs_governor *dbs_g
 	return container_of(policy->governor, struct dbs_governor, gov);
 }
 
-/* Governor Per policy data */
-struct dbs_data {
-	unsigned int min_sampling_rate;
-	int usage_count;
-	void *tuners;
-};
-
 /* Governor specific ops, will be passed to dbs_data->gov_ops */
 struct od_ops {
 	void (*powersave_bias_init_cpu)(int cpu);
@@ -270,7 +272,8 @@ static ssize_t show_sampling_rate_min_go
 static ssize_t show_sampling_rate_min_gov_pol				\
 (struct cpufreq_policy *policy, char *buf)				\
 {									\
-	struct dbs_data *dbs_data = policy->governor_data;		\
+	struct policy_dbs_info *policy_dbs = policy->governor_data;	\
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;		\
 	return sprintf(buf, "%u\n", dbs_data->min_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,8 +34,8 @@ static struct attribute_group *get_sysfs
 void dbs_check_cpu(struct cpufreq_policy *policy, int 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 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;
 	unsigned int sampling_rate;
@@ -94,6 +94,7 @@ void dbs_check_cpu(struct cpufreq_policy
 		j_cdbs->prev_cpu_idle = cur_idle_time;
 
 		if (ignore_nice) {
+			struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
 			u64 cur_nice;
 			unsigned long cur_nice_jiffies;
 
@@ -288,8 +289,8 @@ static void set_sampling_rate(struct dbs
 	}
 }
 
-static int alloc_policy_dbs_info(struct cpufreq_policy *policy,
-				 struct dbs_governor *gov)
+static struct policy_dbs_info *alloc_policy_dbs_info(struct cpufreq_policy *policy,
+						     struct dbs_governor *gov)
 {
 	struct policy_dbs_info *policy_dbs;
 	int j;
@@ -297,16 +298,22 @@ static int alloc_policy_dbs_info(struct
 	/* Allocate memory for the common information for policy->cpus */
 	policy_dbs = kzalloc(sizeof(*policy_dbs), GFP_KERNEL);
 	if (!policy_dbs)
-		return -ENOMEM;
-
-	/* Set policy_dbs for all CPUs, online+offline */
-	for_each_cpu(j, policy->related_cpus)
-		gov->get_cpu_cdbs(j)->shared = policy_dbs;
+		return NULL;
 
+	policy_dbs->policy = policy;
 	mutex_init(&policy_dbs->timer_mutex);
 	atomic_set(&policy_dbs->skip_work, 0);
+	init_irq_work(&policy_dbs->irq_work, dbs_irq_work);
+	init_completion(&policy_dbs->irq_work_done);
 	INIT_WORK(&policy_dbs->work, dbs_work_handler);
-	return 0;
+	/* Set policy_dbs for all CPUs, online+offline */
+	for_each_cpu(j, policy->related_cpus) {
+		struct cpu_dbs_info *j_cdbs = gov->get_cpu_cdbs(j);
+
+		j_cdbs->shared = policy_dbs;
+		j_cdbs->update_util.func = dbs_update_util_handler;
+	}
+	return policy_dbs;
 }
 
 static void free_policy_dbs_info(struct cpufreq_policy *policy,
@@ -328,6 +335,7 @@ static int cpufreq_governor_init(struct
 {
 	struct dbs_governor *gov = dbs_governor_of(policy);
 	struct dbs_data *dbs_data = gov->gdbs_data;
+	struct policy_dbs_info *policy_dbs;
 	unsigned int latency;
 	int ret;
 
@@ -335,26 +343,26 @@ static int cpufreq_governor_init(struct
 	if (policy->governor_data)
 		return -EBUSY;
 
-	if (dbs_data) {
-		if (WARN_ON(have_governor_per_policy()))
-			return -EINVAL;
-
-		ret = alloc_policy_dbs_info(policy, gov);
-		if (ret)
-			return ret;
+	policy_dbs = alloc_policy_dbs_info(policy, gov);
+	if (!policy_dbs)
+		return -ENOMEM;
 
+	if (dbs_data) {
+		if (WARN_ON(have_governor_per_policy())) {
+			ret = -EINVAL;
+			goto free_policy_dbs_info;
+		}
 		dbs_data->usage_count++;
-		policy->governor_data = dbs_data;
+		policy_dbs->dbs_data = dbs_data;
+		policy->governor_data = policy_dbs;
 		return 0;
 	}
 
 	dbs_data = kzalloc(sizeof(*dbs_data), GFP_KERNEL);
-	if (!dbs_data)
-		return -ENOMEM;
-
-	ret = alloc_policy_dbs_info(policy, gov);
-	if (ret)
-		goto free_dbs_data;
+	if (!dbs_data) {
+		ret = -ENOMEM;
+		goto free_policy_dbs_info;
+	}
 
 	dbs_data->usage_count = 1;
 
@@ -376,7 +384,8 @@ static int cpufreq_governor_init(struct
 	if (!have_governor_per_policy())
 		gov->gdbs_data = dbs_data;
 
-	policy->governor_data = dbs_data;
+	policy_dbs->dbs_data = dbs_data;
+	policy->governor_data = policy_dbs;
 
 	ret = sysfs_create_group(get_governor_parent_kobj(policy),
 				 get_sysfs_attr(gov));
@@ -391,38 +400,35 @@ reset_gdbs_data:
 	if (!have_governor_per_policy())
 		gov->gdbs_data = NULL;
 	gov->exit(dbs_data, !policy->governor->initialized);
+	kfree(dbs_data);
+
 free_policy_dbs_info:
 	free_policy_dbs_info(policy, gov);
-free_dbs_data:
-	kfree(dbs_data);
 	return ret;
 }
 
 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 cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;
 
 	/* State should be equivalent to INIT */
-	if (!cdbs->shared || cdbs->shared->policy)
+	if (!dbs_data)
 		return -EBUSY;
 
 	if (!--dbs_data->usage_count) {
 		sysfs_remove_group(get_governor_parent_kobj(policy),
 				   get_sysfs_attr(gov));
 
-		policy->governor_data = NULL;
-
 		if (!have_governor_per_policy())
 			gov->gdbs_data = NULL;
 
 		gov->exit(dbs_data, policy->governor->initialized == 1);
 		kfree(dbs_data);
-	} else {
-		policy->governor_data = NULL;
 	}
 
+	policy->governor_data = NULL;
 	free_policy_dbs_info(policy, gov);
 	return 0;
 }
@@ -430,17 +436,16 @@ 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 policy_dbs_info *policy_dbs = policy->governor_data;
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;
 	unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
-	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
-	struct policy_dbs_info *policy_dbs = cdbs->shared;
 	int io_busy = 0;
 
 	if (!policy->cur)
 		return -EINVAL;
 
 	/* State should be equivalent to INIT */
-	if (!policy_dbs || policy_dbs->policy)
+	if (!dbs_data)
 		return -EBUSY;
 
 	if (gov->governor == GOV_CONSERVATIVE) {
@@ -470,12 +475,7 @@ static int cpufreq_governor_start(struct
 
 		if (ignore_nice)
 			j_cdbs->prev_cpu_nice = kcpustat_cpu(j).cpustat[CPUTIME_NICE];
-
-		j_cdbs->update_util.func = dbs_update_util_handler;
 	}
-	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 =
@@ -498,39 +498,25 @@ static int cpufreq_governor_start(struct
 
 static int cpufreq_governor_stop(struct cpufreq_policy *policy)
 {
-	struct dbs_governor *gov = dbs_governor_of(policy);
-	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
-	struct policy_dbs_info *policy_dbs = cdbs->shared;
-
-	/* State should be equivalent to START */
-	if (!policy_dbs || !policy_dbs->policy)
-		return -EBUSY;
-
-	gov_cancel_work(policy_dbs);
-	policy_dbs->policy = NULL;
-
+	gov_cancel_work(policy->governor_data);
 	return 0;
 }
 
 static int cpufreq_governor_limits(struct cpufreq_policy *policy)
 {
-	struct dbs_governor *gov = dbs_governor_of(policy);
-	unsigned int cpu = policy->cpu;
-	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
 
 	/* State should be equivalent to START */
-	if (!cdbs->shared || !cdbs->shared->policy)
+	if (!policy_dbs->dbs_data)
 		return -EBUSY;
 
-	mutex_lock(&cdbs->shared->timer_mutex);
-	if (policy->max < cdbs->shared->policy->cur)
-		__cpufreq_driver_target(cdbs->shared->policy, policy->max,
-					CPUFREQ_RELATION_H);
-	else if (policy->min > cdbs->shared->policy->cur)
-		__cpufreq_driver_target(cdbs->shared->policy, policy->min,
-					CPUFREQ_RELATION_L);
-	dbs_check_cpu(policy, cpu);
-	mutex_unlock(&cdbs->shared->timer_mutex);
+	mutex_lock(&policy_dbs->timer_mutex);
+	if (policy->max < policy->cur)
+		__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);
+	mutex_unlock(&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
@@ -78,7 +78,8 @@ static unsigned int generic_powersave_bi
 	unsigned int jiffies_total, jiffies_hi, jiffies_lo;
 	struct od_cpu_dbs_info_s *dbs_info = &per_cpu(od_cpu_dbs_info,
 						   policy->cpu);
-	struct dbs_data *dbs_data = policy->governor_data;
+	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;
 
 	if (!dbs_info->freq_table) {
@@ -130,7 +131,8 @@ static void ondemand_powersave_bias_init
 
 static void dbs_freq_increase(struct cpufreq_policy *policy, unsigned int freq)
 {
-	struct dbs_data *dbs_data = policy->governor_data;
+	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;
 
 	if (od_tuners->powersave_bias)
@@ -151,8 +153,9 @@ 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 dbs_data *dbs_data = policy->governor_data;
+	struct policy_dbs_info *policy_dbs = dbs_info->cdbs.shared;
+	struct cpufreq_policy *policy = policy_dbs->policy;
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;
 	struct od_dbs_tuners *od_tuners = dbs_data->tuners;
 
 	dbs_info->freq_lo = 0;
@@ -189,7 +192,8 @@ static void od_check_cpu(int cpu, unsign
 
 static unsigned int od_dbs_timer(struct cpufreq_policy *policy)
 {
-	struct dbs_data *dbs_data = policy->governor_data;
+	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);
@@ -280,7 +284,7 @@ static void update_sampling_rate(struct
 		 * policy will be governed by dbs_data, otherwise there can be
 		 * multiple policies that are governed by the same dbs_data.
 		 */
-		if (dbs_data != policy->governor_data)
+		if (dbs_data != policy_dbs->dbs_data)
 			continue;
 
 		/*
@@ -584,7 +588,7 @@ static void od_set_powersave_bias(unsign
 		if (policy->governor != CPU_FREQ_GOV_ONDEMAND)
 			continue;
 
-		dbs_data = policy->governor_data;
+		dbs_data = policy_dbs->dbs_data;
 		od_tuners = dbs_data->tuners;
 		od_tuners->powersave_bias = default_powersave_bias;
 	}
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
@@ -45,7 +45,8 @@ static unsigned int amd_powersave_bias_t
 	long d_actual, d_reference;
 	struct msr actual, reference;
 	struct cpu_data_t *data = &per_cpu(cpu_data, policy->cpu);
-	struct dbs_data *od_data = policy->governor_data;
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
+	struct dbs_data *od_data = policy_dbs->dbs_data;
 	struct od_dbs_tuners *od_tuners = od_data->tuners;
 	struct od_cpu_dbs_info_s *od_info =
 		dbs_governor_of(policy)->get_cpu_dbs_info_s(policy->cpu);
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -48,7 +48,8 @@ static void cs_check_cpu(int cpu, unsign
 {
 	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 dbs_data *dbs_data = policy->governor_data;
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 
 	/*
@@ -104,7 +105,8 @@ static void cs_check_cpu(int cpu, unsign
 
 static unsigned int cs_dbs_timer(struct cpufreq_policy *policy)
 {
-	struct dbs_data *dbs_data = policy->governor_data;
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
+	struct dbs_data *dbs_data = policy_dbs->dbs_data;
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 
 	dbs_check_cpu(policy, policy->cpu);

[toc] | [prev] | [next] | [standalone]

#1327553 — Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures

On 05-02-16, 03:21, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The struct policy_dbs_info objects representing per-policy governor
> data are not accessible directly from the corresponding policy
> objects.  To access them, one has to get a pointer to the
> struct cpu_dbs_info of policy->cpu and use the "shared" field of
> that which isn't really straightforward.
> 
> To address that rearrange the governor data structures so the
> governor_data pointer in struct cpufreq_policy will point to
> struct policy_dbs_info and that will contain a pointer to
> struct dbs_data.

IMHO, this patch has done way too much over what's mentioned here.

> Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
> @@ -297,16 +298,22 @@ static int alloc_policy_dbs_info(struct
>  	/* Allocate memory for the common information for policy->cpus */
>  	policy_dbs = kzalloc(sizeof(*policy_dbs), GFP_KERNEL);
>  	if (!policy_dbs)
> -		return -ENOMEM;
> -
> -	/* Set policy_dbs for all CPUs, online+offline */
> -	for_each_cpu(j, policy->related_cpus)
> -		gov->get_cpu_cdbs(j)->shared = policy_dbs;
> +		return NULL;
>  
> +	policy_dbs->policy = policy;

Value of policy_dbs->policy was used to verify the state machine of
the governor and so was updated only in start/stop.

You have moved it to INIT first (which shouldn't have been part of
this patch at the least), and then there is no reasoning given on why
that isn't required as part of the state machine now, which I believe
is still required the way it was.

>  	mutex_init(&policy_dbs->timer_mutex);
>  	atomic_set(&policy_dbs->skip_work, 0);
> +	init_irq_work(&policy_dbs->irq_work, dbs_irq_work);
> +	init_completion(&policy_dbs->irq_work_done);
>  	INIT_WORK(&policy_dbs->work, dbs_work_handler);
> -	return 0;
> +	/* Set policy_dbs for all CPUs, online+offline */
> +	for_each_cpu(j, policy->related_cpus) {
> +		struct cpu_dbs_info *j_cdbs = gov->get_cpu_cdbs(j);
> +
> +		j_cdbs->shared = policy_dbs;
> +		j_cdbs->update_util.func = dbs_update_util_handler;
> +	}

All other initializations are moved here for the good, but I think it
could have been done in a separate patch as to make review of this
trivial patch, which isn't so easy to review, easy :)

>  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 cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
> +	struct policy_dbs_info *policy_dbs = policy->governor_data;
> +	struct dbs_data *dbs_data = policy_dbs->dbs_data;
>  
>  	/* State should be equivalent to INIT */
> -	if (!cdbs->shared || cdbs->shared->policy)

For example, to make sure the current state is equivalent to INIT, we
had two checks earlier:
- First one made sure that cdbs->shared is allocated (which was done
  in INIT)
- And second one makes sure that shared->policy is NULL, as that is
  initialized in START.

> +	if (!dbs_data)
>  		return -EBUSY;

But now, the current state can be INIT or START, you can't
differentiate.

>  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 policy_dbs_info *policy_dbs = policy->governor_data;
> +	struct dbs_data *dbs_data = policy_dbs->dbs_data;
>  	unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
> -	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
> -	struct policy_dbs_info *policy_dbs = cdbs->shared;
>  	int io_busy = 0;
>  
>  	if (!policy->cur)
>  		return -EINVAL;
>  
>  	/* State should be equivalent to INIT */
> -	if (!policy_dbs || policy_dbs->policy)
> +	if (!dbs_data)

Same here.. 

>  static int cpufreq_governor_stop(struct cpufreq_policy *policy)
>  {
> -	struct dbs_governor *gov = dbs_governor_of(policy);
> -	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
> -	struct policy_dbs_info *policy_dbs = cdbs->shared;
> -
> -	/* State should be equivalent to START */
> -	if (!policy_dbs || !policy_dbs->policy)

And here ..

> -		return -EBUSY;
> -
> -	gov_cancel_work(policy_dbs);
> -	policy_dbs->policy = NULL;
> -
> +	gov_cancel_work(policy->governor_data);
>  	return 0;
>  }
>  
>  static int cpufreq_governor_limits(struct cpufreq_policy *policy)
>  {
> -	struct dbs_governor *gov = dbs_governor_of(policy);
> -	unsigned int cpu = policy->cpu;
> -	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
> +	struct policy_dbs_info *policy_dbs = policy->governor_data;
>  
>  	/* State should be equivalent to START */
> -	if (!cdbs->shared || !cdbs->shared->policy)
> +	if (!policy_dbs->dbs_data)

And here...

>  		return -EBUSY;
>  
> -	mutex_lock(&cdbs->shared->timer_mutex);
> -	if (policy->max < cdbs->shared->policy->cur)
> -		__cpufreq_driver_target(cdbs->shared->policy, policy->max,
> -					CPUFREQ_RELATION_H);
> -	else if (policy->min > cdbs->shared->policy->cur)
> -		__cpufreq_driver_target(cdbs->shared->policy, policy->min,
> -					CPUFREQ_RELATION_L);
> -	dbs_check_cpu(policy, cpu);
> -	mutex_unlock(&cdbs->shared->timer_mutex);
> +	mutex_lock(&policy_dbs->timer_mutex);
> +	if (policy->max < policy->cur)
> +		__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);
> +	mutex_unlock(&policy_dbs->timer_mutex);
>  
>  	return 0;
>  }

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1328111 — Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures

On Friday, February 05, 2016 02:43:57 PM Viresh Kumar wrote:
> On 05-02-16, 03:21, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > The struct policy_dbs_info objects representing per-policy governor
> > data are not accessible directly from the corresponding policy
> > objects.  To access them, one has to get a pointer to the
> > struct cpu_dbs_info of policy->cpu and use the "shared" field of
> > that which isn't really straightforward.
> > 
> > To address that rearrange the governor data structures so the
> > governor_data pointer in struct cpufreq_policy will point to
> > struct policy_dbs_info and that will contain a pointer to
> > struct dbs_data.
> 
> IMHO, this patch has done way too much over what's mentioned here.
> 
> > Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
> > @@ -297,16 +298,22 @@ static int alloc_policy_dbs_info(struct
> >  	/* Allocate memory for the common information for policy->cpus */
> >  	policy_dbs = kzalloc(sizeof(*policy_dbs), GFP_KERNEL);
> >  	if (!policy_dbs)
> > -		return -ENOMEM;
> > -
> > -	/* Set policy_dbs for all CPUs, online+offline */
> > -	for_each_cpu(j, policy->related_cpus)
> > -		gov->get_cpu_cdbs(j)->shared = policy_dbs;
> > +		return NULL;
> >  
> > +	policy_dbs->policy = policy;
> 
> Value of policy_dbs->policy was used to verify the state machine of
> the governor and so was updated only in start/stop.
> 
> You have moved it to INIT first (which shouldn't have been part of
> this patch at the least),

Why?

> and then there is no reasoning given on why
> that isn't required as part of the state machine now, which I believe
> is still required the way it was.

No, it isn't required.  The whole "state machine" isn't required IMO.

The only user of this is the cpufreq core, so why does the code here have to
double check what the core is doing?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1328451 — Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures

On 05-02-16, 23:47, Rafael J. Wysocki wrote:
> On Friday, February 05, 2016 02:43:57 PM Viresh Kumar wrote:
> > Value of policy_dbs->policy was used to verify the state machine of
> > the governor and so was updated only in start/stop.
> > 
> > You have moved it to INIT first (which shouldn't have been part of
> > this patch at the least),
> 
> Why?

Because it doesn't match $SUBJECT at all..

> > and then there is no reasoning given on why
> > that isn't required as part of the state machine now, which I believe
> > is still required the way it was.
> 
> No, it isn't required.  The whole "state machine" isn't required IMO.

The state machine wasn't required if the core wasn't buggy. Its buggy because we
drop policy->rwsem during set-policy, before calling EXIT. And other
__cpufreq_governor() calls can shoot up at that point of time.

We have seen lots of crashes earlier and so the state machine was introduced to
get them fixed.

It might not be required (after making sure things are working fine now), after
applying my patch series of 7 patches. As that fixes the lock-drop issue ..

> The only user of this is the cpufreq core, so why does the code here have to
> double check what the core is doing?

Because, core doesn't guarantee the order today.

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1328493 — Re: [PATCH v2 9/10] cpufreq: governor: Rearrange governor data structures

On Sunday, February 07, 2016 02:59:11 PM Viresh Kumar wrote:
> On 05-02-16, 23:47, Rafael J. Wysocki wrote:
> > On Friday, February 05, 2016 02:43:57 PM Viresh Kumar wrote:
> > > Value of policy_dbs->policy was used to verify the state machine of
> > > the governor and so was updated only in start/stop.
> > > 
> > > You have moved it to INIT first (which shouldn't have been part of
> > > this patch at the least),
> > 
> > Why?
> 
> Because it doesn't match $SUBJECT at all..
> 
> > > and then there is no reasoning given on why
> > > that isn't required as part of the state machine now, which I believe
> > > is still required the way it was.
> > 
> > No, it isn't required.  The whole "state machine" isn't required IMO.
> 
> The state machine wasn't required if the core wasn't buggy. Its buggy because we
> drop policy->rwsem during set-policy, before calling EXIT. And other
> __cpufreq_governor() calls can shoot up at that point of time.
> 
> We have seen lots of crashes earlier and so the state machine was introduced to
> get them fixed.
> 
> It might not be required (after making sure things are working fine now), after
> applying my patch series of 7 patches. As that fixes the lock-drop issue ..
> 
> > The only user of this is the cpufreq core, so why does the code here have to
> > double check what the core is doing?
> 
> Because, core doesn't guarantee the order today.

OK, so I have reworked this.  I have a series of 3 patches now instead of it
that I'm going to post shortly.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1327407 — [PATCH v2 1/10] cpufreq: Clean up default and fallback governor setup

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The preprocessor magic used for setting the default cpufreq governor
(and for using the performance governor as a fallback one for that
matter) is really nasty, so replace it with __weak functions and
overrides.

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>
---
 drivers/cpufreq/cpufreq.c              |   37 ++++++++++++++++++---------------
 drivers/cpufreq/cpufreq_conservative.c |   10 +++++---
 drivers/cpufreq/cpufreq_ondemand.c     |   36 ++++++++++++++++----------------
 drivers/cpufreq/cpufreq_performance.c  |   18 ++++++++++++----
 drivers/cpufreq/cpufreq_powersave.c    |   10 +++++---
 drivers/cpufreq/cpufreq_userspace.c    |   10 +++++---
 include/linux/cpufreq.h                |   25 +---------------------
 7 files changed, 73 insertions(+), 73 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -1003,6 +1003,11 @@ static int cpufreq_add_dev_interface(str
 	return cpufreq_add_dev_symlink(policy);
 }
 
+__weak struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return NULL;
+}
+
 static int cpufreq_init_policy(struct cpufreq_policy *policy)
 {
 	struct cpufreq_governor *gov = NULL;
@@ -1012,11 +1017,14 @@ static int cpufreq_init_policy(struct cp
 
 	/* Update governor of new_policy to the governor used before hotplug */
 	gov = find_governor(policy->last_governor);
-	if (gov)
+	if (gov) {
 		pr_debug("Restoring governor %s for cpu %d\n",
 				policy->governor->name, policy->cpu);
-	else
-		gov = CPUFREQ_DEFAULT_GOVERNOR;
+	} else {
+		gov = cpufreq_default_governor();
+		if (!gov)
+			return -ENODATA;
+	}
 
 	new_policy.governor = gov;
 
@@ -1964,21 +1972,16 @@ int cpufreq_driver_target(struct cpufreq
 }
 EXPORT_SYMBOL_GPL(cpufreq_driver_target);
 
+__weak struct cpufreq_governor *cpufreq_fallback_governor(void)
+{
+	return NULL;
+}
+
 static int __cpufreq_governor(struct cpufreq_policy *policy,
 					unsigned int event)
 {
 	int ret;
 
-	/* Only must be defined when default governor is known to have latency
-	   restrictions, like e.g. conservative or ondemand.
-	   That this is the case is already ensured in Kconfig
-	*/
-#ifdef CONFIG_CPU_FREQ_GOV_PERFORMANCE
-	struct cpufreq_governor *gov = &cpufreq_gov_performance;
-#else
-	struct cpufreq_governor *gov = NULL;
-#endif
-
 	/* Don't start any governor operations if we are entering suspend */
 	if (cpufreq_suspended)
 		return 0;
@@ -1992,12 +1995,14 @@ static int __cpufreq_governor(struct cpu
 	if (policy->governor->max_transition_latency &&
 	    policy->cpuinfo.transition_latency >
 	    policy->governor->max_transition_latency) {
-		if (!gov)
-			return -EINVAL;
-		else {
+		struct cpufreq_governor *gov = cpufreq_fallback_governor();
+
+		if (gov) {
 			pr_warn("%s governor failed, too long transition latency of HW, fallback to %s governor\n",
 				policy->governor->name, gov->name);
 			policy->governor = gov;
+		} else {
+			return -EINVAL;
 		}
 	}
 
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -26,10 +26,7 @@ static DEFINE_PER_CPU(struct cs_cpu_dbs_
 static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
 				   unsigned int event);
 
-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
-static
-#endif
-struct cpufreq_governor cpufreq_gov_conservative = {
+static struct cpufreq_governor cpufreq_gov_conservative = {
 	.name			= "conservative",
 	.governor		= cs_cpufreq_governor_dbs,
 	.max_transition_latency	= TRANSITION_LATENCY_LIMIT,
@@ -397,6 +394,11 @@ MODULE_DESCRIPTION("'cpufreq_conservativ
 MODULE_LICENSE("GPL");
 
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &cpufreq_gov_conservative;
+}
+
 fs_initcall(cpufreq_gov_dbs_init);
 #else
 module_init(cpufreq_gov_dbs_init);
Index: linux-pm/include/linux/cpufreq.h
===================================================================
--- linux-pm.orig/include/linux/cpufreq.h
+++ linux-pm/include/linux/cpufreq.h
@@ -471,29 +471,8 @@ int __cpufreq_driver_target(struct cpufr
 int cpufreq_register_governor(struct cpufreq_governor *governor);
 void cpufreq_unregister_governor(struct cpufreq_governor *governor);
 
-/* CPUFREQ DEFAULT GOVERNOR */
-/*
- * Performance governor is fallback governor if any other gov failed to auto
- * load due latency restrictions
- */
-#ifdef CONFIG_CPU_FREQ_GOV_PERFORMANCE
-extern struct cpufreq_governor cpufreq_gov_performance;
-#endif
-#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE
-#define CPUFREQ_DEFAULT_GOVERNOR	(&cpufreq_gov_performance)
-#elif defined(CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE)
-extern struct cpufreq_governor cpufreq_gov_powersave;
-#define CPUFREQ_DEFAULT_GOVERNOR	(&cpufreq_gov_powersave)
-#elif defined(CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE)
-extern struct cpufreq_governor cpufreq_gov_userspace;
-#define CPUFREQ_DEFAULT_GOVERNOR	(&cpufreq_gov_userspace)
-#elif defined(CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND)
-extern struct cpufreq_governor cpufreq_gov_ondemand;
-#define CPUFREQ_DEFAULT_GOVERNOR	(&cpufreq_gov_ondemand)
-#elif defined(CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE)
-extern struct cpufreq_governor cpufreq_gov_conservative;
-#define CPUFREQ_DEFAULT_GOVERNOR	(&cpufreq_gov_conservative)
-#endif
+struct cpufreq_governor *cpufreq_default_governor(void);
+struct cpufreq_governor *cpufreq_fallback_governor(void);
 
 /*********************************************************************
  *                     FREQUENCY TABLE HELPERS                       *
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -31,9 +31,7 @@ static DEFINE_PER_CPU(struct od_cpu_dbs_
 
 static struct od_ops od_ops;
 
-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
 static struct cpufreq_governor cpufreq_gov_ondemand;
-#endif
 
 static unsigned int default_powersave_bias;
 
@@ -553,6 +551,19 @@ static struct common_dbs_data od_dbs_cda
 	.mutex = __MUTEX_INITIALIZER(od_dbs_cdata.mutex),
 };
 
+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;
@@ -604,22 +615,6 @@ void od_unregister_powersave_bias_handle
 }
 EXPORT_SYMBOL_GPL(od_unregister_powersave_bias_handler);
 
-static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
-		unsigned int event)
-{
-	return cpufreq_governor_dbs(policy, &od_dbs_cdata, event);
-}
-
-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
-static
-#endif
-struct cpufreq_governor cpufreq_gov_ondemand = {
-	.name			= "ondemand",
-	.governor		= od_cpufreq_governor_dbs,
-	.max_transition_latency	= TRANSITION_LATENCY_LIMIT,
-	.owner			= THIS_MODULE,
-};
-
 static int __init cpufreq_gov_dbs_init(void)
 {
 	return cpufreq_register_governor(&cpufreq_gov_ondemand);
@@ -637,6 +632,11 @@ MODULE_DESCRIPTION("'cpufreq_ondemand' -
 MODULE_LICENSE("GPL");
 
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &cpufreq_gov_ondemand;
+}
+
 fs_initcall(cpufreq_gov_dbs_init);
 #else
 module_init(cpufreq_gov_dbs_init);
Index: linux-pm/drivers/cpufreq/cpufreq_performance.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_performance.c
+++ linux-pm/drivers/cpufreq/cpufreq_performance.c
@@ -33,10 +33,7 @@ static int cpufreq_governor_performance(
 	return 0;
 }
 
-#ifdef CONFIG_CPU_FREQ_GOV_PERFORMANCE_MODULE
-static
-#endif
-struct cpufreq_governor cpufreq_gov_performance = {
+static struct cpufreq_governor cpufreq_gov_performance = {
 	.name		= "performance",
 	.governor	= cpufreq_governor_performance,
 	.owner		= THIS_MODULE,
@@ -52,6 +49,19 @@ static void __exit cpufreq_gov_performan
 	cpufreq_unregister_governor(&cpufreq_gov_performance);
 }
 
+#ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &cpufreq_gov_performance;
+}
+#endif
+#ifndef CONFIG_CPU_FREQ_GOV_PERFORMANCE_MODULE
+struct cpufreq_governor *cpufreq_fallback_governor(void)
+{
+	return &cpufreq_gov_performance;
+}
+#endif
+
 MODULE_AUTHOR("Dominik Brodowski <linux@brodo.de>");
 MODULE_DESCRIPTION("CPUfreq policy governor 'performance'");
 MODULE_LICENSE("GPL");
Index: linux-pm/drivers/cpufreq/cpufreq_powersave.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_powersave.c
+++ linux-pm/drivers/cpufreq/cpufreq_powersave.c
@@ -33,10 +33,7 @@ static int cpufreq_governor_powersave(st
 	return 0;
 }
 
-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE
-static
-#endif
-struct cpufreq_governor cpufreq_gov_powersave = {
+static struct cpufreq_governor cpufreq_gov_powersave = {
 	.name		= "powersave",
 	.governor	= cpufreq_governor_powersave,
 	.owner		= THIS_MODULE,
@@ -57,6 +54,11 @@ MODULE_DESCRIPTION("CPUfreq policy gover
 MODULE_LICENSE("GPL");
 
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &cpufreq_gov_powersave;
+}
+
 fs_initcall(cpufreq_gov_powersave_init);
 #else
 module_init(cpufreq_gov_powersave_init);
Index: linux-pm/drivers/cpufreq/cpufreq_userspace.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_userspace.c
+++ linux-pm/drivers/cpufreq/cpufreq_userspace.c
@@ -89,10 +89,7 @@ static int cpufreq_governor_userspace(st
 	return rc;
 }
 
-#ifndef CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE
-static
-#endif
-struct cpufreq_governor cpufreq_gov_userspace = {
+static struct cpufreq_governor cpufreq_gov_userspace = {
 	.name		= "userspace",
 	.governor	= cpufreq_governor_userspace,
 	.store_setspeed	= cpufreq_set,
@@ -116,6 +113,11 @@ MODULE_DESCRIPTION("CPUfreq policy gover
 MODULE_LICENSE("GPL");
 
 #ifdef CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE
+struct cpufreq_governor *cpufreq_default_governor(void)
+{
+	return &cpufreq_gov_userspace;
+}
+
 fs_initcall(cpufreq_gov_userspace_init);
 #else
 module_init(cpufreq_gov_userspace_init);

[toc] | [prev] | [next] | [standalone]

#1327408 — [PATCH v2 0/10] cpufreq: governor: ondemand/conservative data structures rework

On Thursday, February 04, 2016 12:12:52 AM 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.
> [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.

Some bugs fixed, some comments addressed.  It's time for a v2. :-)

The most significant difference from the previous one is that I've dropped patch
[3/11] (so there are 10 of them in the series now) due to a problem with it pointed
out by Viresh.  Fortunately, this particular one was completely not essential and
the dependencies on it were rather cosmetic.

In addition to that I've rebased the series on top of
https://patchwork.kernel.org/patch/8229901/ as that differs from its previous
version quite a bit.

I've already queued up [1/10] for 4.6 as it seems totally uncontroversial.

The whole series (along with some patches it depends on) is available from the
git branch at

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-cpufreq-rjw

in case someone wants to try it.  I've tested it (very lightly) on an x86 laptop
with the ACPI cpufreq driver and the ondemand governor.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1327409 — [PATCH v2 3/10] cpufreq: governor: Avoid passing dbs_data pointers around unnecessarily

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Do not pass struct dbs_data pointers to the family of functions
implementing governor operations in cpufreq_governor.c as they can
take that pointer from policy->governor by themselves.

The cpufreq_governor_init() case is slightly more complicated, since
policy->governor may be NULL when it is invoked, but then it can reach
the pointer in question via its cdata argument just fine.

While at it, rework cpufreq_governor_dbs() to avoid a pointless
policy_governor check in the CPUFREQ_GOV_POLICY_INIT case.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq_governor.c |   68 ++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 41 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -325,9 +325,9 @@ static void free_common_dbs_info(struct
 }
 
 static int cpufreq_governor_init(struct cpufreq_policy *policy,
-				 struct dbs_data *dbs_data,
 				 struct common_dbs_data *cdata)
 {
+	struct dbs_data *dbs_data = cdata->gdbs_data;
 	unsigned int latency;
 	int ret;
 
@@ -399,9 +399,9 @@ free_dbs_data:
 	return ret;
 }
 
-static int cpufreq_governor_exit(struct cpufreq_policy *policy,
-				 struct dbs_data *dbs_data)
+static int cpufreq_governor_exit(struct cpufreq_policy *policy)
 {
+	struct dbs_data *dbs_data = policy->governor_data;
 	struct common_dbs_data *cdata = dbs_data->cdata;
 	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(policy->cpu);
 
@@ -428,9 +428,9 @@ static int cpufreq_governor_exit(struct
 	return 0;
 }
 
-static int cpufreq_governor_start(struct cpufreq_policy *policy,
-				  struct dbs_data *dbs_data)
+static int cpufreq_governor_start(struct cpufreq_policy *policy)
 {
+	struct dbs_data *dbs_data = policy->governor_data;
 	struct common_dbs_data *cdata = dbs_data->cdata;
 	unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
 	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu);
@@ -497,9 +497,9 @@ static int cpufreq_governor_start(struct
 	return 0;
 }
 
-static int cpufreq_governor_stop(struct cpufreq_policy *policy,
-				 struct dbs_data *dbs_data)
+static int cpufreq_governor_stop(struct cpufreq_policy *policy)
 {
+	struct dbs_data *dbs_data = policy->governor_data;
 	struct cpu_dbs_info *cdbs = dbs_data->cdata->get_cpu_cdbs(policy->cpu);
 	struct cpu_common_dbs_info *shared = cdbs->shared;
 
@@ -513,9 +513,9 @@ static int cpufreq_governor_stop(struct
 	return 0;
 }
 
-static int cpufreq_governor_limits(struct cpufreq_policy *policy,
-				   struct dbs_data *dbs_data)
+static int cpufreq_governor_limits(struct cpufreq_policy *policy)
 {
+	struct dbs_data *dbs_data = policy->governor_data;
 	struct common_dbs_data *cdata = dbs_data->cdata;
 	unsigned int cpu = policy->cpu;
 	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu);
@@ -540,45 +540,31 @@ static int cpufreq_governor_limits(struc
 int cpufreq_governor_dbs(struct cpufreq_policy *policy,
 			 struct common_dbs_data *cdata, unsigned int event)
 {
-	struct dbs_data *dbs_data;
-	int ret;
+	int ret = -EINVAL;
 
 	/* Lock governor to block concurrent initialization of governor */
 	mutex_lock(&dbs_data_mutex);
 
-	if (have_governor_per_policy())
-		dbs_data = policy->governor_data;
-	else
-		dbs_data = cdata->gdbs_data;
-
-	if (!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT)) {
-		ret = -EINVAL;
-		goto unlock;
-	}
-
-	switch (event) {
-	case CPUFREQ_GOV_POLICY_INIT:
-		ret = cpufreq_governor_init(policy, dbs_data, cdata);
-		break;
-	case CPUFREQ_GOV_POLICY_EXIT:
-		ret = cpufreq_governor_exit(policy, dbs_data);
-		break;
-	case CPUFREQ_GOV_START:
-		ret = cpufreq_governor_start(policy, dbs_data);
-		break;
-	case CPUFREQ_GOV_STOP:
-		ret = cpufreq_governor_stop(policy, dbs_data);
-		break;
-	case CPUFREQ_GOV_LIMITS:
-		ret = cpufreq_governor_limits(policy, dbs_data);
-		break;
-	default:
-		ret = -EINVAL;
+	if (event == CPUFREQ_GOV_POLICY_INIT) {
+		ret = cpufreq_governor_init(policy, cdata);
+	} else if (policy->governor_data) {
+		switch (event) {
+		case CPUFREQ_GOV_POLICY_EXIT:
+			ret = cpufreq_governor_exit(policy);
+			break;
+		case CPUFREQ_GOV_START:
+			ret = cpufreq_governor_start(policy);
+			break;
+		case CPUFREQ_GOV_STOP:
+			ret = cpufreq_governor_stop(policy);
+			break;
+		case CPUFREQ_GOV_LIMITS:
+			ret = cpufreq_governor_limits(policy);
+			break;
+		}
 	}
 
-unlock:
 	mutex_unlock(&dbs_data_mutex);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(cpufreq_governor_dbs);

[toc] | [prev] | [next] | [standalone]

#1327509 — Re: [PATCH v2 3/10] cpufreq: governor: Avoid passing dbs_data pointers around unnecessarily

On 05-02-16, 03:15, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Do not pass struct dbs_data pointers to the family of functions
> implementing governor operations in cpufreq_governor.c as they can
> take that pointer from policy->governor by themselves.
> 
> The cpufreq_governor_init() case is slightly more complicated, since
> policy->governor may be NULL when it is invoked, but then it can reach
> the pointer in question via its cdata argument just fine.
> 
> While at it, rework cpufreq_governor_dbs() to avoid a pointless
> policy_governor check in the CPUFREQ_GOV_POLICY_INIT case.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq_governor.c |   68 ++++++++++++++-----------------------
>  1 file changed, 27 insertions(+), 41 deletions(-)

Took some time to review, but it looks nice cleanup :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1327410 — [PATCH v2 5/10] cpufreq: governor: Rename some data types and variables

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The ondemand and conservative governors are represented by
struct common_dbs_data whose name doesn't reflect the purpose it
is used for, so rename it to struct dbs_governor and rename
variables of that type accordingly.

No functional changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/amd_freq_sensitivity.c |    2 
 drivers/cpufreq/cpufreq_conservative.c |    8 +--
 drivers/cpufreq/cpufreq_governor.c     |   88 ++++++++++++++++-----------------
 drivers/cpufreq/cpufreq_governor.h     |   12 ++--
 drivers/cpufreq/cpufreq_ondemand.c     |    8 +--
 5 files changed, 59 insertions(+), 59 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -119,7 +119,7 @@ static struct notifier_block cs_cpufreq_
 };
 
 /************************** sysfs interface ************************/
-static struct common_dbs_data cs_dbs_cdata;
+static struct dbs_governor cs_dbs_gov;
 
 static ssize_t store_sampling_down_factor(struct dbs_data *dbs_data,
 		const char *buf, size_t count)
@@ -328,7 +328,7 @@ define_get_cpu_dbs_routines(cs_cpu_dbs_i
 static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
 				   unsigned int event);
 
-static struct common_dbs_data cs_dbs_cdata = {
+static struct dbs_governor cs_dbs_gov = {
 	.gov = {
 		.name = "conservative",
 		.governor = cs_cpufreq_governor_dbs,
@@ -346,12 +346,12 @@ static struct common_dbs_data cs_dbs_cda
 	.exit = cs_exit,
 };
 
-#define CPU_FREQ_GOV_CONSERVATIVE	(&cs_dbs_cdata.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_cdata, event);
+	return cpufreq_governor_dbs(policy, &cs_dbs_gov, event);
 }
 
 static int dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -28,14 +28,14 @@ EXPORT_SYMBOL_GPL(dbs_data_mutex);
 static struct attribute_group *get_sysfs_attr(struct dbs_data *dbs_data)
 {
 	if (have_governor_per_policy())
-		return dbs_data->cdata->attr_group_gov_pol;
+		return dbs_data->gov->attr_group_gov_pol;
 	else
-		return dbs_data->cdata->attr_group_gov_sys;
+		return dbs_data->gov->attr_group_gov_sys;
 }
 
 void dbs_check_cpu(struct dbs_data *dbs_data, int cpu)
 {
-	struct cpu_dbs_info *cdbs = dbs_data->cdata->get_cpu_cdbs(cpu);
+	struct cpu_dbs_info *cdbs = dbs_data->gov->get_cpu_cdbs(cpu);
 	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;
@@ -44,9 +44,9 @@ void dbs_check_cpu(struct dbs_data *dbs_
 	unsigned int ignore_nice;
 	unsigned int j;
 
-	if (dbs_data->cdata->governor == GOV_ONDEMAND) {
+	if (dbs_data->gov->governor == GOV_ONDEMAND) {
 		struct od_cpu_dbs_info_s *od_dbs_info =
-				dbs_data->cdata->get_cpu_dbs_info_s(cpu);
+				dbs_data->gov->get_cpu_dbs_info_s(cpu);
 
 		/*
 		 * Sometimes, the ondemand governor uses an additional
@@ -71,7 +71,7 @@ void dbs_check_cpu(struct dbs_data *dbs_
 		unsigned int load;
 		int io_busy = 0;
 
-		j_cdbs = dbs_data->cdata->get_cpu_cdbs(j);
+		j_cdbs = dbs_data->gov->get_cpu_cdbs(j);
 
 		/*
 		 * For the purpose of ondemand, waiting for disk IO is
@@ -79,7 +79,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->cdata->governor == GOV_ONDEMAND)
+		if (dbs_data->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 +160,7 @@ void dbs_check_cpu(struct dbs_data *dbs_
 			max_load = load;
 	}
 
-	dbs_data->cdata->gov_check_cpu(cpu, max_load);
+	dbs_data->gov->gov_check_cpu(cpu, max_load);
 }
 EXPORT_SYMBOL_GPL(dbs_check_cpu);
 
@@ -176,7 +176,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->cdata->get_cpu_cdbs(cpu);
+		struct cpu_dbs_info *cdbs = dbs_data->gov->get_cpu_cdbs(cpu);
 
 		cpufreq_set_update_util_data(cpu, &cdbs->update_util);
 	}
@@ -226,7 +226,7 @@ static void dbs_work_handler(struct work
 	 * parallel.
 	 */
 	mutex_lock(&shared->timer_mutex);
-	delay = dbs_data->cdata->gov_dbs_timer(policy);
+	delay = dbs_data->gov->gov_dbs_timer(policy);
 	shared->sample_delay_ns = jiffies_to_nsecs(delay);
 	shared->time_stamp = ktime_get();
 	mutex_unlock(&shared->timer_mutex);
@@ -279,7 +279,7 @@ static void dbs_update_util_handler(stru
 static void set_sampling_rate(struct dbs_data *dbs_data,
 		unsigned int sampling_rate)
 {
-	if (dbs_data->cdata->governor == GOV_CONSERVATIVE) {
+	if (dbs_data->gov->governor == GOV_CONSERVATIVE) {
 		struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 		cs_tuners->sampling_rate = sampling_rate;
 	} else {
@@ -289,7 +289,7 @@ static void set_sampling_rate(struct dbs
 }
 
 static int alloc_common_dbs_info(struct cpufreq_policy *policy,
-				 struct common_dbs_data *cdata)
+				 struct dbs_governor *gov)
 {
 	struct cpu_common_dbs_info *shared;
 	int j;
@@ -301,7 +301,7 @@ static int alloc_common_dbs_info(struct
 
 	/* Set shared for all CPUs, online+offline */
 	for_each_cpu(j, policy->related_cpus)
-		cdata->get_cpu_cdbs(j)->shared = shared;
+		gov->get_cpu_cdbs(j)->shared = shared;
 
 	mutex_init(&shared->timer_mutex);
 	atomic_set(&shared->skip_work, 0);
@@ -310,24 +310,24 @@ static int alloc_common_dbs_info(struct
 }
 
 static void free_common_dbs_info(struct cpufreq_policy *policy,
-				 struct common_dbs_data *cdata)
+				 struct dbs_governor *gov)
 {
-	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(policy->cpu);
+	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
 	struct cpu_common_dbs_info *shared = cdbs->shared;
 	int j;
 
 	mutex_destroy(&shared->timer_mutex);
 
 	for_each_cpu(j, policy->cpus)
-		cdata->get_cpu_cdbs(j)->shared = NULL;
+		gov->get_cpu_cdbs(j)->shared = NULL;
 
 	kfree(shared);
 }
 
 static int cpufreq_governor_init(struct cpufreq_policy *policy,
-				 struct common_dbs_data *cdata)
+				 struct dbs_governor *gov)
 {
-	struct dbs_data *dbs_data = cdata->gdbs_data;
+	struct dbs_data *dbs_data = gov->gdbs_data;
 	unsigned int latency;
 	int ret;
 
@@ -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, cdata);
+		ret = alloc_common_dbs_info(policy, gov);
 		if (ret)
 			return ret;
 
@@ -352,14 +352,14 @@ static int cpufreq_governor_init(struct
 	if (!dbs_data)
 		return -ENOMEM;
 
-	ret = alloc_common_dbs_info(policy, cdata);
+	ret = alloc_common_dbs_info(policy, gov);
 	if (ret)
 		goto free_dbs_data;
 
-	dbs_data->cdata = cdata;
+	dbs_data->gov = gov;
 	dbs_data->usage_count = 1;
 
-	ret = cdata->init(dbs_data, !policy->governor->initialized);
+	ret = gov->init(dbs_data, !policy->governor->initialized);
 	if (ret)
 		goto free_common_dbs_info;
 
@@ -375,7 +375,7 @@ static int cpufreq_governor_init(struct
 					latency * LATENCY_MULTIPLIER));
 
 	if (!have_governor_per_policy())
-		cdata->gdbs_data = dbs_data;
+		gov->gdbs_data = dbs_data;
 
 	policy->governor_data = dbs_data;
 
@@ -390,10 +390,10 @@ reset_gdbs_data:
 	policy->governor_data = NULL;
 
 	if (!have_governor_per_policy())
-		cdata->gdbs_data = NULL;
-	cdata->exit(dbs_data, !policy->governor->initialized);
+		gov->gdbs_data = NULL;
+	gov->exit(dbs_data, !policy->governor->initialized);
 free_common_dbs_info:
-	free_common_dbs_info(policy, cdata);
+	free_common_dbs_info(policy, gov);
 free_dbs_data:
 	kfree(dbs_data);
 	return ret;
@@ -402,8 +402,8 @@ free_dbs_data:
 static int cpufreq_governor_exit(struct cpufreq_policy *policy)
 {
 	struct dbs_data *dbs_data = policy->governor_data;
-	struct common_dbs_data *cdata = dbs_data->cdata;
-	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(policy->cpu);
+	struct dbs_governor *gov = dbs_data->gov;
+	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
 
 	/* State should be equivalent to INIT */
 	if (!cdbs->shared || cdbs->shared->policy)
@@ -416,24 +416,24 @@ static int cpufreq_governor_exit(struct
 		policy->governor_data = NULL;
 
 		if (!have_governor_per_policy())
-			cdata->gdbs_data = NULL;
+			gov->gdbs_data = NULL;
 
-		cdata->exit(dbs_data, policy->governor->initialized == 1);
+		gov->exit(dbs_data, policy->governor->initialized == 1);
 		kfree(dbs_data);
 	} else {
 		policy->governor_data = NULL;
 	}
 
-	free_common_dbs_info(policy, cdata);
+	free_common_dbs_info(policy, gov);
 	return 0;
 }
 
 static int cpufreq_governor_start(struct cpufreq_policy *policy)
 {
 	struct dbs_data *dbs_data = policy->governor_data;
-	struct common_dbs_data *cdata = dbs_data->cdata;
+	struct dbs_governor *gov = dbs_data->gov;
 	unsigned int sampling_rate, ignore_nice, j, cpu = policy->cpu;
-	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu);
+	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
 	struct cpu_common_dbs_info *shared = cdbs->shared;
 	int io_busy = 0;
 
@@ -444,7 +444,7 @@ static int cpufreq_governor_start(struct
 	if (!shared || shared->policy)
 		return -EBUSY;
 
-	if (cdata->governor == GOV_CONSERVATIVE) {
+	if (gov->governor == GOV_CONSERVATIVE) {
 		struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 
 		sampling_rate = cs_tuners->sampling_rate;
@@ -458,7 +458,7 @@ static int cpufreq_governor_start(struct
 	}
 
 	for_each_cpu(j, policy->cpus) {
-		struct cpu_dbs_info *j_cdbs = cdata->get_cpu_cdbs(j);
+		struct cpu_dbs_info *j_cdbs = gov->get_cpu_cdbs(j);
 		unsigned int prev_load;
 
 		j_cdbs->prev_cpu_idle =
@@ -478,15 +478,15 @@ static int cpufreq_governor_start(struct
 	init_irq_work(&shared->irq_work, dbs_irq_work);
 	init_completion(&shared->irq_work_done);
 
-	if (cdata->governor == GOV_CONSERVATIVE) {
+	if (gov->governor == GOV_CONSERVATIVE) {
 		struct cs_cpu_dbs_info_s *cs_dbs_info =
-			cdata->get_cpu_dbs_info_s(cpu);
+			gov->get_cpu_dbs_info_s(cpu);
 
 		cs_dbs_info->down_skip = 0;
 		cs_dbs_info->requested_freq = policy->cur;
 	} else {
-		struct od_ops *od_ops = cdata->gov_ops;
-		struct od_cpu_dbs_info_s *od_dbs_info = cdata->get_cpu_dbs_info_s(cpu);
+		struct od_ops *od_ops = gov->gov_ops;
+		struct od_cpu_dbs_info_s *od_dbs_info = gov->get_cpu_dbs_info_s(cpu);
 
 		od_dbs_info->rate_mult = 1;
 		od_dbs_info->sample_type = OD_NORMAL_SAMPLE;
@@ -500,7 +500,7 @@ 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->cdata->get_cpu_cdbs(policy->cpu);
+	struct cpu_dbs_info *cdbs = dbs_data->gov->get_cpu_cdbs(policy->cpu);
 	struct cpu_common_dbs_info *shared = cdbs->shared;
 
 	/* State should be equivalent to START */
@@ -516,9 +516,9 @@ static int cpufreq_governor_stop(struct
 static int cpufreq_governor_limits(struct cpufreq_policy *policy)
 {
 	struct dbs_data *dbs_data = policy->governor_data;
-	struct common_dbs_data *cdata = dbs_data->cdata;
+	struct dbs_governor *gov = dbs_data->gov;
 	unsigned int cpu = policy->cpu;
-	struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu);
+	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
 
 	/* State should be equivalent to START */
 	if (!cdbs->shared || !cdbs->shared->policy)
@@ -538,7 +538,7 @@ static int cpufreq_governor_limits(struc
 }
 
 int cpufreq_governor_dbs(struct cpufreq_policy *policy,
-			 struct common_dbs_data *cdata, unsigned int event)
+			 struct dbs_governor *gov, 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, cdata);
+		ret = cpufreq_governor_init(policy, gov);
 	} 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
@@ -79,7 +79,7 @@ __ATTR(_name, 0644, show_##_name##_gov_p
 static ssize_t show_##file_name##_gov_sys				\
 (struct kobject *kobj, struct attribute *attr, char *buf)		\
 {									\
-	struct _gov##_dbs_tuners *tuners = _gov##_dbs_cdata.gdbs_data->tuners; \
+	struct _gov##_dbs_tuners *tuners = _gov##_dbs_gov.gdbs_data->tuners; \
 	return sprintf(buf, "%u\n", tuners->file_name);			\
 }									\
 									\
@@ -95,7 +95,7 @@ static ssize_t show_##file_name##_gov_po
 static ssize_t store_##file_name##_gov_sys				\
 (struct kobject *kobj, struct attribute *attr, const char *buf, size_t count) \
 {									\
-	struct dbs_data *dbs_data = _gov##_dbs_cdata.gdbs_data;		\
+	struct dbs_data *dbs_data = _gov##_dbs_gov.gdbs_data;		\
 	return store_##file_name(dbs_data, buf, count);			\
 }									\
 									\
@@ -202,7 +202,7 @@ struct cs_dbs_tuners {
 
 /* Common Governor data across policies */
 struct dbs_data;
-struct common_dbs_data {
+struct dbs_governor {
 	struct cpufreq_governor gov;
 
 	#define GOV_ONDEMAND		0
@@ -230,7 +230,7 @@ struct common_dbs_data {
 
 /* Governor Per policy data */
 struct dbs_data {
-	struct common_dbs_data *cdata;
+	struct dbs_governor *gov;
 	unsigned int min_sampling_rate;
 	int usage_count;
 	void *tuners;
@@ -259,7 +259,7 @@ static inline int delay_for_sampling_rat
 static ssize_t show_sampling_rate_min_gov_sys				\
 (struct kobject *kobj, struct attribute *attr, char *buf)		\
 {									\
-	struct dbs_data *dbs_data = _gov##_dbs_cdata.gdbs_data;		\
+	struct dbs_data *dbs_data = _gov##_dbs_gov.gdbs_data;		\
 	return sprintf(buf, "%u\n", dbs_data->min_sampling_rate);	\
 }									\
 									\
@@ -277,7 +277,7 @@ void gov_set_update_util(struct cpu_comm
 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 common_dbs_data *cdata, unsigned int event);
+		struct dbs_governor *gov, 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
@@ -219,7 +219,7 @@ static unsigned int od_dbs_timer(struct
 }
 
 /************************** sysfs interface ************************/
-static struct common_dbs_data od_dbs_cdata;
+static struct dbs_governor od_dbs_gov;
 
 /**
  * update_sampling_rate - update sampling rate effective immediately if needed.
@@ -538,7 +538,7 @@ static struct od_ops od_ops = {
 static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
 				   unsigned int event);
 
-static struct common_dbs_data od_dbs_cdata = {
+static struct dbs_governor od_dbs_gov = {
 	.gov = {
 		.name = "ondemand",
 		.governor = od_cpufreq_governor_dbs,
@@ -557,12 +557,12 @@ static struct common_dbs_data od_dbs_cda
 	.exit = od_exit,
 };
 
-#define CPU_FREQ_GOV_ONDEMAND	(&od_dbs_cdata.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_cdata, event);
+	return cpufreq_governor_dbs(policy, &od_dbs_gov, event);
 }
 
 static void od_set_powersave_bias(unsigned int powersave_bias)
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->cdata->get_cpu_dbs_info_s(policy->cpu);
+		od_data->gov->get_cpu_dbs_info_s(policy->cpu);
 
 	if (!od_info->freq_table)
 		return freq_next;

[toc] | [prev] | [next] | [standalone]

#1327512 — Re: [PATCH v2 5/10] cpufreq: governor: Rename some data types and variables

On 05-02-16, 03:17, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The ondemand and conservative governors are represented by
> struct common_dbs_data whose name doesn't reflect the purpose it
> is used for, so rename it to struct dbs_governor and rename
> variables of that type 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]

#1327411 — [PATCH v2 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu()

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
@@ -282,7 +282,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
@@ -194,9 +194,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;
 
@@ -207,7 +205,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
@@ -31,13 +31,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;
@@ -515,7 +516,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]

#1327552 — Re: [PATCH v2 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu()

On 05-02-16, 03:21, Rafael J. Wysocki wrote:
> 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(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1328297 — [PATCH v3 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu()

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>
---

Changes from v2:
- Rebase on top of https://patchwork.kernel.org/patch/8241231/
  and on the v3 of [8/10] without [9/10].

---
 drivers/cpufreq/cpufreq_conservative.c |    2 +-
 drivers/cpufreq/cpufreq_governor.c     |    8 ++++----
 drivers/cpufreq/cpufreq_governor.h     |    2 +-
 drivers/cpufreq/cpufreq_ondemand.c     |    6 ++----
 4 files changed, 8 insertions(+), 10 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 @@ static ssize_t show_sampling_rate_min_go
 
 extern struct mutex dbs_data_mutex;
 extern struct mutex cpufreq_governor_lock;
-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
@@ -190,9 +190,7 @@ static void od_check_cpu(int cpu, unsign
 static unsigned int od_dbs_timer(struct cpufreq_policy *policy)
 {
 	struct dbs_data *dbs_data = policy->governor_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;
 
@@ -203,7 +201,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
@@ -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(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
@@ -31,8 +31,9 @@ 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)
 {
+	int cpu = policy->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;
@@ -516,8 +517,7 @@ static int cpufreq_governor_stop(struct
 static int cpufreq_governor_limits(struct cpufreq_policy *policy)
 {
 	struct dbs_governor *gov = dbs_governor_of(policy);
-	unsigned int cpu = policy->cpu;
-	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(cpu);
+	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
 
 	/* State should be equivalent to START */
 	if (!cdbs->policy_dbs || !cdbs->policy_dbs->policy)
@@ -530,7 +530,7 @@ static int cpufreq_governor_limits(struc
 	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);
+	dbs_check_cpu(policy);
 	mutex_unlock(&cdbs->policy_dbs->timer_mutex);
 
 	return 0;

[toc] | [prev] | [next] | [standalone]

#1328456 — Re: [PATCH v3 10/10] cpufreq: governor: Drop cpu argument from dbs_check_cpu()

On 06-02-16, 13:50, Rafael J. Wysocki wrote:
> 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>
> ---
> 
> Changes from v2:
> - Rebase on top of https://patchwork.kernel.org/patch/8241231/
>   and on the v3 of [8/10] without [9/10].

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1327412 — [PATCH v2 4/10] cpufreq: governor: Put governor structure into common_dbs_data

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>
Acked-by: Saravana Kannan <skannan@codeaurora.org>
---
 drivers/cpufreq/cpufreq_conservative.c |   78 +++++++++++++++++----------------
 drivers/cpufreq/cpufreq_governor.h     |    3 -
 drivers/cpufreq/cpufreq_ondemand.c     |   28 ++++++-----
 3 files changed, 58 insertions(+), 51 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -31,8 +31,6 @@ static DEFINE_PER_CPU(struct od_cpu_dbs_
 
 static struct od_ops od_ops;
 
-static struct cpufreq_governor cpufreq_gov_ondemand;
-
 static unsigned int default_powersave_bias;
 
 static void ondemand_powersave_bias_init_cpu(int cpu)
@@ -537,7 +535,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,
@@ -550,19 +557,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;
@@ -588,7 +590,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;
@@ -616,12 +618,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>");
@@ -633,7 +635,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);
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -203,7 +203,8 @@ struct cs_dbs_tuners {
 /* Common Governor data across policies */
 struct dbs_data;
 struct common_dbs_data {
-	/* Common across governors */
+	struct cpufreq_governor gov;
+
 	#define GOV_ONDEMAND		0
 	#define GOV_CONSERVATIVE	1
 	int governor;

[toc] | [prev] | [next] | [standalone]

#1327513 — Re: [PATCH v2 4/10] cpufreq: governor: Put governor structure into common_dbs_data

On 05-02-16, 03:16, 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>
> Acked-by: Saravana Kannan <skannan@codeaurora.org>

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1328293 — Re: [PATCH v2 0/10] cpufreq: governor: ondemand/conservative data structures rework

On Friday, February 05, 2016 03:07:27 AM Rafael J. Wysocki wrote:
> On Thursday, February 04, 2016 12:12:52 AM 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.
> > [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.
> 
> Some bugs fixed, some comments addressed.  It's time for a v2. :-)
> 
> The most significant difference from the previous one is that I've dropped patch
> [3/11] (so there are 10 of them in the series now) due to a problem with it pointed
> out by Viresh.  Fortunately, this particular one was completely not essential and
> the dependencies on it were rather cosmetic.
> 
> In addition to that I've rebased the series on top of
> https://patchwork.kernel.org/patch/8229901/ as that differs from its previous
> version quite a bit.
> 
> I've already queued up [1/10] for 4.6 as it seems totally uncontroversial.
> 
> The whole series (along with some patches it depends on) is available from the
> git branch at
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-cpufreq-rjw
> 
> in case someone wants to try it.  I've tested it (very lightly) on an x86 laptop
> with the ACPI cpufreq driver and the ondemand governor.

I'm going to tentatively queue up patches [2-8,10/10] with Viresh's ACKs (thanks
Viresh!) for 4.6 (tentatively, because they depend on the timers elimination
series which is still under ongoing review, but that seems to be reaching
conclusion).

Patches [8,10/10] needed a non-trivial rebase, so I'll post the new versions
shortly for completeness.

I'm going to rework patch [9/10] on top of that into a few separate patches
that will hopefully be easier to digest.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1328501 — [PATCH 1/3] cpufreq: governor: Simplify cpufreq_governor_limits()

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use the observation that cpufreq_governor_limits() doesn't have to
get to the policy object it wants to manipulate by walking the
reference chain cdbs->policy_dbs->policy, as the final pointer is
actually equal to its argument, and make it access the policy
object directy via its argument.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq_governor.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -518,20 +518,19 @@ static int cpufreq_governor_limits(struc
 {
 	struct dbs_governor *gov = dbs_governor_of(policy);
 	struct cpu_dbs_info *cdbs = gov->get_cpu_cdbs(policy->cpu);
+	struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
 
 	/* State should be equivalent to START */
-	if (!cdbs->policy_dbs || !cdbs->policy_dbs->policy)
+	if (!policy_dbs || !policy_dbs->policy)
 		return -EBUSY;
 
-	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->policy_dbs->policy->cur)
-		__cpufreq_driver_target(cdbs->policy_dbs->policy, policy->min,
-					CPUFREQ_RELATION_L);
+	mutex_lock(&policy_dbs->timer_mutex);
+	if (policy->max < policy->cur)
+		__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);
-	mutex_unlock(&cdbs->policy_dbs->timer_mutex);
+	mutex_unlock(&policy_dbs->timer_mutex);
 
 	return 0;
 }

[toc] | [prev] | [next] | [standalone]

#1328509 — Re: [PATCH 1/3] cpufreq: governor: Simplify cpufreq_governor_limits()

On 07-02-16, 16:23, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Use the observation that cpufreq_governor_limits() doesn't have to
> get to the policy object it wants to manipulate by walking the
> reference chain cdbs->policy_dbs->policy, as the final pointer is
> actually equal to its argument, and make it access the policy
> object directy via its argument.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq_governor.c |   17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)

Why the hell did we write it that way earlier ? :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web