Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1577413 > unrolled thread
| Started by | Juri Lelli <juri.lelli@arm.com> |
|---|---|
| First post | 2017-02-09 10:30 +0100 |
| Last post | 2017-02-13 16:20 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 6/9] arm, arm64: factorize common cpu capacity default code Juri Lelli <juri.lelli@arm.com> - 2017-02-09 10:30 +0100
Re: [PATCH v2 6/9] arm, arm64: factorize common cpu capacity default code Greg KH <gregkh@linuxfoundation.org> - 2017-02-10 15:40 +0100
Re: [PATCH v2 6/9] arm, arm64: factorize common cpu capacity default code Juri Lelli <juri.lelli@arm.com> - 2017-02-13 16:20 +0100
| From | Juri Lelli <juri.lelli@arm.com> |
|---|---|
| Date | 2017-02-09 10:30 +0100 |
| Subject | [PATCH v2 6/9] arm, arm64: factorize common cpu capacity default code |
| Message-ID | <t8Td9-4n9-29@gated-at.bofh.it> |
arm and arm64 share lot of code relative to parsing CPU capacity
information from DT, using that information for appropriate scaling and
exposing a sysfs interface for chaging such values at runtime.
Factorize such code in a common place (driver/base/arch_topology.c) in
preparation for further additions.
Suggested-by: Will Deacon <will.deacon@arm.com>
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
Changes from v1:
- keep the original GPLv2 header
---
arch/arm/Kconfig | 1 +
arch/arm/kernel/topology.c | 213 ++------------------------------------
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/topology.c | 219 +--------------------------------------
drivers/base/Kconfig | 8 ++
drivers/base/Makefile | 1 +
drivers/base/arch_topology.c | 237 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 257 insertions(+), 423 deletions(-)
create mode 100644 drivers/base/arch_topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 186c4c214e0a..6dd5736c1e3c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -19,6 +19,7 @@ config ARM
select EDAC_SUPPORT
select EDAC_ATOMIC_SCRUB
select GENERIC_ALLOCATOR
+ select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
select GENERIC_CLOCKEVENTS_BROADCAST if SMP
select GENERIC_EARLY_IOREMAP
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index c760a321935b..51e9ed6439f1 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -43,75 +43,10 @@
* to run the rebalance_domains for all idle cores and the cpu_capacity can be
* updated during this sequence.
*/
-static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
-static DEFINE_MUTEX(cpu_scale_mutex);
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
-{
- return per_cpu(cpu_scale, cpu);
-}
-
-static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
-{
- per_cpu(cpu_scale, cpu) = capacity;
-}
-
-static ssize_t cpu_capacity_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
-
- return sprintf(buf, "%lu\n",
- arch_scale_cpu_capacity(NULL, cpu->dev.id));
-}
-
-static ssize_t cpu_capacity_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t count)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
- int this_cpu = cpu->dev.id, i;
- unsigned long new_capacity;
- ssize_t ret;
-
- if (count) {
- ret = kstrtoul(buf, 0, &new_capacity);
- if (ret)
- return ret;
- if (new_capacity > SCHED_CAPACITY_SCALE)
- return -EINVAL;
-
- mutex_lock(&cpu_scale_mutex);
- for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
- set_capacity_scale(i, new_capacity);
- mutex_unlock(&cpu_scale_mutex);
- }
-
- return count;
-}
-
-static DEVICE_ATTR_RW(cpu_capacity);
-
-static int register_cpu_capacity_sysctl(void)
-{
- int i;
- struct device *cpu;
-
- for_each_possible_cpu(i) {
- cpu = get_cpu_device(i);
- if (!cpu) {
- pr_err("%s: too early to get CPU%d device!\n",
- __func__, i);
- continue;
- }
- device_create_file(cpu, &dev_attr_cpu_capacity);
- }
-
- return 0;
-}
-subsys_initcall(register_cpu_capacity_sysctl);
+extern unsigned long
+arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+extern void set_capacity_scale(unsigned int cpu, unsigned long capacity);
#ifdef CONFIG_OF
struct cpu_efficiency {
@@ -140,145 +75,9 @@ static unsigned long *__cpu_capacity;
static unsigned long middle_capacity = 1;
static bool cap_from_dt = true;
-static u32 *raw_capacity;
-static bool cap_parsing_failed;
-static u32 capacity_scale;
-
-static int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
-{
- int ret = 1;
- u32 cpu_capacity;
-
- if (cap_parsing_failed)
- return !ret;
-
- ret = of_property_read_u32(cpu_node,
- "capacity-dmips-mhz",
- &cpu_capacity);
- if (!ret) {
- if (!raw_capacity) {
- raw_capacity = kcalloc(num_possible_cpus(),
- sizeof(*raw_capacity),
- GFP_KERNEL);
- if (!raw_capacity) {
- pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
- cap_parsing_failed = true;
- return ret;
- }
- }
- capacity_scale = max(cpu_capacity, capacity_scale);
- raw_capacity[cpu] = cpu_capacity;
- pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
- cpu_node->full_name, raw_capacity[cpu]);
- } else {
- if (raw_capacity) {
- pr_err("cpu_capacity: missing %s raw capacity\n",
- cpu_node->full_name);
- pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
- }
- cap_parsing_failed = true;
- kfree(raw_capacity);
- }
-
- return !ret;
-}
-
-static void normalize_cpu_capacity(void)
-{
- u64 capacity;
- int cpu;
-
- if (!raw_capacity || cap_parsing_failed)
- return;
-
- pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
- mutex_lock(&cpu_scale_mutex);
- for_each_possible_cpu(cpu) {
- capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
- / capacity_scale;
- set_capacity_scale(cpu, capacity);
- pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
- cpu, arch_scale_cpu_capacity(NULL, cpu));
- }
- mutex_unlock(&cpu_scale_mutex);
-}
-
-#ifdef CONFIG_CPU_FREQ
-static cpumask_var_t cpus_to_visit;
-static bool cap_parsing_done;
-static void parsing_done_workfn(struct work_struct *work);
-static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
-
-static int
-init_cpu_capacity_callback(struct notifier_block *nb,
- unsigned long val,
- void *data)
-{
- struct cpufreq_policy *policy = data;
- int cpu;
-
- if (cap_parsing_failed || cap_parsing_done)
- return 0;
-
- switch (val) {
- case CPUFREQ_NOTIFY:
- pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
- cpumask_pr_args(policy->related_cpus),
- cpumask_pr_args(cpus_to_visit));
- cpumask_andnot(cpus_to_visit,
- cpus_to_visit,
- policy->related_cpus);
- for_each_cpu(cpu, policy->related_cpus) {
- raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
- policy->cpuinfo.max_freq / 1000UL;
- capacity_scale = max(raw_capacity[cpu], capacity_scale);
- }
- if (cpumask_empty(cpus_to_visit)) {
- normalize_cpu_capacity();
- kfree(raw_capacity);
- pr_debug("cpu_capacity: parsing done\n");
- cap_parsing_done = true;
- schedule_work(&parsing_done_work);
- }
- }
- return 0;
-}
-
-static struct notifier_block init_cpu_capacity_notifier = {
- .notifier_call = init_cpu_capacity_callback,
-};
-
-static int __init register_cpufreq_notifier(void)
-{
- if (cap_parsing_failed)
- return -EINVAL;
-
- if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
- pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
- return -ENOMEM;
- }
- cpumask_copy(cpus_to_visit, cpu_possible_mask);
-
- return cpufreq_register_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-core_initcall(register_cpufreq_notifier);
-
-static void parsing_done_workfn(struct work_struct *work)
-{
- cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-
-#else
-static int __init free_raw_capacity(void)
-{
- kfree(raw_capacity);
-
- return 0;
-}
-core_initcall(free_raw_capacity);
-#endif
+extern bool cap_parsing_failed;
+extern void normalize_cpu_capacity(void);
+extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
/*
* Iterate all CPUs' descriptor in DT and compute the efficiency
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 111742126897..7534bb41ee09 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -36,6 +36,7 @@ config ARM64
select EDAC_SUPPORT
select FRAME_POINTER
select GENERIC_ALLOCATOR
+ select GENERIC_ARCH_TOPOLOGY
select GENERIC_CLOCKEVENTS
select GENERIC_CLOCKEVENTS_BROADCAST
select GENERIC_CPU_AUTOPROBE
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 565dd69888cc..f629f7524d65 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -11,7 +11,6 @@
* for more details.
*/
-#include <linux/acpi.h>
#include <linux/cpu.h>
#include <linux/cpumask.h>
#include <linux/init.h>
@@ -22,226 +21,14 @@
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/cpufreq.h>
#include <asm/cpu.h>
#include <asm/cputype.h>
#include <asm/topology.h>
-static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
-static DEFINE_MUTEX(cpu_scale_mutex);
-
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
-{
- return per_cpu(cpu_scale, cpu);
-}
-
-static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
-{
- per_cpu(cpu_scale, cpu) = capacity;
-}
-
-static ssize_t cpu_capacity_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
-
- return sprintf(buf, "%lu\n",
- arch_scale_cpu_capacity(NULL, cpu->dev.id));
-}
-
-static ssize_t cpu_capacity_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t count)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
- int this_cpu = cpu->dev.id, i;
- unsigned long new_capacity;
- ssize_t ret;
-
- if (count) {
- ret = kstrtoul(buf, 0, &new_capacity);
- if (ret)
- return ret;
- if (new_capacity > SCHED_CAPACITY_SCALE)
- return -EINVAL;
-
- mutex_lock(&cpu_scale_mutex);
- for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
- set_capacity_scale(i, new_capacity);
- mutex_unlock(&cpu_scale_mutex);
- }
-
- return count;
-}
-
-static DEVICE_ATTR_RW(cpu_capacity);
-
-static int register_cpu_capacity_sysctl(void)
-{
- int i;
- struct device *cpu;
-
- for_each_possible_cpu(i) {
- cpu = get_cpu_device(i);
- if (!cpu) {
- pr_err("%s: too early to get CPU%d device!\n",
- __func__, i);
- continue;
- }
- device_create_file(cpu, &dev_attr_cpu_capacity);
- }
-
- return 0;
-}
-subsys_initcall(register_cpu_capacity_sysctl);
-
-static u32 capacity_scale;
-static u32 *raw_capacity;
-static bool cap_parsing_failed;
-
-static void __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
-{
- int ret;
- u32 cpu_capacity;
-
- if (cap_parsing_failed)
- return;
-
- ret = of_property_read_u32(cpu_node,
- "capacity-dmips-mhz",
- &cpu_capacity);
- if (!ret) {
- if (!raw_capacity) {
- raw_capacity = kcalloc(num_possible_cpus(),
- sizeof(*raw_capacity),
- GFP_KERNEL);
- if (!raw_capacity) {
- pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
- cap_parsing_failed = true;
- return;
- }
- }
- capacity_scale = max(cpu_capacity, capacity_scale);
- raw_capacity[cpu] = cpu_capacity;
- pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
- cpu_node->full_name, raw_capacity[cpu]);
- } else {
- if (raw_capacity) {
- pr_err("cpu_capacity: missing %s raw capacity\n",
- cpu_node->full_name);
- pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
- }
- cap_parsing_failed = true;
- kfree(raw_capacity);
- }
-}
-
-static void normalize_cpu_capacity(void)
-{
- u64 capacity;
- int cpu;
-
- if (!raw_capacity || cap_parsing_failed)
- return;
-
- pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
- mutex_lock(&cpu_scale_mutex);
- for_each_possible_cpu(cpu) {
- pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
- cpu, raw_capacity[cpu]);
- capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
- / capacity_scale;
- set_capacity_scale(cpu, capacity);
- pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
- cpu, arch_scale_cpu_capacity(NULL, cpu));
- }
- mutex_unlock(&cpu_scale_mutex);
-}
-
-#ifdef CONFIG_CPU_FREQ
-static cpumask_var_t cpus_to_visit;
-static bool cap_parsing_done;
-static void parsing_done_workfn(struct work_struct *work);
-static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
-
-static int
-init_cpu_capacity_callback(struct notifier_block *nb,
- unsigned long val,
- void *data)
-{
- struct cpufreq_policy *policy = data;
- int cpu;
-
- if (cap_parsing_failed || cap_parsing_done)
- return 0;
-
- switch (val) {
- case CPUFREQ_NOTIFY:
- pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
- cpumask_pr_args(policy->related_cpus),
- cpumask_pr_args(cpus_to_visit));
- cpumask_andnot(cpus_to_visit,
- cpus_to_visit,
- policy->related_cpus);
- for_each_cpu(cpu, policy->related_cpus) {
- raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
- policy->cpuinfo.max_freq / 1000UL;
- capacity_scale = max(raw_capacity[cpu], capacity_scale);
- }
- if (cpumask_empty(cpus_to_visit)) {
- normalize_cpu_capacity();
- kfree(raw_capacity);
- pr_debug("cpu_capacity: parsing done\n");
- cap_parsing_done = true;
- schedule_work(&parsing_done_work);
- }
- }
- return 0;
-}
-
-static struct notifier_block init_cpu_capacity_notifier = {
- .notifier_call = init_cpu_capacity_callback,
-};
-
-static int __init register_cpufreq_notifier(void)
-{
- /*
- * on ACPI-based systems we need to use the default cpu capacity
- * until we have the necessary code to parse the cpu capacity, so
- * skip registering cpufreq notifier.
- */
- if (!acpi_disabled || cap_parsing_failed)
- return -EINVAL;
-
- if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
- pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
- return -ENOMEM;
- }
- cpumask_copy(cpus_to_visit, cpu_possible_mask);
-
- return cpufreq_register_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-core_initcall(register_cpufreq_notifier);
-
-static void parsing_done_workfn(struct work_struct *work)
-{
- cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-
-#else
-static int __init free_raw_capacity(void)
-{
- kfree(raw_capacity);
-
- return 0;
-}
-core_initcall(free_raw_capacity);
-#endif
+extern bool cap_parsing_failed;
+extern void normalize_cpu_capacity(void);
+extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
static int __init get_cpu_for_node(struct device_node *node)
{
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index d718ae4b907a..307ea31187dd 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -339,4 +339,12 @@ config CMA_ALIGNMENT
endif
+config GENERIC_ARCH_TOPOLOGY
+ bool
+ help
+ Enable support for architectures common topology code: e.g., parsing
+ CPU capacity information from DT, usage of such information for
+ appropriate scaling, sysfs interface for changing capacity values at
+ runtime.
+
endmenu
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index f2816f6ff76a..397e5c344e6a 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_SOC_BUS) += soc.o
obj-$(CONFIG_PINCTRL) += pinctrl.o
obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
+obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
obj-y += test/
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
new file mode 100644
index 000000000000..c1dd430adad2
--- /dev/null
+++ b/drivers/base/arch_topology.c
@@ -0,0 +1,237 @@
+/*
+ * driver/base/arch_topology.c - Arch specific cpu topology information
+ *
+ * Copyright (C) 2016, ARM Ltd.
+ * Written by: Juri Lelli, ARM Ltd.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/acpi.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/topology.h>
+
+static DEFINE_MUTEX(cpu_scale_mutex);
+static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
+
+unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+{
+ return per_cpu(cpu_scale, cpu);
+}
+
+void set_capacity_scale(unsigned int cpu, unsigned long capacity)
+{
+ per_cpu(cpu_scale, cpu) = capacity;
+}
+
+static ssize_t cpu_capacity_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+
+ return sprintf(buf, "%lu\n",
+ arch_scale_cpu_capacity(NULL, cpu->dev.id));
+}
+
+static ssize_t cpu_capacity_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+ int this_cpu = cpu->dev.id, i;
+ unsigned long new_capacity;
+ ssize_t ret;
+
+ if (count) {
+ ret = kstrtoul(buf, 0, &new_capacity);
+ if (ret)
+ return ret;
+ if (new_capacity > SCHED_CAPACITY_SCALE)
+ return -EINVAL;
+
+ mutex_lock(&cpu_scale_mutex);
+ for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+ set_capacity_scale(i, new_capacity);
+ mutex_unlock(&cpu_scale_mutex);
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+ int i;
+ struct device *cpu;
+
+ for_each_possible_cpu(i) {
+ cpu = get_cpu_device(i);
+ if (!cpu) {
+ pr_err("%s: too early to get CPU%d device!\n",
+ __func__, i);
+ continue;
+ }
+ device_create_file(cpu, &dev_attr_cpu_capacity);
+ }
+
+ return 0;
+}
+subsys_initcall(register_cpu_capacity_sysctl);
+
+u32 capacity_scale;
+u32 *raw_capacity;
+bool cap_parsing_failed;
+
+void normalize_cpu_capacity(void)
+{
+ u64 capacity;
+ int cpu;
+
+ if (!raw_capacity || cap_parsing_failed)
+ return;
+
+ pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+ mutex_lock(&cpu_scale_mutex);
+ for_each_possible_cpu(cpu) {
+ pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
+ cpu, raw_capacity[cpu]);
+ capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
+ / capacity_scale;
+ set_capacity_scale(cpu, capacity);
+ pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
+ cpu, arch_scale_cpu_capacity(NULL, cpu));
+ }
+ mutex_unlock(&cpu_scale_mutex);
+}
+
+int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+{
+ int ret = 1;
+ u32 cpu_capacity;
+
+ if (cap_parsing_failed)
+ return !ret;
+
+ ret = of_property_read_u32(cpu_node,
+ "capacity-dmips-mhz",
+ &cpu_capacity);
+ if (!ret) {
+ if (!raw_capacity) {
+ raw_capacity = kcalloc(num_possible_cpus(),
+ sizeof(*raw_capacity),
+ GFP_KERNEL);
+ if (!raw_capacity) {
+ pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
+ cap_parsing_failed = true;
+ return ret;
+ }
+ }
+ capacity_scale = max(cpu_capacity, capacity_scale);
+ raw_capacity[cpu] = cpu_capacity;
+ pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
+ cpu_node->full_name, raw_capacity[cpu]);
+ } else {
+ if (raw_capacity) {
+ pr_err("cpu_capacity: missing %s raw capacity\n",
+ cpu_node->full_name);
+ pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
+ }
+ cap_parsing_failed = true;
+ kfree(raw_capacity);
+ }
+
+ return !ret;
+}
+
+#ifdef CONFIG_CPU_FREQ
+static cpumask_var_t cpus_to_visit;
+static bool cap_parsing_done;
+static void parsing_done_workfn(struct work_struct *work);
+static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
+
+static int
+init_cpu_capacity_callback(struct notifier_block *nb,
+ unsigned long val,
+ void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu;
+
+ if (cap_parsing_failed || cap_parsing_done)
+ return 0;
+
+ switch (val) {
+ case CPUFREQ_NOTIFY:
+ pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
+ cpumask_pr_args(policy->related_cpus),
+ cpumask_pr_args(cpus_to_visit));
+ cpumask_andnot(cpus_to_visit,
+ cpus_to_visit,
+ policy->related_cpus);
+ for_each_cpu(cpu, policy->related_cpus) {
+ raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+ policy->cpuinfo.max_freq / 1000UL;
+ capacity_scale = max(raw_capacity[cpu], capacity_scale);
+ }
+ if (cpumask_empty(cpus_to_visit)) {
+ normalize_cpu_capacity();
+ kfree(raw_capacity);
+ pr_debug("cpu_capacity: parsing done\n");
+ cap_parsing_done = true;
+ schedule_work(&parsing_done_work);
+ }
+ }
+ return 0;
+}
+
+static struct notifier_block init_cpu_capacity_notifier = {
+ .notifier_call = init_cpu_capacity_callback,
+};
+
+static int __init register_cpufreq_notifier(void)
+{
+ /*
+ * on ACPI-based systems we need to use the default cpu capacity
+ * until we have the necessary code to parse the cpu capacity, so
+ * skip registering cpufreq notifier.
+ */
+ if (!acpi_disabled || cap_parsing_failed)
+ return -EINVAL;
+
+ if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
+ pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
+ return -ENOMEM;
+ }
+
+ cpumask_copy(cpus_to_visit, cpu_possible_mask);
+
+ return cpufreq_register_notifier(&init_cpu_capacity_notifier,
+ CPUFREQ_POLICY_NOTIFIER);
+}
+core_initcall(register_cpufreq_notifier);
+
+static void parsing_done_workfn(struct work_struct *work)
+{
+ cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
+ CPUFREQ_POLICY_NOTIFIER);
+}
+
+#else
+static int __init free_raw_capacity(void)
+{
+ kfree(raw_capacity);
+
+ return 0;
+}
+core_initcall(free_raw_capacity);
+#endif
--
2.10.0
[toc] | [next] | [standalone]
| From | Greg KH <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2017-02-10 15:40 +0100 |
| Subject | Re: [PATCH v2 6/9] arm, arm64: factorize common cpu capacity default code |
| Message-ID | <t9kwF-4AQ-5@gated-at.bofh.it> |
| In reply to | #1577413 |
On Thu, Feb 09, 2017 at 09:25:22AM +0000, Juri Lelli wrote:
> arm and arm64 share lot of code relative to parsing CPU capacity
> information from DT, using that information for appropriate scaling and
> exposing a sysfs interface for chaging such values at runtime.
>
> Factorize such code in a common place (driver/base/arch_topology.c) in
> preparation for further additions.
>
> Suggested-by: Will Deacon <will.deacon@arm.com>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
>
> Changes from v1:
> - keep the original GPLv2 header
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/kernel/topology.c | 213 ++------------------------------------
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/topology.c | 219 +--------------------------------------
> drivers/base/Kconfig | 8 ++
> drivers/base/Makefile | 1 +
> drivers/base/arch_topology.c | 237 +++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 257 insertions(+), 423 deletions(-)
> create mode 100644 drivers/base/arch_topology.c
Ah, so you want _me_ to maintain this, ok, I better review it...
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -339,4 +339,12 @@ config CMA_ALIGNMENT
>
> endif
>
> +config GENERIC_ARCH_TOPOLOGY
> + bool
> + help
> + Enable support for architectures common topology code: e.g., parsing
> + CPU capacity information from DT, usage of such information for
> + appropriate scaling, sysfs interface for changing capacity values at
> + runtime.
Mix of spaces and tabs :(
> +
> endmenu
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index f2816f6ff76a..397e5c344e6a 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_SOC_BUS) += soc.o
> obj-$(CONFIG_PINCTRL) += pinctrl.o
> obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
> obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
> +obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
>
> obj-y += test/
>
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> new file mode 100644
> index 000000000000..c1dd430adad2
> --- /dev/null
> +++ b/drivers/base/arch_topology.c
> @@ -0,0 +1,237 @@
> +/*
> + * driver/base/arch_topology.c - Arch specific cpu topology information
No need to keep the filename in the file, you know what it is called :)
> + *
> + * Copyright (C) 2016, ARM Ltd.
> + * Written by: Juri Lelli, ARM Ltd.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
So, v2 only? Please be specific. Even better yet, use a SPDX header if
you want to, those are always nice.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/topology.h>
> +
> +static DEFINE_MUTEX(cpu_scale_mutex);
> +static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
> +
> +unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
Why do you have sd here? You never use it:
> +{
> + return per_cpu(cpu_scale, cpu);
See? What am I missing?
> +}
> +
> +void set_capacity_scale(unsigned int cpu, unsigned long capacity)
> +{
> + per_cpu(cpu_scale, cpu) = capacity;
> +}
> +
> +static ssize_t cpu_capacity_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct cpu *cpu = container_of(dev, struct cpu, dev);
> +
> + return sprintf(buf, "%lu\n",
> + arch_scale_cpu_capacity(NULL, cpu->dev.id));
> +}
> +
> +static ssize_t cpu_capacity_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct cpu *cpu = container_of(dev, struct cpu, dev);
> + int this_cpu = cpu->dev.id, i;
new line for:
int i;
please.
> + unsigned long new_capacity;
> + ssize_t ret;
> +
> + if (count) {
if (!count)
return 0;
then you can get on with the rest of the logic. Don't indent if you
don't have to.
> + ret = kstrtoul(buf, 0, &new_capacity);
> + if (ret)
> + return ret;
> + if (new_capacity > SCHED_CAPACITY_SCALE)
> + return -EINVAL;
> +
> + mutex_lock(&cpu_scale_mutex);
> + for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
> + set_capacity_scale(i, new_capacity);
> + mutex_unlock(&cpu_scale_mutex);
> + }
> +
> + return count;
> +}
No documentation for these sysfs file? Not good :(
> +
> +static DEVICE_ATTR_RW(cpu_capacity);
> +
> +static int register_cpu_capacity_sysctl(void)
> +{
> + int i;
> + struct device *cpu;
> +
> + for_each_possible_cpu(i) {
> + cpu = get_cpu_device(i);
> + if (!cpu) {
> + pr_err("%s: too early to get CPU%d device!\n",
> + __func__, i);
What is this going to help with?
> + continue;
> + }
> + device_create_file(cpu, &dev_attr_cpu_capacity);
You realize you just raced userspace, right? Why do it this way and not
register the files when the CPU device is created/removed?
> + }
> +
> + return 0;
> +}
> +subsys_initcall(register_cpu_capacity_sysctl);
> +
> +u32 capacity_scale;
> +u32 *raw_capacity;
> +bool cap_parsing_failed;
globals? really? That's bold :(
> +
> +void normalize_cpu_capacity(void)
naming is hard, but try to put a good, descriptive, prefix on everything
you are exporting in the same file, the same prefix.
cpu_capacity_normalize()?
cpu_capacity_register_sysctl()?
and so on.
> +{
> + u64 capacity;
> + int cpu;
> +
> + if (!raw_capacity || cap_parsing_failed)
> + return;
> +
> + pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
> + mutex_lock(&cpu_scale_mutex);
> + for_each_possible_cpu(cpu) {
> + pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
> + cpu, raw_capacity[cpu]);
> + capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
> + / capacity_scale;
> + set_capacity_scale(cpu, capacity);
> + pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
> + cpu, arch_scale_cpu_capacity(NULL, cpu));
> + }
> + mutex_unlock(&cpu_scale_mutex);
> +}
> +
> +int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
cpu_capacity_parse()?
thanks,
greg k-h
[toc] | [prev] | [next] | [standalone]
| From | Juri Lelli <juri.lelli@arm.com> |
|---|---|
| Date | 2017-02-13 16:20 +0100 |
| Subject | Re: [PATCH v2 6/9] arm, arm64: factorize common cpu capacity default code |
| Message-ID | <taqA1-5qu-7@gated-at.bofh.it> |
| In reply to | #1578507 |
Hi Greg,
On 10/02/17 15:28, Greg KH wrote:
> On Thu, Feb 09, 2017 at 09:25:22AM +0000, Juri Lelli wrote:
> > arm and arm64 share lot of code relative to parsing CPU capacity
> > information from DT, using that information for appropriate scaling and
> > exposing a sysfs interface for chaging such values at runtime.
> >
> > Factorize such code in a common place (driver/base/arch_topology.c) in
> > preparation for further additions.
> >
> > Suggested-by: Will Deacon <will.deacon@arm.com>
> > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> > ---
> >
> > Changes from v1:
> > - keep the original GPLv2 header
> > ---
> > arch/arm/Kconfig | 1 +
> > arch/arm/kernel/topology.c | 213 ++------------------------------------
> > arch/arm64/Kconfig | 1 +
> > arch/arm64/kernel/topology.c | 219 +--------------------------------------
> > drivers/base/Kconfig | 8 ++
> > drivers/base/Makefile | 1 +
> > drivers/base/arch_topology.c | 237 +++++++++++++++++++++++++++++++++++++++++++
> > 7 files changed, 257 insertions(+), 423 deletions(-)
> > create mode 100644 drivers/base/arch_topology.c
>
> Ah, so you want _me_ to maintain this, ok, I better review it...
>
This has been suggested as a possible way to stop replicating code between arm
and arm64 (and possibly other archs in the future). Are you in principle OK
with it?
Thanks a lot for your comments, please find my answers below.
> > --- a/drivers/base/Kconfig
> > +++ b/drivers/base/Kconfig
> > @@ -339,4 +339,12 @@ config CMA_ALIGNMENT
> >
> > endif
> >
> > +config GENERIC_ARCH_TOPOLOGY
> > + bool
> > + help
> > + Enable support for architectures common topology code: e.g., parsing
> > + CPU capacity information from DT, usage of such information for
> > + appropriate scaling, sysfs interface for changing capacity values at
> > + runtime.
>
> Mix of spaces and tabs :(
>
Argh. :(
> > +
> > endmenu
> > diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> > index f2816f6ff76a..397e5c344e6a 100644
> > --- a/drivers/base/Makefile
> > +++ b/drivers/base/Makefile
> > @@ -23,6 +23,7 @@ obj-$(CONFIG_SOC_BUS) += soc.o
> > obj-$(CONFIG_PINCTRL) += pinctrl.o
> > obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
> > obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
> > +obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> >
> > obj-y += test/
> >
> > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > new file mode 100644
> > index 000000000000..c1dd430adad2
> > --- /dev/null
> > +++ b/drivers/base/arch_topology.c
> > @@ -0,0 +1,237 @@
> > +/*
> > + * driver/base/arch_topology.c - Arch specific cpu topology information
>
> No need to keep the filename in the file, you know what it is called :)
>
OK, removed.
> > + *
> > + * Copyright (C) 2016, ARM Ltd.
> > + * Written by: Juri Lelli, ARM Ltd.
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License. See the file "COPYING" in the main directory of this archive
> > + * for more details.
>
> So, v2 only? Please be specific. Even better yet, use a SPDX header if
> you want to, those are always nice.
>
Yes, v2 only.
* for more details.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
Would do, right?
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/cpu.h>
> > +#include <linux/cpufreq.h>
> > +#include <linux/device.h>
> > +#include <linux/of.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/topology.h>
> > +
> > +static DEFINE_MUTEX(cpu_scale_mutex);
> > +static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
> > +
> > +unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
>
> Why do you have sd here? You never use it:
>
> > +{
> > + return per_cpu(cpu_scale, cpu);
>
> See? What am I missing?
>
This is how this function is defined in kernel/sched/sched.h:
#ifndef arch_scale_cpu_capacity
static __always_inline
unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
{
if (sd && (sd->flags & SD_SHARE_CPUCAPACITY) && (sd->span_weight > 1))
return sd->smt_gain / sd->span_weight;
return SCHED_CAPACITY_SCALE;
}
#endif
and in this case the sd argument is used: there is a call site in fair.c
that passes a non NULL sd, updated_cpu_capacity().
A following set of patches will re-define the function so that the
drivers one gets used by the kernel (only arm and arm64 will currently
want this), with something like this in arch code
#define arch_scale_cpu_capacity atd_scale_cpu_capacity
Please note that last patch of this set renames this function atd_scale_
cpu_capacity, to (hopefully) make this approach more clear.
Does it make more sense to you?
> > +}
> > +
> > +void set_capacity_scale(unsigned int cpu, unsigned long capacity)
> > +{
> > + per_cpu(cpu_scale, cpu) = capacity;
> > +}
> > +
> > +static ssize_t cpu_capacity_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > +
> > + return sprintf(buf, "%lu\n",
> > + arch_scale_cpu_capacity(NULL, cpu->dev.id));
> > +}
> > +
> > +static ssize_t cpu_capacity_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf,
> > + size_t count)
> > +{
> > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > + int this_cpu = cpu->dev.id, i;
>
> new line for:
> int i;
> please.
>
Sure.
> > + unsigned long new_capacity;
> > + ssize_t ret;
> > +
> > + if (count) {
>
> if (!count)
> return 0;
>
> then you can get on with the rest of the logic. Don't indent if you
> don't have to.
>
Right.
> > + ret = kstrtoul(buf, 0, &new_capacity);
> > + if (ret)
> > + return ret;
> > + if (new_capacity > SCHED_CAPACITY_SCALE)
> > + return -EINVAL;
> > +
> > + mutex_lock(&cpu_scale_mutex);
> > + for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
> > + set_capacity_scale(i, new_capacity);
> > + mutex_unlock(&cpu_scale_mutex);
> > + }
> > +
> > + return count;
> > +}
>
> No documentation for these sysfs file? Not good :(
>
Patch 2/9 introduces some documentation. There is already more in
Documentation/devicetree/bindings/arm/cpu-capacity.txt.
Do you think I should improve further?
> > +
> > +static DEVICE_ATTR_RW(cpu_capacity);
> > +
> > +static int register_cpu_capacity_sysctl(void)
> > +{
> > + int i;
> > + struct device *cpu;
> > +
> > + for_each_possible_cpu(i) {
> > + cpu = get_cpu_device(i);
> > + if (!cpu) {
> > + pr_err("%s: too early to get CPU%d device!\n",
> > + __func__, i);
>
> What is this going to help with?
>
Not much I guess, I can remove it.
> > + continue;
> > + }
> > + device_create_file(cpu, &dev_attr_cpu_capacity);
>
> You realize you just raced userspace, right? Why do it this way and not
> register the files when the CPU device is created/removed?
>
Humm, my intention for doing it this way is that I'd like to make all
the code dealing with cpu_capacity confined in a single place (this
file), without the need to modify other files.
> > + }
> > +
> > + return 0;
> > +}
> > +subsys_initcall(register_cpu_capacity_sysctl);
AFAIU, for both arm and arm64 CPU device is registered with a
subsys_initcall(topology_init), so I'm doing the same. Other archs seem to do
similar things. Could you explain a little more why this is a problem?
> > +
> > +u32 capacity_scale;
> > +u32 *raw_capacity;
> > +bool cap_parsing_failed;
>
> globals? really? That's bold :(
>
Yeah, ugly. However, patch 7/9 is making cap_parsing_failed static. The other
two can be made static already, I should have done that in the first place. :(
BTW, with this set I'm trying to incrementally fix things (after moving code in
the new place), does it look reasonable to you or would you prefer to squash
intermediate steps?
> > +
> > +void normalize_cpu_capacity(void)
>
> naming is hard, but try to put a good, descriptive, prefix on everything
> you are exporting in the same file, the same prefix.
>
> cpu_capacity_normalize()?
> cpu_capacity_register_sysctl()?
>
> and so on.
>
> > +{
> > + u64 capacity;
> > + int cpu;
> > +
> > + if (!raw_capacity || cap_parsing_failed)
> > + return;
> > +
> > + pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
> > + mutex_lock(&cpu_scale_mutex);
> > + for_each_possible_cpu(cpu) {
> > + pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
> > + cpu, raw_capacity[cpu]);
> > + capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
> > + / capacity_scale;
> > + set_capacity_scale(cpu, capacity);
> > + pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
> > + cpu, arch_scale_cpu_capacity(NULL, cpu));
> > + }
> > + mutex_unlock(&cpu_scale_mutex);
> > +}
> > +
> > +int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
>
> cpu_capacity_parse()?
>
OK, I'll try to fix the naming as you suggest. Thanks!
Best,
- Juri
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web