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

Started by"Rafael J. Wysocki" <rjw@rjwysocki.net>
First post2016-02-04 00:50 +0100
Last post2016-02-07 17:00 +0100
Articles 8 on this page of 48 — 4 participants

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


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

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-02-08 02:00 +0100
SubjectRe: [PATCH 1/3] cpufreq: governor: Simplify cpufreq_governor_limits()
Message-ID<qZIlj-5T6-5@gated-at.bofh.it>
In reply to#1328509
On Sunday, February 07, 2016 09:10:24 PM Viresh Kumar wrote:
> 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 ? :)

Honestly, I have no idea.

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

Thanks!

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


#1328503 — [PATCH 0/3] cpufreq: governor: Data structure rearrangement

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-02-07 16:30 +0100
Subject[PATCH 0/3] cpufreq: governor: Data structure rearrangement
Message-ID<qZzrH-7X0-5@gated-at.bofh.it>
In reply to#1327408
Hi,

This was part of my recent cleanup series
(http://marc.info/?l=linux-pm&m=145463901630950&w=4), but Viresh had a good
point about the patch doing too much and possibly going too far, so I've
reworked it into a series of three smaller patches and dropped some more
aggressive changes.

[1/3] simplifies cpufreq_governor_limits() to access the policy via its
      argument directly.
[2/3] rearranges the data structures so policy->governor_data points to the
      struct policy_dbs corresponding to that policy and the latter points
      to a struct dbs_data.
[3/3] makes the initialization and cleanup of struct cpu_dbs_info more
      symmetrical.

Patches are on top of my bleeding-edge branch (and there is a separate
pm-cpufreq-test branch with cpufreq patches only in case someone wants/needs
to test those alone).

Thanks,
Rafael

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


#1328505 — [PATCH 3/3] cpufreq: governor: Symmetrize cpu_dbs_info initialization and cleanup

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-02-07 16:30 +0100
Subject[PATCH 3/3] cpufreq: governor: Symmetrize cpu_dbs_info initialization and cleanup
Message-ID<qZzrI-7X0-21@gated-at.bofh.it>
In reply to#1328503
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make the initialization of struct cpu_dbs_info objects in
alloc_policy_dbs_info() and the code that cleans them up in
free_policy_dbs_info() more symmetrical.  In particular,
set/clear the update_util.func field in those functions along
with the policy_dbs field.

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

Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -303,14 +303,18 @@ static struct policy_dbs_info *alloc_pol
 	if (!policy_dbs)
 		return NULL;
 
-	/* Set policy_dbs for all CPUs, online+offline */
-	for_each_cpu(j, policy->related_cpus)
-		gov->get_cpu_cdbs(j)->policy_dbs = policy_dbs;
-
 	mutex_init(&policy_dbs->timer_mutex);
 	atomic_set(&policy_dbs->skip_work, 0);
 	init_irq_work(&policy_dbs->irq_work, dbs_irq_work);
 	INIT_WORK(&policy_dbs->work, dbs_work_handler);
+
+	/* 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->policy_dbs = policy_dbs;
+		j_cdbs->update_util.func = dbs_update_util_handler;
+	}
 	return policy_dbs;
 }
 
@@ -323,9 +327,12 @@ static void free_policy_dbs_info(struct
 
 	mutex_destroy(&policy_dbs->timer_mutex);
 
-	for_each_cpu(j, policy->cpus)
-		gov->get_cpu_cdbs(j)->policy_dbs = NULL;
+	for_each_cpu(j, policy->related_cpus) {
+		struct cpu_dbs_info *j_cdbs = gov->get_cpu_cdbs(j);
 
+		j_cdbs->policy_dbs = NULL;
+		j_cdbs->update_util.func = NULL;
+	}
 	kfree(policy_dbs);
 }
 
@@ -476,8 +483,6 @@ 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;
 

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


#1328511 — Re: [PATCH 3/3] cpufreq: governor: Symmetrize cpu_dbs_info initialization and cleanup

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-07 17:00 +0100
SubjectRe: [PATCH 3/3] cpufreq: governor: Symmetrize cpu_dbs_info initialization and cleanup
Message-ID<qZzUK-87R-3@gated-at.bofh.it>
In reply to#1328505
On 07-02-16, 16:25, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Make the initialization of struct cpu_dbs_info objects in
> alloc_policy_dbs_info() and the code that cleans them up in
> free_policy_dbs_info() more symmetrical.  In particular,
> set/clear the update_util.func field in those functions along
> with the policy_dbs field.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq_governor.c |   21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)

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

-- 
viresh

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


#1328506 — [PATCH 2/3] cpufreq: governor: Rearrange governor data structures

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-02-07 16:30 +0100
Subject[PATCH 2/3] cpufreq: governor: Rearrange governor data structures
Message-ID<qZzrI-7X0-23@gated-at.bofh.it>
In reply to#1328503
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 policy_dbs 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 (instead of struct dbs_data) 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     |   74 ++++++++++++++++-----------------
 drivers/cpufreq/cpufreq_governor.h     |   27 ++++++------
 drivers/cpufreq/cpufreq_ondemand.c     |   18 ++++----
 5 files changed, 68 insertions(+), 60 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -85,7 +85,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);			\
 }
@@ -101,8 +102,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)					\
@@ -130,6 +131,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;
@@ -144,6 +152,8 @@ struct policy_dbs_info {
 	atomic_t skip_work;
 	struct irq_work irq_work;
 	struct work_struct work;
+	/* dbs_data may be shared between multiple policy objects */
+	struct dbs_data *dbs_data;
 };
 
 static inline void gov_update_sample_delay(struct policy_dbs_info *policy_dbs,
@@ -204,7 +214,6 @@ struct cs_dbs_tuners {
 };
 
 /* Common Governor data across policies */
-struct dbs_data;
 struct dbs_governor {
 	struct cpufreq_governor gov;
 
@@ -236,13 +245,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);
@@ -273,7 +275,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
@@ -35,8 +35,8 @@ void dbs_check_cpu(struct cpufreq_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;
+	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;
@@ -95,6 +95,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;
 
@@ -291,8 +292,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;
@@ -300,7 +301,7 @@ 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;
+		return NULL;
 
 	/* Set policy_dbs for all CPUs, online+offline */
 	for_each_cpu(j, policy->related_cpus)
@@ -310,7 +311,7 @@ static int alloc_policy_dbs_info(struct
 	atomic_set(&policy_dbs->skip_work, 0);
 	init_irq_work(&policy_dbs->irq_work, dbs_irq_work);
 	INIT_WORK(&policy_dbs->work, dbs_work_handler);
-	return 0;
+	return policy_dbs;
 }
 
 static void free_policy_dbs_info(struct cpufreq_policy *policy,
@@ -332,6 +333,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;
 
@@ -339,26 +341,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;
 
@@ -380,7 +382,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));
@@ -395,21 +398,21 @@ 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->policy_dbs || cdbs->policy_dbs->policy)
+	if (policy_dbs->policy)
 		return -EBUSY;
 
 	if (!--dbs_data->usage_count) {
@@ -434,17 +437,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->policy_dbs;
 	int io_busy = 0;
 
 	if (!policy->cur)
 		return -EINVAL;
 
 	/* State should be equivalent to INIT */
-	if (!policy_dbs || policy_dbs->policy)
+	if (policy_dbs->policy)
 		return -EBUSY;
 
 	if (gov->governor == GOV_CONSERVATIVE) {
@@ -500,12 +502,10 @@ 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->policy_dbs;
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
 
 	/* State should be equivalent to START */
-	if (!policy_dbs || !policy_dbs->policy)
+	if (!policy_dbs->policy)
 		return -EBUSY;
 
 	gov_cancel_work(policy_dbs);
@@ -516,12 +516,10 @@ static int cpufreq_governor_stop(struct
 
 static int cpufreq_governor_limits(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->policy_dbs;
+	struct policy_dbs_info *policy_dbs = policy->governor_data;
 
 	/* State should be equivalent to START */
-	if (!policy_dbs || !policy_dbs->policy)
+	if (!policy_dbs->policy)
 		return -EBUSY;
 
 	mutex_lock(&policy_dbs->timer_mutex);
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.policy_dbs->policy;
-	struct dbs_data *dbs_data = policy->governor_data;
+	struct policy_dbs_info *policy_dbs = dbs_info->cdbs.policy_dbs;
+	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;
 	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;
@@ -277,7 +281,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) {
 			mutex_lock(&policy_dbs->timer_mutex);
 			/*
 			 * On 32-bit architectures this may race with the
@@ -586,7 +590,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.policy_dbs->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);

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


#1328508 — Re: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-07 16:50 +0100
SubjectRe: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures
Message-ID<qZzL4-84f-7@gated-at.bofh.it>
In reply to#1328506
On 07-02-16, 16:24, Rafael J. Wysocki wrote:
>  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->policy_dbs || cdbs->policy_dbs->policy)
> +	if (policy_dbs->policy)
>  		return -EBUSY;

We can crash here if policy_dbs is NULL, which can happen if EXIT is called
twice (due to the lock-dropping thing in cpufreq-core). So we really need to
keep these checks for now atleast.

-- 
viresh

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


#1328512 — Re: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-07 17:00 +0100
SubjectRe: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures
Message-ID<qZzUK-87R-5@gated-at.bofh.it>
In reply to#1328508
On 07-02-16, 21:15, Viresh Kumar wrote:
> On 07-02-16, 16:24, Rafael J. Wysocki wrote:
> >  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->policy_dbs || cdbs->policy_dbs->policy)
> > +	if (policy_dbs->policy)
> >  		return -EBUSY;
> 
> We can crash here if policy_dbs is NULL, which can happen if EXIT is called
> twice (due to the lock-dropping thing in cpufreq-core). So we really need to
> keep these checks for now atleast.

Well no, things have changed now a bit with your recent changes and so this
wouldn't be an issue here. Sorry for the noise.

-- 
viresh

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


#1328514 — Re: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-07 17:00 +0100
SubjectRe: [PATCH 2/3] cpufreq: governor: Rearrange governor data structures
Message-ID<qZzUK-87R-13@gated-at.bofh.it>
In reply to#1328506
On 07-02-16, 16:24, 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 policy_dbs 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 (instead of struct dbs_data) 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     |   74 ++++++++++++++++-----------------
>  drivers/cpufreq/cpufreq_governor.h     |   27 ++++++------
>  drivers/cpufreq/cpufreq_ondemand.c     |   18 ++++----
>  5 files changed, 68 insertions(+), 60 deletions(-)

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

-- 
viresh

[toc] | [prev] | [standalone]


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

Back to top | Article view | linux.kernel


csiph-web