Hi Greg,
You weren't included in the first [1] version of this series, as it was
targeting arch/arm*/ directories then.
Here are some cleanups for the arch_topology core.
Tested on ARM64 Hikey board by setting following in cpu nodes in DT:
capacity-dmips-mhz = <1000>;
V2->V3:
- topology_parse_cpu_capacity() returns bool now (Juri).
- "Get rid of cap_parsing_done" is the last patch now and its
implementation is change a bit (Juri).
V1->V2:
- based over linux-next/master (to get Juri's recent changes)
- More cleanups included. V1 only had the first patch.
- Rename of cap_parsing_failed isn't required anymore (as it is
localized to a single routine now).
--
viresh
[1] https://marc.info/?l=linux-arm-kernel&m=149795553024005
Viresh Kumar (5):
arch_topology: Don't break lines unnecessarily
arch_topology: Convert switch block to if block
arch_topology: Change return type of topology_parse_cpu_capacity() to
bool
arch_topology: Localize cap_parsing_failed to
topology_parse_cpu_capacity()
arch_topology: Get rid of cap_parsing_done
drivers/base/arch_topology.c | 78 +++++++++++++++++++++----------------------
include/linux/arch_topology.h | 4 ++-
2 files changed, 42 insertions(+), 40 deletions(-)
--
2.13.0.71.gd7076ec9c9cb
There is no need of line break at few places, avoid them.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/base/arch_topology.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index d1c33a85059e..0ad79b5cd56d 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -41,8 +41,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
{
struct cpu *cpu = container_of(dev, struct cpu, dev);
- return sprintf(buf, "%lu\n",
- topology_get_cpu_scale(NULL, cpu->dev.id));
+ return sprintf(buf, "%lu\n", topology_get_cpu_scale(NULL, cpu->dev.id));
}
static ssize_t cpu_capacity_store(struct device *dev,
@@ -128,8 +127,7 @@ int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
if (cap_parsing_failed)
return !ret;
- ret = of_property_read_u32(cpu_node,
- "capacity-dmips-mhz",
+ ret = of_property_read_u32(cpu_node, "capacity-dmips-mhz",
&cpu_capacity);
if (!ret) {
if (!raw_capacity) {
@@ -181,8 +179,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
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,
+ cpumask_andnot(cpus_to_visit, cpus_to_visit,
policy->related_cpus);
for_each_cpu(cpu, policy->related_cpus) {
raw_capacity[cpu] = topology_get_cpu_scale(NULL, cpu) *
--
2.13.0.71.gd7076ec9c9cb
We only need to take care of one case here (CPUFREQ_NOTIFY) and there is
no need to add an extra level of indentation to the case specific code
by using a switch block. Use an if block instead.
Also add some blank lines to make the code look better.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/base/arch_topology.c | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 0ad79b5cd56d..a3cd7c869c3e 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -174,26 +174,29 @@ init_cpu_capacity_callback(struct notifier_block *nb,
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] = topology_get_cpu_scale(NULL, cpu) *
- policy->cpuinfo.max_freq / 1000UL;
- capacity_scale = max(raw_capacity[cpu], capacity_scale);
- }
- if (cpumask_empty(cpus_to_visit)) {
- topology_normalize_cpu_scale();
- kfree(raw_capacity);
- pr_debug("cpu_capacity: parsing done\n");
- cap_parsing_done = true;
- schedule_work(&parsing_done_work);
- }
+ if (val != CPUFREQ_NOTIFY)
+ return 0;
+
+ 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] = topology_get_cpu_scale(NULL, cpu) *
+ policy->cpuinfo.max_freq / 1000UL;
+ capacity_scale = max(raw_capacity[cpu], capacity_scale);
}
+
+ if (cpumask_empty(cpus_to_visit)) {
+ topology_normalize_cpu_scale();
+ kfree(raw_capacity);
+ pr_debug("cpu_capacity: parsing done\n");
+ cap_parsing_done = true;
+ schedule_work(&parsing_done_work);
+ }
+
return 0;
}
--
2.13.0.71.gd7076ec9c9cb
There is no need to check for cap_parsing_done flag anymore as
!raw_capacity flag alone is enough for us. Remove the (now) useless flag
cap_parsing_done.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/base/arch_topology.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 9e4d2107f4fa..74043ead9da1 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -167,7 +167,6 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
#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);
@@ -179,7 +178,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
struct cpufreq_policy *policy = data;
int cpu;
- if (!raw_capacity || cap_parsing_done)
+ if (!raw_capacity)
return 0;
if (val != CPUFREQ_NOTIFY)
@@ -201,7 +200,6 @@ init_cpu_capacity_callback(struct notifier_block *nb,
topology_normalize_cpu_scale();
free_raw_capacity();
pr_debug("cpu_capacity: parsing done\n");
- cap_parsing_done = true;
schedule_work(&parsing_done_work);
}
--
2.13.0.71.gd7076ec9c9cb
topology_parse_cpu_capacity() returns 1 on success and 0 on errors. Make
it return bool instead of int as that suits the purpose better.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/base/arch_topology.c | 8 ++++----
include/linux/arch_topology.h | 4 +++-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index a3cd7c869c3e..5728e2fbb765 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -119,13 +119,13 @@ void topology_normalize_cpu_scale(void)
mutex_unlock(&cpu_scale_mutex);
}
-int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
{
- int ret = 1;
+ int ret;
u32 cpu_capacity;
if (cap_parsing_failed)
- return !ret;
+ return false;
ret = of_property_read_u32(cpu_node, "capacity-dmips-mhz",
&cpu_capacity);
@@ -137,7 +137,7 @@ int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
if (!raw_capacity) {
pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
cap_parsing_failed = true;
- return 0;
+ return false;
}
}
capacity_scale = max(cpu_capacity, capacity_scale);
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 9af3c174c03a..716ce587247e 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,10 +4,12 @@
#ifndef _LINUX_ARCH_TOPOLOGY_H_
#define _LINUX_ARCH_TOPOLOGY_H_
+#include <linux/types.h>
+
void topology_normalize_cpu_scale(void);
struct device_node;
-int topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+bool topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
struct sched_domain;
unsigned long topology_get_cpu_scale(struct sched_domain *sd, int cpu);
--
2.13.0.71.gd7076ec9c9cb
cap_parsing_failed is only required in topology_parse_cpu_capacity() to
know if we have already tried to allocate raw_capacity and failed, or if
at least one of the cpu_node didn't had the required
"capacity-dmips-mhz" property.
All other users can use raw_capacity instead of cap_parsing_failed.
Make sure we set raw_capacity to NULL after we free it.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/base/arch_topology.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 5728e2fbb765..9e4d2107f4fa 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -95,14 +95,21 @@ subsys_initcall(register_cpu_capacity_sysctl);
static u32 capacity_scale;
static u32 *raw_capacity;
-static bool cap_parsing_failed;
+
+static int __init free_raw_capacity(void)
+{
+ kfree(raw_capacity);
+ raw_capacity = NULL;
+
+ return 0;
+}
void topology_normalize_cpu_scale(void)
{
u64 capacity;
int cpu;
- if (!raw_capacity || cap_parsing_failed)
+ if (!raw_capacity)
return;
pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
@@ -121,6 +128,7 @@ void topology_normalize_cpu_scale(void)
bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
{
+ static bool cap_parsing_failed;
int ret;
u32 cpu_capacity;
@@ -151,7 +159,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
}
cap_parsing_failed = true;
- kfree(raw_capacity);
+ free_raw_capacity();
}
return !ret;
@@ -171,7 +179,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
struct cpufreq_policy *policy = data;
int cpu;
- if (cap_parsing_failed || cap_parsing_done)
+ if (!raw_capacity || cap_parsing_done)
return 0;
if (val != CPUFREQ_NOTIFY)
@@ -191,7 +199,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
if (cpumask_empty(cpus_to_visit)) {
topology_normalize_cpu_scale();
- kfree(raw_capacity);
+ free_raw_capacity();
pr_debug("cpu_capacity: parsing done\n");
cap_parsing_done = true;
schedule_work(&parsing_done_work);
@@ -233,11 +241,5 @@ static void parsing_done_workfn(struct work_struct *work)
}
#else
-static int __init free_raw_capacity(void)
-{
- kfree(raw_capacity);
-
- return 0;
-}
core_initcall(free_raw_capacity);
#endif
--
2.13.0.71.gd7076ec9c9cb
Hi Viresh,
On 23/06/17 14:55, Viresh Kumar wrote:
> Hi Greg,
>
> You weren't included in the first [1] version of this series, as it was
> targeting arch/arm*/ directories then.
>
> Here are some cleanups for the arch_topology core.
>
> Tested on ARM64 Hikey board by setting following in cpu nodes in DT:
> capacity-dmips-mhz = <1000>;
>
The set looks ok to me. Also tested on JunoR2 and TC2.
You can add my
Reviewed-and-tested-by: Juri Lelli <[email protected]>
Best,
- Juri