Thomas Spura wrote:
> 2012/4/21 Németh Márton <nm...@freemail.hu>:
>> diff --git a/main.cpp b/main.cpp
>> index 0e57ee1..67db2eb 100644
>> --- a/main.cpp
>> +++ b/main.cpp
>> @@ -240,6 +240,7 @@ void report(int time, int iterations, char *file)
>>                one_measurement(time);
>>                report_show_tunables();
>>                finish_report_output();
>> +               clear_tuning();
>>        }
>>        /* and wrap up */
>>        learn_parameters(50, 0);
>> @@ -415,6 +416,7 @@ int main(int argc, char **argv)
>>        learn_parameters(500, 0);
>>        save_parameters("saved_parameters.powertop");
>>        end_pci_access();
>> +       clear_tuning();
>>        reset_display();
>>
>>        clear_all_devices();
> 
> These two chunks don't apply here. Did you create the patch against
> currend HEAD commit 1dfdb80d? But look fine, when applying manually.

Yes, I used 1dfdb80d revision as a base. I don't know what could have went 
wrong.

>> diff --git a/tuning/tuning.cpp b/tuning/tuning.cpp
>> index a0c3ffa..6a359ae 100644
>> --- a/tuning/tuning.cpp
>> +++ b/tuning/tuning.cpp
>> @@ -312,3 +312,15 @@ void report_show_tunables(void)
>>                        fprintf(reportout.csv_report,"\n");
>>        }
>>  }
>> +
>> +void clear_tuning()
>> +{
>> +       while (!all_tunables.empty()) {
>> +               delete all_tunables.back();
>> +               all_tunables.pop_back();
>> +       }
>> +       while (!all_untunables.empty()) {
>> +               delete all_untunables.back();
>> +               all_untunables.pop_back();
>> +       }
>> +}
> 
> I'd prefer a for loop like in commit 1dfdb80d. Easier to read and
> should be faster than poping and always check for empty().

Are you speaking about this commit?


commit 1dfdb80d6333960ed310fff7b60439f79d154a84
Author: Chris E Ferron <chris.e.fer...@linux.intel.com>
Date:   Fri Apr 20 13:59:50 2012 -0700

    Added some error handling, code clean-up, comment removal, event name 
adjustments.

diff --git a/cpu/cpu.cpp b/cpu/cpu.cpp
index d95e3b5..756fe5e 100644
--- a/cpu/cpu.cpp
+++ b/cpu/cpu.cpp
@@ -940,13 +940,11 @@ void perf_power_bundle::handle_trace_point(void *trace, 
int cpunr, uint64_t time
        class abstract_cpu *cpu;
        int type;

-       if (event_names.find(type) == event_names.end())
-               return;
-
        rec.data = trace;

        type = pevent_data_type(perf_event::pevent, &rec);
        event = pevent_find_event(perf_event::pevent, type);
+
        if (!event)
                return;

@@ -955,7 +953,6 @@ void perf_power_bundle::handle_trace_point(void *trace, int 
cpunr, uint64_t time
                return;
        }

-       printf("event: %s\n", event->name);
        cpu = all_cpus[cpunr];

 #if 0
@@ -970,18 +967,23 @@ void perf_power_bundle::handle_trace_point(void *trace, 
int cpunr, uint64_t time
        if (strcmp(event->name, "cpu_idle")==0) {

                ret = pevent_get_field_val(NULL, event, "state", &rec, &val, 0);
+                if (ret < 0) {
+                        fprintf(stderr, "cpu_idle event returned no state?\n");
+                        exit(-1);
+                }
+
                if (val == 4294967295)
                        cpu->go_unidle(time);
                else
                        cpu->go_idle(time);
        }

-       if (strcmp(event->name, "power_frequency") ||
-               strcmp(event->name, "cpu_frequency")==0){
+       if (strcmp(event->name, "power_frequency") == 0
+       || strcmp(event->name, "cpu_frequency") == 0){

                ret = pevent_get_field_val(NULL, event, "state", &rec, &val, 0);
                if (ret < 0) {
-                       fprintf(stderr, "no state?\n");
+                       fprintf(stderr, "power or cpu_frequecny event returned 
no state?\n");
                        exit(-1);
                }

diff --git a/process/do_process.cpp b/process/do_process.cpp
index be3463a..019cebc 100644
--- a/process/do_process.cpp
+++ b/process/do_process.cpp
@@ -384,8 +384,10 @@ void perf_process_bundle::handle_trace_point(void *trace, 
int cpu, uint64_t time
                int vec;

                ret = pevent_get_field_val(NULL, event, "vec", &rec, &val, 0);
-               if (ret < 0)
-                       return;
+                if (ret < 0) {
+                        fprintf(stderr, "softirq_entry event returned no 
vector number?\n");
+                        return;
+                }
                vec = (int)val;

                if (vec <= 9)
@@ -419,8 +421,10 @@ void perf_process_bundle::handle_trace_point(void *trace, 
int cpu, uint64_t time
                uint64_t tmr;

                ret = pevent_get_field_val(NULL, event, "function", &rec, &val, 
0);
-               if (ret < 0)
+               if (ret < 0) {
+                       fprintf(stderr, "timer_expire_entry event returned no 
fucntion value?\n");
                        return;
+               }
                function = (uint64_t)val;

                timer = find_create_timer(function);
@@ -429,8 +433,10 @@ void perf_process_bundle::handle_trace_point(void *trace, 
int cpu, uint64_t time
                        return;

                ret = pevent_get_field_val(NULL, event, "timer", &rec, &val, 0);
-               if (ret < 0)
+               if (ret < 0) {
+                       fprintf(stderr, "softirq_entry event returned no timer 
?\n");
                        return;
+               }
                tmr = (uint64_t)val;

                push_consumer(cpu, timer);
@@ -469,7 +475,7 @@ void perf_process_bundle::handle_trace_point(void *trace, 
int cpu, uint64_t time

                timer = find_create_timer(function);

-               ret = pevent_get_field_val(NULL, event, "htimer", &rec, &val, 
0);
+               ret = pevent_get_field_val(NULL, event, "hrtimer", &rec, &val, 
0);
                if (ret < 0)
                        return;
                tmr = (uint64_t)val;
@@ -490,7 +496,7 @@ void perf_process_bundle::handle_trace_point(void *trace, 
int cpu, uint64_t time
                        return;
                }

-               ret = pevent_get_field_val(NULL, event, "htimer", &rec, &val, 
0);
+               ret = pevent_get_field_val(NULL, event, "hrtimer", &rec, &val, 
0);
                if (ret < 0)
                        return;
                tmr = (uint64_t)val;
@@ -597,6 +603,7 @@ void perf_process_bundle::handle_trace_point(void *trace, 
int cpu, uint64_t time

                ret = pevent_get_field_val(NULL, event, "dev", &rec, &val, 0);
                if (ret < 0)
+                       
                        return;
                dev = (int)val;

@@ -626,8 +633,8 @@ void start_process_measurement(void)
                perf_events->add_event("irq:softirq_exit");
                perf_events->add_event("timer:timer_expire_entry");
                perf_events->add_event("timer:timer_expire_exit");
-               perf_events->add_event("timer:hrtimer_expire_entry");
-               perf_events->add_event("timer:hrtimer_expire_exit");
+               perf_events->add_event("hrtimer_expire_entry");
+               perf_events->add_event("hrtimer_expire_exit");
                if (!perf_events->add_event("power:cpu_idle")){
                        perf_events->add_event("power:power_start");
                        perf_events->add_event("power:power_end");
diff --git a/process/process.cpp b/process/process.cpp
index 6b7ec9d..71f81b6 100644
--- a/process/process.cpp
+++ b/process/process.cpp
@@ -175,7 +175,6 @@ class process * find_create_process(const char *comm, int 
pid)
        return new_proc;
 }

-/* C++... really? */
 class process * find_create_process(char *comm, int pid)
 {
        return find_create_process((const char*)comm, pid);



I couldn't really find any added "for" loop in it.

Regards,

        Márton Németh

_______________________________________________
Power mailing list
Power@bughost.org
https://bughost.org/mailman/listinfo/power

Reply via email to