Hi, I had informed Chris about it, I guess he will be correcting it.
Br John > -----Original Message----- > From: Németh Márton [mailto:nm...@freemail.hu] > Sent: Saturday, April 21, 2012 12:48 PM > To: Powertop Mailing List; Mathew, John > Subject: handle_trace_point() + cpu_idle: conditional jump depends on > uninitialised value > > Hi, > > I run powertop [1] version 1dfdb80d6333960ed310fff7b60439f79d154a84 > together with Valgrind. I get this message (among others): > > ==2739== Conditional jump or move depends on uninitialised value(s) > ==2739== at 0x80689BB: > perf_process_bundle::handle_trace_point(void*, int, unsigned long long) > (do_process.cpp:552) > ==2739== by 0x806391A: perf_bundle::process() (perf_bundle.cpp:303) > ==2739== by 0x8069A53: process_process_data() (do_process.cpp:1101) > ==2739== by 0x804FA4E: one_measurement(int) (main.cpp:174) > ==2739== by 0x8050399: main (main.cpp:390) > > I checked the function perf_process_bundle::handle_trace_point() and it > really > looks like that the val variable is used uninitialized when event->name > is "cpu_idle": > > void perf_process_bundle::handle_trace_point(void *trace, int cpu, > uint64_t time) > { > // ... > unsigned long long val; > // ... > > else if (strcmp(event->name, "cpu_idle") == 0) { > if (val == 4294967295) // ---> equals 0xFFFF_FFFF > consume_blame(cpu); > else > set_wakeup_pending(cpu); > } > // ... > } > > I have even found the commit introduced this fragment (using "git > blame") > but I'm not sure from where the value of the "val" variable should come > from. > > commit ed91c3e55760cd5d906821d87377a087f9681016 > Author: John Mathew <john.mat...@intel.com> > Date: Wed Apr 18 16:50:32 2012 -0700 > > From kernel version 2.6.41 the power_start, power_end > and power_frequency power traces will be replaced with > cpu_idle and cpu_frequency power traces. Refer kernel > documentation events-power.txt for details. This patch > enables the use of new power traces and will fall back > to old traces if new traces are not avaialable > > Patch rebased by hand to HEAD > > > References: > [1] powertop git repo > git://github.com/fenrus75/powertop.git > > [2] Valgrind > http://valgrind.org/ > > Regards, > > Márton Németh --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
--- Begin Message ---Hi Chris, The rest looks fine, I have a doubt on this diff --git a/process/do_process.cpp b/process/do_process.cpp index 4954e1a..1fd4812 100644 --- a/process/do_process.cpp +++ b/process/do_process.cpp @@ -542,6 +542,12 @@ void perf_process_bundle::handle_trace_point(void *trace, int cpu, uint64_t time t = work->done(time, wk); consumer_child_time(cpu, t); } + else if (strcmp(event->name, "cpu_idle") == 0) { Do we need to initialize val with a call to ret = pevent_get_field_val(NULL, event, "state", &rec, &val, 0); before using val? >>>>>>>>>>>>>>>>>>>>>>>>>>>+ if (val == 4294967295) + consume_blame(cpu); + else + set_wakeup_pending(cpu); + } Br John > -----Original Message----- > From: Chris Ferron [mailto:chris.e.fer...@linux.intel.com] > Sent: Thursday, April 19, 2012 2:56 AM > To: Mathew, John > Subject: Re: [PATCH] Add support for new power traces > > Hi john, > First sorry it has taken me so long to get to your patch. There is > a huge backlog. Second thanks for the patch, but I need to enlist your > help. > Due to the backlog, by the time I got to your patch, it would not > apply. > This is due to several key change, most of which is being how we are > pulling pevents. > I have re-based you patch by hand and regenerated a new patch. Can you > take a look at it, and insure it still acts as you intended it to? > > Thanks > Chris
--- End Message ---
_______________________________________________ Power mailing list Power@bughost.org https://bughost.org/mailman/listinfo/power
