Re: [Powertop] segfault on Sheevaplug (ARM Kirkwood)

[Sergey Senozhatsky <sergey.senozhatsky at gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5918 bytes --]

On (05/20/12 04:01), Rui DaCosta wrote:
>    Thanks,
>    this has gotten past the issue. I now get:
>    PowerTOP v2.0 needs the kernel to support the 'perf' subsystem
>    as well as support for trace points in the kernel:
> 
>    CONFIG_PERF_EVENTS=y
>    CONFIG_PERF_COUNTERS=y
>    CONFIG_TRACEPOINTS=y
>    CONFIG_TRACING=y
> 
>    all these except CONFIG_PERF_COUNTERS are already set, so i'll need to see
>    if i can get a kernel built with that option on.
>

>
>    Will this patch make it into trunk?
>

Well, it depends. The patch itself is quite innocent -- assuming default
number of processors being 1 instead of -1 will not do any harm. Of course,
such default value could be considered as debugging friendly, yet segfault
is still no good.

We'll see what project owners think about that.

	-ss

>    Many thanks.
> 
>    ──────────────────────────────────────────────────────────────────────────
> 
>    From: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
>    To: Rui DaCosta <ruidc(a)yahoo.com>
>    Cc: Arjan van de Ven <arjan(a)linux.intel.com>; Chris Ferron
>    <chris.e.ferron(a)linux.intel.com>; powertop(a)lists.01.org
>    Sent: Sunday, 20 May 2012, 11:34
>    Subject: Re: [Powertop] segfault on Sheevaplug (ARM Kirkwood)
>    On (05/20/12 02:19), Rui DaCosta wrote:
>    > Sure and thanks,
>    > (v1.13 worked fine btw)
>    > Processor    : Feroceon 88FR131 rev 1 (v5l)
>    > BogoMIPS    : 1191.11
>    > Features    : swp half thumb fastmult edsp
>    > CPU implementer    : 0x56
>    > CPU architecture: 5TE
>    > CPU variant    : 0x2
>    > CPU part    : 0x131
>    > CPU revision    : 1
>    >
>    > Hardware    : Marvell SheevaPlug Reference Board
>    > Revision    : 0000
>    > Serial        : 0000000000000000
>    >
>    >
> 
>    Thanks,
> 
>    Well, that's the problem. Current cpu info parser doesn't understand your
>    cpuinfo format. It awaits for sane values on special places. For example,
>    word
>    "processor" should be followed by a number, not model name.
> 
>        processor    : 2
>        vendor_id    : GenuineIntel
>        cpu family    : 6
>        model        : 37
>        bogomips    : 4522.66
> 
>    while cpuinfo on your system is totally different.
> 
>    the following is untested patch (I'm a bit skeptical) plus I don't have
>    ARM device for testing.
> 
>    ---
> 
>    src/cpu/cpu.cpp |  26 ++++++++++++++------------
>    1 file changed, 14 insertions(+), 12 deletions(-)
> 
>    diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
>    index 09d4a2d..143e18c 100644
>    --- a/src/cpu/cpu.cpp
>    +++ b/src/cpu/cpu.cpp
>    @@ -225,7 +225,7 @@ void enumerate_cpus(void)
>        ifstream file;
>        char line[1024];
> 
>    -    int number = -1;
>    +    int number = 1;
>        char vendor[128];
>        int family = 0;
>        int model = 0;
>    @@ -236,7 +236,6 @@ void enumerate_cpus(void)
>            return;
> 
>        while (file) {
>    -
>            file.getline(line, sizeof(line));
>            if (strncmp(line, "vendor_id\t",10) == 0) {
>                char *c;
>    @@ -247,42 +246,45 @@ void enumerate_cpus(void)
>                        c++;
>                    strncpy(vendor,c, 127);
>                }
>    -        }
>    -        if (strncmp(line, "processor\t",10) == 0) {
>    +        } else if (strncmp(line, "processor\t",10) == 0) {
>                char *c;
>                c = strchr(line, ':');
>                if (c) {
>                    c++;
>                    number = strtoull(c, NULL, 10);
>                }
>    -        }
>    -        if (strncmp(line, "cpu family\t",11) == 0) {
>    +        } else if (strncmp(line, "Processor\t",10) == 0) {
>    +            char *c;
>    +            c = strchr(line, ':');
>    +            if (c) {
>    +                c++;
>    +                if (*c == ' ')
>    +                    c++;
>    +                strncpy(vendor, c, 127);
>    +            }
>    +        }  else if (strncmp(line, "cpu family\t",11) == 0) {
>                char *c;
>                c = strchr(line, ':');
>                if (c) {
>                    c++;
>                    family = strtoull(c, NULL, 10);
>                }
>    -        }
>    -        if (strncmp(line, "model\t",6) == 0) {
>    +        } else if (strncmp(line, "model\t",6) == 0) {
>                char *c;
>                c = strchr(line, ':');
>                if (c) {
>                    c++;
>                    model = strtoull(c, NULL, 10);
>                }
>    -        }
>    -        if (strncasecmp(line, "bogomips\t", 9) == 0) {
>    +        } else if (strncasecmp(line, "bogomips\t", 9) == 0) {
>                handle_one_cpu(number, vendor, family, model);
>                set_max_cpu(number);
>            }
>        }
> 
>    -
>        file.close();
> 
>        perf_events = new perf_power_bundle();
>    -
>        if (!perf_events->add_event("power:cpu_idle")){
>            perf_events->add_event("power:power_start");
>            perf_events->add_event("power:power_end");