On 05/20/2012 02:57 PM, Sergey Senozhatsky wrote:
> 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.
*In GENERAL*
Well that is an interesting question. I personally will not be spending
any time on arm.
That said, i am also not opposed to somewhat blindly accepting patches
for ARM, especially from trusted and active members.
*As long as ARM work isn't effecting PowerTOP.*

Now if you find a bug, then that is a different story, as long as the
bug is not purely ARM specific it will get attention.

*this instance*
I will pull this patch and take a good look at it, since it looks to me
to be a bug as well. Thanks Sergey


-Chris



>
>     -ss
>
>>    Many thanks.
>>
>>    ──────────────────────────────────────────────────────────────────────────
>>
>>    From: Sergey Senozhatsky<sergey.senozhatsky@gmail.com>
>>    To: Rui DaCosta<ruidc@yahoo.com>
>>    Cc: Arjan van de Ven<arjan@linux.intel.com>; Chris Ferron
>>    <chris.e.ferron@linux.intel.com>; powertop@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");