Thomas Spura wrote:
> 2012/4/21 Németh Márton <[email protected]>:
>> 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 <[email protected]>
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
[email protected]
https://bughost.org/mailman/listinfo/power
