Re: [PATCH V3 2/4] cpufreq: governor: Implement per policy instancesof governors

From: Viresh Kumar
Date: Wed Mar 27 2013 - 06:04:16 EST

On 27 March 2013 09:59, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> On 27 March 2013 01:18, Jacob Shin <jacob.shin@xxxxxxx> wrote:

>> Hmm .. I don't think this works for both ondemand and conservative
>> governors running at the same time .

Hi Jacob,

This must be early morning for you and you must be looking for something
to start with.

I am still stuck at a issue, which i am not able to fix.
- cat of */cpufreq/ondemand/** isn't showing anything on console, but all
pointers are correctly set and i can see the right values with printk()
- I am able to see all prints for have_multiple_policies enabled.
- Due to this, i haven't tested any corner cases for now.

@Rafael: I will post all this as a separate patch later which you can then
fold into original commit. I am just waiting for a complete fix.

Following is the work i have done until now (attached too), leave earlier
fixups:

--------x-------------x------------------

From: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date: Tue, 26 Mar 2013 23:20:18 +0530
Subject: [PATCH] fixup! cpufreq: governor: Implement per policy instances of
governors

---
drivers/cpufreq/cpufreq.c | 13 +++++++
drivers/cpufreq/cpufreq_conservative.c | 24 ++++++------
drivers/cpufreq/cpufreq_governor.c | 71 ++++++++++++++++++++++++----------
drivers/cpufreq/cpufreq_governor.h | 28 ++++++++++++--
drivers/cpufreq/cpufreq_ondemand.c | 25 ++++++------
include/linux/cpufreq.h | 2 +
6 files changed, 113 insertions(+), 50 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 3d83b02..3d990b4 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -136,6 +136,11 @@ struct kobject *get_governor_parent_kobj(struct
cpufreq_policy *policy)
return cpufreq_global_kobject;
}

+bool have_multiple_policies(void)
+{
+ return cpufreq_driver->have_multiple_policies;
+}
+
static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs)
{
struct cpufreq_policy *data;
@@ -1554,6 +1559,13 @@ static int __cpufreq_governor(struct
cpufreq_policy *policy,
policy->cpu, event);
ret = policy->governor->governor(policy, event);

+ if (!ret) {
+ if (event == CPUFREQ_GOV_POLICY_INIT)
+ policy->governor->initialized++;
+ else if (event == CPUFREQ_GOV_POLICY_EXIT)
+ policy->governor->initialized--;
+ }
+
/* we keep one module reference alive for
each CPU governed by this CPU */
if ((event != CPUFREQ_GOV_START) || ret)
@@ -1577,6 +1589,7 @@ int cpufreq_register_governor(struct
cpufreq_governor *governor)

mutex_lock(&cpufreq_governor_mutex);

+ governor->initialized = 0;
err = -EBUSY;
if (__find_governor(governor->name) == NULL) {
err = 0;
diff --git a/drivers/cpufreq/cpufreq_conservative.c
b/drivers/cpufreq/cpufreq_conservative.c
index 63499c8..c09d7c8 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -157,11 +157,13 @@ static int dbs_cpufreq_notifier(struct
notifier_block *nb, unsigned long val,
}

/************************** sysfs interface ************************/
+static struct common_dbs_data cs_dbs_cdata;
+declare_get_tuners(cs);
+
static ssize_t store_sampling_down_factor(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ struct cs_dbs_tuners *cs_tuners = cs_get_tuners(policy, NULL);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -176,8 +178,8 @@ static ssize_t store_sampling_down_factor(struct
cpufreq_policy *policy,
static ssize_t store_sampling_rate(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ struct dbs_data *dbs_data = NULL;
+ struct cs_dbs_tuners *cs_tuners = cs_get_tuners(policy, &dbs_data);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -192,8 +194,7 @@ static ssize_t store_sampling_rate(struct
cpufreq_policy *policy,
static ssize_t store_up_threshold(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ struct cs_dbs_tuners *cs_tuners = cs_get_tuners(policy, NULL);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -208,8 +209,7 @@ static ssize_t store_up_threshold(struct
cpufreq_policy *policy,
static ssize_t store_down_threshold(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ struct cs_dbs_tuners *cs_tuners = cs_get_tuners(policy, NULL);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -226,8 +226,7 @@ static ssize_t store_down_threshold(struct
cpufreq_policy *policy,
static ssize_t store_ignore_nice(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ struct cs_dbs_tuners *cs_tuners = cs_get_tuners(policy, NULL);
unsigned int input, j;
int ret;

@@ -259,8 +258,7 @@ static ssize_t store_ignore_nice(struct
cpufreq_policy *policy,
static ssize_t store_freq_step(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
+ struct cs_dbs_tuners *cs_tuners = cs_get_tuners(policy, NULL);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -285,7 +283,7 @@ show_one(cs, up_threshold, up_threshold);
show_one(cs, down_threshold, down_threshold);
show_one(cs, ignore_nice, ignore_nice);
show_one(cs, freq_step, freq_step);
-declare_show_sampling_rate_min();
+declare_show_sampling_rate_min(cs);

cpufreq_freq_attr_rw(sampling_rate);
cpufreq_freq_attr_rw(sampling_down_factor);
diff --git a/drivers/cpufreq/cpufreq_governor.c
b/drivers/cpufreq/cpufreq_governor.c
index 41e5e56..54ca5fc 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -216,10 +216,9 @@ static void set_sampling_rate(struct dbs_data *dbs_data,
int cpufreq_governor_dbs(struct cpufreq_policy *policy,
struct common_dbs_data *cdata, unsigned int event)
{
- struct dbs_data *dbs_data = policy->governor_data;
+ struct dbs_data *dbs_data;
struct od_cpu_dbs_info_s *od_dbs_info = NULL;
struct cs_cpu_dbs_info_s *cs_dbs_info = NULL;
- struct cs_ops *cs_ops = NULL;
struct od_ops *od_ops = NULL;
struct od_dbs_tuners *od_tuners = NULL;
struct cs_dbs_tuners *cs_tuners = NULL;
@@ -228,11 +227,22 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
int io_busy = 0;
int rc;

+ if (have_multiple_policies())
+ dbs_data = policy->governor_data;
+ else
+ dbs_data = cdata->gdbs_data;
+
WARN_ON(!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT));

switch (event) {
case CPUFREQ_GOV_POLICY_INIT:
- WARN_ON(dbs_data);
+ if (have_multiple_policies()) {
+ WARN_ON(dbs_data);
+ } else if (dbs_data) {
+ policy->governor_data = dbs_data;
+ return 0;
+ }
+
dbs_data = kzalloc(sizeof(*dbs_data), GFP_KERNEL);
if (!dbs_data) {
pr_err("%s: POLICY_INIT: kzalloc failed\n", __func__);
@@ -246,6 +256,15 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
kfree(dbs_data);
return rc;
}
+
+ rc = sysfs_create_group(get_governor_parent_kobj(policy),
+ dbs_data->cdata->attr_group);
+ if (rc) {
+ cdata->exit(dbs_data);
+ kfree(dbs_data);
+ return rc;
+ }
+
policy->governor_data = dbs_data;

/* policy latency is in nS. Convert it to uS first */
@@ -258,10 +277,36 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
MIN_LATENCY_MULTIPLIER * latency);
set_sampling_rate(dbs_data, max(dbs_data->min_sampling_rate,
latency * LATENCY_MULTIPLIER));
+
+ if (dbs_data->cdata->governor == GOV_CONSERVATIVE) {
+ struct cs_ops *cs_ops = dbs_data->cdata->gov_ops;
+
+ cpufreq_register_notifier(cs_ops->notifier_block,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ }
+
+ if (!have_multiple_policies())
+ cdata->gdbs_data = dbs_data;
+
return 0;
case CPUFREQ_GOV_POLICY_EXIT:
- cdata->exit(dbs_data);
- kfree(dbs_data);
+ if ((policy->governor->initialized == 1) ||
+ have_multiple_policies()) {
+ sysfs_remove_group(get_governor_parent_kobj(policy),
+ dbs_data->cdata->attr_group);
+
+ if (dbs_data->cdata->governor == GOV_CONSERVATIVE) {
+ struct cs_ops *cs_ops = dbs_data->cdata->gov_ops;
+
+ cpufreq_register_notifier(cs_ops->notifier_block,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ }
+
+ cdata->exit(dbs_data);
+ kfree(dbs_data);
+ cdata->gdbs_data = NULL;
+ }
+
policy->governor_data = NULL;
return 0;
}
@@ -273,7 +318,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
cs_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
sampling_rate = cs_tuners->sampling_rate;
ignore_nice = cs_tuners->ignore_nice;
- cs_ops = dbs_data->cdata->gov_ops;
} else {
od_tuners = dbs_data->tuners;
od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
@@ -307,13 +351,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
dbs_data->cdata->gov_dbs_timer);
}

- rc = sysfs_create_group(get_governor_parent_kobj(policy),
- dbs_data->cdata->attr_group);
- if (rc) {
- mutex_unlock(&dbs_data->mutex);
- return rc;
- }
-
/*
* conservative does not implement micro like ondemand
* governor, thus we are bound to jiffes/HZ
@@ -322,9 +359,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
cs_dbs_info->down_skip = 0;
cs_dbs_info->enable = 1;
cs_dbs_info->requested_freq = policy->cur;
-
- cpufreq_register_notifier(cs_ops->notifier_block,
- CPUFREQ_TRANSITION_NOTIFIER);
} else {
od_dbs_info->rate_mult = 1;
od_dbs_info->sample_type = OD_NORMAL_SAMPLE;
@@ -349,11 +383,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
mutex_lock(&dbs_data->mutex);
mutex_destroy(&cpu_cdbs->timer_mutex);

- sysfs_remove_group(get_governor_parent_kobj(policy),
- dbs_data->cdata->attr_group);
- if (dbs_data->cdata->governor == GOV_CONSERVATIVE)
- cpufreq_unregister_notifier(cs_ops->notifier_block,
- CPUFREQ_TRANSITION_NOTIFIER);
mutex_unlock(&dbs_data->mutex);

break;
diff --git a/drivers/cpufreq/cpufreq_governor.h
b/drivers/cpufreq/cpufreq_governor.h
index 8b18d25..fcf10d9 100644
--- a/drivers/cpufreq/cpufreq_governor.h
+++ b/drivers/cpufreq/cpufreq_governor.h
@@ -40,13 +40,29 @@
/* Ondemand Sampling types */
enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE};

+
+#define declare_get_tuners(_gov) \
+static struct _gov##_dbs_tuners *_gov##_get_tuners \
+(struct cpufreq_policy *policy, struct dbs_data **ldbs_data) \
+{ \
+ if (have_multiple_policies()) { \
+ struct dbs_data *dbs_data = policy->governor_data; \
+ if (ldbs_data) \
+ *ldbs_data = dbs_data; \
+ return dbs_data->tuners; \
+ } else { \
+ if (ldbs_data) \
+ *ldbs_data = _gov##_dbs_cdata.gdbs_data; \
+ return _gov##_dbs_cdata.gdbs_data->tuners; \
+ } \
+}
+
/* Macro creating sysfs show routines */
#define show_one(_gov, file_name, object) \
static ssize_t show_##file_name \
(struct cpufreq_policy *policy, char *buf) \
{ \
- struct dbs_data *dbs_data = policy->governor_data; \
- struct _gov##_dbs_tuners *tuners = dbs_data->tuners; \
+ struct _gov##_dbs_tuners *tuners = _gov##_get_tuners(policy, NULL); \
return sprintf(buf, "%u\n", tuners->file_name); \
}

@@ -133,6 +149,9 @@ struct common_dbs_data {
int governor;
struct attribute_group *attr_group;

+ /* Common data for platforms that don't set have_multiple_policies */
+ struct dbs_data *gdbs_data;
+
struct cpu_dbs_common_info *(*get_cpu_cdbs)(int cpu);
void *(*get_cpu_dbs_info_s)(int cpu);
void (*gov_dbs_timer)(struct work_struct *work);
@@ -177,11 +196,12 @@ static inline int
delay_for_sampling_rate(unsigned int sampling_rate)
return delay;
}

-#define declare_show_sampling_rate_min() \
+#define declare_show_sampling_rate_min(_gov) \
static ssize_t show_sampling_rate_min(struct cpufreq_policy *policy, \
char *buf) \
{ \
- struct dbs_data *dbs_data = policy->governor_data; \
+ struct dbs_data *dbs_data = NULL; \
+ _gov##_get_tuners(policy, &dbs_data); \
return sprintf(buf, "%u\n", dbs_data->min_sampling_rate); \
}

diff --git a/drivers/cpufreq/cpufreq_ondemand.c
b/drivers/cpufreq/cpufreq_ondemand.c
index 29ed48a..4860137 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -257,6 +257,9 @@ max_delay:
}

/************************** sysfs interface ************************/
+static struct common_dbs_data od_dbs_cdata;
+declare_get_tuners(od);
+
/**
* update_sampling_rate - update sampling rate effective immediately if needed.
* @new_rate: new sampling rate
@@ -321,12 +324,14 @@ static void update_sampling_rate(struct dbs_data
*dbs_data,
static ssize_t store_sampling_rate(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
+ struct dbs_data *dbs_data = NULL;
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
if (ret != 1)
return -EINVAL;
+
+ od_get_tuners(policy, &dbs_data);
update_sampling_rate(dbs_data, input);
return count;
}
@@ -334,8 +339,7 @@ static ssize_t store_sampling_rate(struct
cpufreq_policy *policy,
static ssize_t store_io_is_busy(struct cpufreq_policy *policy, const char *buf,
size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct od_dbs_tuners *od_tuners = dbs_data->tuners;
+ struct od_dbs_tuners *od_tuners = od_get_tuners(policy, NULL);
unsigned int input;
int ret;
unsigned int j;
@@ -358,8 +362,7 @@ static ssize_t store_io_is_busy(struct
cpufreq_policy *policy, const char *buf,
static ssize_t store_up_threshold(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct od_dbs_tuners *od_tuners = dbs_data->tuners;
+ struct od_dbs_tuners *od_tuners = od_get_tuners(policy, NULL);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -379,8 +382,7 @@ static ssize_t store_up_threshold(struct
cpufreq_policy *policy,
static ssize_t store_sampling_down_factor(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct od_dbs_tuners *od_tuners = dbs_data->tuners;
+ struct od_dbs_tuners *od_tuners = od_get_tuners(policy, NULL);
unsigned int input, j;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -401,8 +403,7 @@ static ssize_t store_sampling_down_factor(struct
cpufreq_policy *policy,
static ssize_t store_ignore_nice(struct cpufreq_policy *policy, const
char *buf,
size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct od_dbs_tuners *od_tuners = dbs_data->tuners;
+ struct od_dbs_tuners *od_tuners = od_get_tuners(policy, NULL);
unsigned int input;
int ret;

@@ -437,8 +438,7 @@ static ssize_t store_ignore_nice(struct
cpufreq_policy *policy, const char *buf,
static ssize_t store_powersave_bias(struct cpufreq_policy *policy,
const char *buf, size_t count)
{
- struct dbs_data *dbs_data = policy->governor_data;
- struct od_dbs_tuners *od_tuners = dbs_data->tuners;
+ struct od_dbs_tuners *od_tuners = od_get_tuners(policy, NULL);
unsigned int input;
int ret;
ret = sscanf(buf, "%u", &input);
@@ -460,7 +460,7 @@ show_one(od, up_threshold, up_threshold);
show_one(od, sampling_down_factor, sampling_down_factor);
show_one(od, ignore_nice, ignore_nice);
show_one(od, powersave_bias, powersave_bias);
-declare_show_sampling_rate_min();
+declare_show_sampling_rate_min(od);

cpufreq_freq_attr_rw(sampling_rate);
cpufreq_freq_attr_rw(io_is_busy);
@@ -530,6 +530,7 @@ static int od_init(struct dbs_data *dbs_data)
tuners->io_is_busy = should_io_be_busy();

dbs_data->tuners = tuners;
+ pr_info("%s: tuners %p\n", __func__, tuners);
mutex_init(&dbs_data->mutex);
return 0;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index dd53cea..394ea0d 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -187,6 +187,7 @@ static inline unsigned long cpufreq_scale(unsigned
long old, u_int div, u_int mu

struct cpufreq_governor {
char name[CPUFREQ_NAME_LEN];
+ int initialized;
int (*governor) (struct cpufreq_policy *policy,
unsigned int event);
ssize_t (*show_setspeed) (struct cpufreq_policy *policy,
@@ -323,6 +324,7 @@ const char *cpufreq_get_current_driver(void);
int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu);
int cpufreq_update_policy(unsigned int cpu);
struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
+bool have_multiple_policies(void);

#ifdef CONFIG_CPU_FREQ
/* query the current CPU frequency (in kHz). If zero, cpufreq
couldn't detect it */

Attachment: 0001-fixup-cpufreq-governor-Implement-per-policy-instance.patch
Description: Binary data