lkml.org 
[lkml]   [2012]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 6/6] cpupower tools: Fix warning and a bug with the cpu package count
    Date
    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 <trenn@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


    \
     
     \ /
      Last update: 2012-08-09 12:43    [W:0.029 / U:0.068 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site