Hi Rafael et. al.,
We had some discussions [1] earlier on the PM mailing about
upstreaming the cpufreq governor most widely used on Android Mobile
phones and tablets: Interactive governor.
People (including Rafael) mostly agreed that we better get it upstreamed
and here is an attempt to upstream most of it (idle notifiers aren't
included in this series).
I picked the latest code spread over 70-80 patches from [2]. The
unmodified code, based over mainline is pushed [4] for reference.
I have updated the governor to align it with the current practices
followed with mainline governors, like using utilization hooks from the
scheduler and handling kobject (for governor's sysfs directory) in a
race free manner. And of course this included general cleanup of the
governor as well. This version is pushed here [3] for testing.
The Android version of interactive governor also uses idle EXIT
notifiers, but that code isn't part of this series and will be sent
separately later. For people interested in looking at that, those are 3
minor patches on top of this series and are pushed here [5].
I haven't changed the core logic of the governor intentionally, as that
rather requires more in-depth knowledge of the use case for which the
optimizations have been done. So, we should do any such thing later on
with new patches, so that people can fix regressions easily.
I also haven't tried to change the userspace interface as that may have
broken the userspace that already exists and uses this governor.
This has been lightly tested on my Exynos board (dual ARM A15), where
the governor gets inserted/removed multiple times, the sysfs files are
all functional. The frequency gets changed with load, etc.
Amit Pundir (Linaro) has done extensive testing with Android and
compared results of both Original Interactive governor and the modified
one (in this series) and found them to be comparable.
This series is based of pm/bleeding-edge branch.
V1->V2:
- Changes to fix compilation issues with latest mainline
- Timer APIs got updated
- s/mod_timer_pinned/mod_timer
- s/init_timer/init_timer_pinned
- Updated prototypes of cpufreq_frequency_table_target() and
update_util_handler()
--
viresh
[1] http://marc.info/?l=linux-pm&m=146301864519072
[2] https://android.googlesource.com/kernel/common remotes/android/android-4.4
[3] git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git cpufreq/interactive
[4] git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git cpufreq/interactive-orig
[5] git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git cpufreq/intearctive-idle-notifier
Viresh Kumar (2):
cpufreq: Move gov_attr_* macros to cpufreq.h
cpufreq: Add android's 'interactive' governor
Documentation/cpu-freq/governors.txt | 86 ++
drivers/cpufreq/Kconfig | 30 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/cpufreq_governor.h | 8 -
drivers/cpufreq/cpufreq_interactive.c | 1363 ++++++++++++++++++++++++++++
include/linux/cpufreq.h | 12 +
include/trace/events/cpufreq_interactive.h | 112 +++
kernel/sched/cpufreq_schedutil.c | 8 +-
8 files changed, 1608 insertions(+), 12 deletions(-)
create mode 100644 drivers/cpufreq/cpufreq_interactive.c
create mode 100644 include/trace/events/cpufreq_interactive.h
--
2.7.1.410.g6faf27b
This is leftover from an earlier patch which removed the usage of
platform data but forgot to remove this line. Remove it now.
Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
---
drivers/cpufreq/cpufreq-dt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 3957de801ae8..2bd20534155d 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -366,8 +366,6 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
if (ret)
return ret;
- dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
-
ret = cpufreq_register_driver(&dt_cpufreq_driver);
if (ret)
dev_err(&pdev->dev, "failed register driver: %d\n", ret);
--
2.7.1.410.g6faf27b
If a cpufreq driver is registered very early in the boot stage (e.g.
registered from postcore_initcall()), then cpufreq core may generate
kernel warnings for it.
In this case, the CPUs are brought online, then the cpufreq driver is
registered, and then the CPU topology devices are registered. However,
by the time cpufreq_add_dev() gets called, the cpu device isn't stored
in the per-cpu variable (cpu_sys_devices,) which is read by
get_cpu_device().
So the cpufreq core fails to get device for the CPU, for which
cpufreq_add_dev() was called in the first place and we will hit a
WARN_ON(!cpu_dev).
Even if we reuse the 'dev' parameter passed to cpufreq_add_dev() to
avoid that warning, there might be other CPUs online that share the
policy with the cpu for which cpufreq_add_dev() is called. Eventually
get_cpu_device() will return NULL for them as well, and we will hit the
same WARN_ON() again.
In order to fix these issues, change cpufreq core to create links to the
policy for a cpu only when cpufreq_add_dev() is called for that CPU.
Reuse the 'real_cpus' mask to track that as well.
Note that cpufreq_remove_dev() already handles removal of the links for
individual CPUs and cpufreq_add_dev() has aligned with that now.
Reported-by: Russell King <rmk+kernel(a)arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel(a)arm.linux.org.uk>
Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
---
V1->V2:
- Updated changelog based on suggestions from Russell
- Tested by from Russell
drivers/cpufreq/cpufreq.c | 89 +++++++++++++++--------------------------------
1 file changed, 28 insertions(+), 61 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 13fb589b6d2c..3a64136bf21b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -916,58 +916,18 @@ static struct kobj_type ktype_cpufreq = {
.release = cpufreq_sysfs_release,
};
-static int add_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
+static int add_cpu_dev_symlink(struct cpufreq_policy *policy,
+ struct device *dev)
{
- struct device *cpu_dev;
-
- pr_debug("%s: Adding symlink for CPU: %u\n", __func__, cpu);
-
- if (!policy)
- return 0;
-
- cpu_dev = get_cpu_device(cpu);
- if (WARN_ON(!cpu_dev))
- return 0;
-
- return sysfs_create_link(&cpu_dev->kobj, &policy->kobj, "cpufreq");
+ dev_dbg(dev, "%s: Adding symlink\n", __func__);
+ return sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq");
}
-static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu)
+static void remove_cpu_dev_symlink(struct cpufreq_policy *policy,
+ struct device *dev)
{
- struct device *cpu_dev;
-
- pr_debug("%s: Removing symlink for CPU: %u\n", __func__, cpu);
-
- cpu_dev = get_cpu_device(cpu);
- if (WARN_ON(!cpu_dev))
- return;
-
- sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
-}
-
-/* Add/remove symlinks for all related CPUs */
-static int cpufreq_add_dev_symlink(struct cpufreq_policy *policy)
-{
- unsigned int j;
- int ret = 0;
-
- /* Some related CPUs might not be present (physically hotplugged) */
- for_each_cpu(j, policy->real_cpus) {
- ret = add_cpu_dev_symlink(policy, j);
- if (ret)
- break;
- }
-
- return ret;
-}
-
-static void cpufreq_remove_dev_symlink(struct cpufreq_policy *policy)
-{
- unsigned int j;
-
- /* Some related CPUs might not be present (physically hotplugged) */
- for_each_cpu(j, policy->real_cpus)
- remove_cpu_dev_symlink(policy, j);
+ dev_dbg(dev, "%s: Removing symlink\n", __func__);
+ sysfs_remove_link(&dev->kobj, "cpufreq");
}
static int cpufreq_add_dev_interface(struct cpufreq_policy *policy)
@@ -999,7 +959,7 @@ static int cpufreq_add_dev_interface(struct cpufreq_policy *policy)
return ret;
}
- return cpufreq_add_dev_symlink(policy);
+ return 0;
}
__weak struct cpufreq_governor *cpufreq_default_governor(void)
@@ -1129,7 +1089,6 @@ static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy, bool notify)
down_write(&policy->rwsem);
cpufreq_stats_free_table(policy);
- cpufreq_remove_dev_symlink(policy);
kobj = &policy->kobj;
cmp = &policy->kobj_unregister;
up_write(&policy->rwsem);
@@ -1211,8 +1170,8 @@ static int cpufreq_online(unsigned int cpu)
if (new_policy) {
/* related_cpus should at least include policy->cpus. */
cpumask_copy(policy->related_cpus, policy->cpus);
- /* Remember CPUs present at the policy creation time. */
- cpumask_and(policy->real_cpus, policy->cpus, cpu_present_mask);
+ /* Clear mask of registered CPUs */
+ cpumask_clear(policy->real_cpus);
}
/*
@@ -1327,6 +1286,8 @@ static int cpufreq_online(unsigned int cpu)
return ret;
}
+static void cpufreq_offline(unsigned int cpu);
+
/**
* cpufreq_add_dev - the cpufreq interface for a CPU device.
* @dev: CPU device.
@@ -1336,22 +1297,28 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
{
struct cpufreq_policy *policy;
unsigned cpu = dev->id;
+ int ret;
dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu);
- if (cpu_online(cpu))
- return cpufreq_online(cpu);
+ if (cpu_online(cpu)) {
+ ret = cpufreq_online(cpu);
+ if (ret)
+ return ret;
+ }
- /*
- * A hotplug notifier will follow and we will handle it as CPU online
- * then. For now, just create the sysfs link, unless there is no policy
- * or the link is already present.
- */
+ /* Create sysfs link on CPU registration */
policy = per_cpu(cpufreq_cpu_data, cpu);
if (!policy || cpumask_test_and_set_cpu(cpu, policy->real_cpus))
return 0;
- return add_cpu_dev_symlink(policy, cpu);
+ ret = add_cpu_dev_symlink(policy, dev);
+ if (ret) {
+ cpumask_clear_cpu(cpu, policy->real_cpus);
+ cpufreq_offline(cpu);
+ }
+
+ return ret;
}
static void cpufreq_offline(unsigned int cpu)
@@ -1432,7 +1399,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif)
cpufreq_offline(cpu);
cpumask_clear_cpu(cpu, policy->real_cpus);
- remove_cpu_dev_symlink(policy, cpu);
+ remove_cpu_dev_symlink(policy, dev);
if (cpumask_empty(policy->real_cpus))
cpufreq_policy_free(policy, true);
--
2.7.1.410.g6faf27b