On Tuesday 07 August 2012 04:24:48 Palmer Cox wrote: > The pkgs member of cpupower_topology is being used as the number of > cpu packages. As the comment in get_cpu_topology notes, the package ids > are not guaranteed to be contiguous. So, simply setting pkgs to the value > of the highest physical_package_id doesn't actually provide a count of > the number of cpu packages. Instead, calculate pkgs by setting it to > the number of distinct physical_packge_id values which is pretty easy > to do after the core_info structs are sorted. Calculating pkgs this > way also has the nice benefit of getting rid of a sign comparison warning > that GCC 4.6 was reporting. > --- > tools/power/cpupower/utils/helpers/topology.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/tools/power/cpupower/utils/helpers/topology.c > b/tools/power/cpupower/utils/helpers/topology.c > index 4e2b583..fd3cc4d 100644 > --- a/tools/power/cpupower/utils/helpers/topology.c > +++ b/tools/power/cpupower/utils/helpers/topology.c > @@ -64,7 +64,7 @@ static int __compare(const void *t1, const void *t2) > */ > int get_cpu_topology(struct cpupower_topology *cpu_top) > { > - int cpu, cpus = sysconf(_SC_NPROCESSORS_CONF); > + int cpu, last_pkg, cpus = sysconf(_SC_NPROCESSORS_CONF); > > cpu_top->core_info = malloc(sizeof(struct cpuid_core_info) * cpus); > if (cpu_top->core_info == NULL) > @@ -78,20 +78,28 @@ int get_cpu_topology(struct cpupower_topology *cpu_top) > "physical_package_id", > &(cpu_top->core_info[cpu].pkg)) < 0) > return -1; > - if ((int)cpu_top->core_info[cpu].pkg != -1 && > - cpu_top->core_info[cpu].pkg > cpu_top->pkgs) > - cpu_top->pkgs = cpu_top->core_info[cpu].pkg; > if(sysfs_topology_read_file( > cpu, > "core_id", > &(cpu_top->core_info[cpu].core)) < 0) > return -1; > } > - cpu_top->pkgs++; > > qsort(cpu_top->core_info, cpus, sizeof(struct cpuid_core_info), > __compare); > > + /* Count the number of distinct pkgs values. This works > + becuase the primary sort of of the core_info structs was just becuase ... of of ... struct instead of structs
Otherwise the first 4 patches look like rather nice and straight forward cleanups/fixes. Feel free to add a Reviewed-by: Thomas Renninger <tr...@suse.de> Let me have a closer look at patch 5 and 6. I had problems getting rid of the compiler warning, looks like you found an easy way to clean this up. I will be back and have time for this in the beginning of next week. On which platforms (topology) did you test this? Thomas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/