Re: [PATCH v6 1/2] powerpc: Detect the presence of big-cores via "ibm,thread-groups"
From: Srikar Dronamraju <hidden>
Date: 2018-08-09 13:27:57
Also in:
lkml
* Gautham R. Shenoy [off-list ref] [2018-08-09 11:02:07]:
int threads_per_core, threads_per_subcore, threads_shift; +bool has_big_cores; cpumask_t threads_core_mask; EXPORT_SYMBOL_GPL(threads_per_core); EXPORT_SYMBOL_GPL(threads_per_subcore); EXPORT_SYMBOL_GPL(threads_shift); +EXPORT_SYMBOL_GPL(has_big_cores);
Why do we need EXPORT_SYMBOL_GPL?
EXPORT_SYMBOL_GPL(threads_core_mask);
+ *
+ * Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ */
+int parse_thread_groups(struct device_node *dn,
+ struct thread_groups *tg)
+{
+ unsigned int nr_groups, threads_per_group, property;
+ int i;
+ u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
+ u32 *thread_list;
+ size_t total_threads;
+ int ret;
+
+ ret = of_property_read_u32_array(dn, "ibm,thread-groups",
+ thread_group_array, 3);
+
+ if (ret)
+ goto out_err;
+
+ property = thread_group_array[0];
+ nr_groups = thread_group_array[1];
+ threads_per_group = thread_group_array[2];
+ total_threads = nr_groups * threads_per_group;
+Shouldnt we check for property and nr_groups If the property is not 1 and nr_groups < 1, we should error out No point in calling a of_property read if property is not right. Nit: Cant we directly assign to tg->property, and hence avoid local variables, property, nr_groups and threads_per_group?
+ ret = of_property_read_u32_array(dn, "ibm,thread-groups",
+ thread_group_array,
+ 3 + total_threads);
+
+static inline bool dt_has_big_core(struct device_node *dn,
+ struct thread_groups *tg)
+{
+ if (parse_thread_groups(dn, tg))
+ return false;
+
+ if (tg->property != 1)
+ return false;
+
+ if (tg->nr_groups < 1)
+ return false;Can avoid these check if we can check in parse_thread_groups.
quoted hunk ↗ jump to hunk
/** * setup_cpu_maps - initialize the following cpu maps: * cpu_possible_mask@@ -457,6 +605,7 @@ void __init smp_setup_cpu_maps(void) int cpu = 0; int nthreads = 1;diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c index 755dc98..f5717de 100644 --- a/arch/powerpc/kernel/sysfs.c +++ b/arch/powerpc/kernel/sysfs.c@@ -18,6 +18,7 @@ #include <asm/smp.h> #include <asm/pmc.h> #include <asm/firmware.h> +#include <asm/cputhreads.h> #include "cacheinfo.h" #include "setup.h"@@ -1025,6 +1026,33 @@ static ssize_t show_physical_id(struct device *dev, } static DEVICE_ATTR(physical_id, 0444, show_physical_id, NULL); +static ssize_t show_small_core_siblings(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct cpu *cpu = container_of(dev, struct cpu, dev); + struct device_node *dn = of_get_cpu_node(cpu->dev.id, NULL); + struct thread_groups tg; + int i, j; + ssize_t ret = 0; +
Here we need to check for validity of dn and error out accordingly.
+ if (parse_thread_groups(dn, &tg)) + return -ENODATA;
Did we miss a of_node_put(dn)?
+ + i = get_cpu_thread_group_start(cpu->dev.id, &tg); + + if (i == -1) + return -ENODATA; + + for (j = 0; j < tg.threads_per_group - 1; j++) + ret += sprintf(buf + ret, "%d,", tg.thread_list[i + j]);
Here, we are making the assumption that group_start will always be the first thread in the thread_group. However we didnt make the same assumption in get_cpu_thread_group_start.