Re: [PATCH 0/4] CPUFreq: Implement per policy instances of governors

From: Viresh Kumar
Date: Tue Feb 05 2013 - 11:21:16 EST


On 4 February 2013 17:08, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> Currently, there can't be multiple instances of single governor_type. If we have
> a multi-package system, where we have multiple instances of struct policy (per
> package), we can't have multiple instances of same governor. i.e. We can't have
> multiple instances of ondemand governor for multiple packages.
>
> Governors directory in sysfs is created at /sys/devices/system/cpu/cpufreq/
> governor-name/. Which again reflects that there can be only one instance of a
> governor_type in the system.
>
> This is a bottleneck for multicluster system, where we want different packages
> to use same governor type, but with different tunables.
>
> This patchset is inclined towards fixing this issue.

After a long & hot discussion over the changes made by this patchset, following
patches came out:

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

commit 15b5548c9ccfb8088270f7574710d9d67edfe33b
Author: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date: Tue Feb 5 21:29:05 2013 +0530

cpufreq: Make governors directory sysfs location based on
have_multiple_policies

Until now directory for governors tunables was getting created in
cpu/cpufreq/<gov-name>. With the introduction of following patch:
"cpufreq: governor: Implement per policy instances of governors"

this directory would be created in
cpu/cpu<num>/cpufreq/<gov-name>. This might
break userspace of existing platforms. Lets do this change only
for platforms
which need support for multiple policies and thus above mentioned patch.

From now on, such platforms would be require to do following from
their init()
routines:

policy->have_multiple_policies = true;

Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
drivers/cpufreq/cpufreq_governor.c | 2 +-
include/linux/cpufreq.h | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq_governor.c
b/drivers/cpufreq/cpufreq_governor.c
index 545ae24..41ee86f 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -300,7 +300,7 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
dbs_data->cdata->gov_dbs_timer);
}

- rc = sysfs_create_group(&policy->kobj,
+ rc = sysfs_create_group(get_governor_parent_kobj(policy),
dbs_data->cdata->attr_group);
if (rc) {
mutex_unlock(&dbs_data->mutex);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 5dae7e6..6e1abd2 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -107,6 +107,11 @@ struct cpufreq_policy {
unsigned int policy; /* see above */
struct cpufreq_governor *governor; /* see below */
void *governor_data;
+ /* This should be set by init() of platforms having multiple
+ * clock-domains, i.e. supporting multiple policies. With this sysfs
+ * directories of governor would be created in cpu/cpu<num>/cpufreq/
+ * directory */
+ bool have_multiple_policies;

struct work_struct update; /* if update_policy() needs to be
* called, but you're in IRQ context */
@@ -134,6 +139,15 @@ static inline bool policy_is_shared(struct
cpufreq_policy *policy)
return cpumask_weight(policy->cpus) > 1;
}

+static inline struct kobject *
+get_governor_parent_kobj(struct cpufreq_policy *policy)
+{
+ if (policy->have_multiple_policies)
+ return &policy->kobj;
+ else
+ return cpufreq_global_kobject;
+}
+
/******************** cpufreq transition notifiers *******************/

#define CPUFREQ_PRECHANGE (0)

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

Plus the following patch, though i am still not in favor of this patch.
@Rafael: Please share your opinion too on this one. :)

--------------x------------------x-------------------
commit 1c7e9871fce7388136eda1c86ca75a520e4d3b9d
Author: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date: Tue Feb 5 21:41:40 2013 +0530

cpufreq: Add Kconfig option to enable/disable have_multiple_policies

have_multiple_policies is required by platforms having multiple
clock-domains
for cpus, i.e. supporting multiple policies for cpus. This patch adds in a
Kconfig option for enabling execution of this code.

Requested-by: Borislav Petkov <bp@xxxxxxxxx>
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
drivers/cpufreq/Kconfig | 3 +++
include/linux/cpufreq.h | 4 ++++
2 files changed, 7 insertions(+)

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index cbcb21e..e6e6939 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -23,6 +23,9 @@ config CPU_FREQ_TABLE
config CPU_FREQ_GOV_COMMON
bool

+config CPU_FREQ_HAVE_MULTIPLE_POLICIES
+ bool
+
config CPU_FREQ_STAT
tristate "CPU frequency translation statistics"
select CPU_FREQ_TABLE
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6e1abd2..a092fcb 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -107,11 +107,13 @@ struct cpufreq_policy {
unsigned int policy; /* see above */
struct cpufreq_governor *governor; /* see below */
void *governor_data;
+#ifdef CONFIG_CPU_FREQ_HAVE_MULTIPLE_POLICIES
/* This should be set by init() of platforms having multiple
* clock-domains, i.e. supporting multiple policies. With this sysfs
* directories of governor would be created in cpu/cpu<num>/cpufreq/
* directory */
bool have_multiple_policies;
+#endif

struct work_struct update; /* if update_policy() needs to be
* called, but you're in IRQ context */
@@ -142,9 +144,11 @@ static inline bool policy_is_shared(struct
cpufreq_policy *policy)
static inline struct kobject *
get_governor_parent_kobj(struct cpufreq_policy *policy)
{
+#ifdef CONFIG_CPU_FREQ_HAVE_MULTIPLE_POLICIES
if (policy->have_multiple_policies)
return &policy->kobj;
else
+#endif
return cpufreq_global_kobject;
}


--------------x------------------x-------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/