Messages in this thread Patch in this message | | | From | Thomas Renninger <> | Subject | Re: perf timechart broken | Date | Tue, 11 Jan 2011 09:55:36 +0100 |
| |
On Tuesday 11 January 2011 02:36:28 Frederic Weisbecker wrote: > On Fri, Jan 07, 2011 at 11:04:37AM +0100, Thomas Renninger wrote: > > Hi, > > > > Latest x86 tip has another perf timechart issue introduce > > by latest commits. > > > > ./perf timechart > > gives me: > > "no trace data in the file" > > > > I reverted the latest changes and things seem to break > > between d854861c4292a4e675a5d3bfd862c5f7421c81e8 > > and > > 69aad6f1ee69546dea8535ab8f3da9f445d57328 > > (linux-2.6-x86 tree ids) > > > > Be aware that ./perf timechart is currently broken > > and segfaults on idle events (in 2.6.36 and 2.6.37). > > You mean .36 perf tools faults on .37 kernel? or the opposite? > Is it because power_idle events weren't present on old tools? > Do you have a pointer to those patches? perf timechart will segfault if perf.data has power_{start,end} events included on 2.6.36 and 2.6.37 kernels.
> > I submitted a patch to stable@ but it did not get accepted, > > becaust it's not mainline yet. > > I concentrated on my latest patches instead of making sure > > the tiny fix gets into 2.6.37 first. > > > > Ingo: It's a bit late for 2.6.37, if there is any chance > > to still get it in, the patch below is against linus master... > > otherwise I'll submit it for stable 36+37 asap. > > It's too late for .37, but it's fine, we just need to add > a "Cc: stable@kernel.org" tag in the patch for it to be > backported. I'll submit it to stable@ (it wasn't taken because the patch which included the fix wasn't mainline yet and I forgot to submit it for 2.6.37-rcX).
I can take care of that, but it would be great if someone could look at the issue that perf timechart shows: "no trace data in the file" in x86/tip which seems introduced by one of Arnaldo's latest commits. Reverting some of his latest patches, solved it for me.
> > Anyway to test above regression this mail is about, first > > apply below patch otherwise you get a segfault if idle > > (power_start/end) events got logged. > > > > Thanks, > > > > Thomas > > > > --- > > perf timechart: Fix segfault on cpu idle (power_start/end) events > > > > power_end event does not have type and other attributes and thus does > > not match struct power_event. > > Another struct could be created, but data.cpu is fine for fixing the segfault > > and will work as long as C-states got initiated on the same CPU the idle state > > takes place which is the case for all recent HW. > > > > The power_start/end events get deprecated anyway, thus this is an easy, > > riskless and sufficient solution for the segfault problem. > > > > Signed-off-by: Thomas Renninger <trenn@suse.de> > > CC: mingo@elte.hu > > CC: arjan@linux.intel.com > > > > --- > > tools/perf/builtin-timechart.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c > > index 9bcc38f..b3028eb 100644 > > --- a/tools/perf/builtin-timechart.c > > +++ b/tools/perf/builtin-timechart.c > > @@ -502,7 +502,7 @@ static int process_sample_event(event_t *event, struct perf_session *session) > > c_state_start(pe->cpu_id, data.time, pe->value); > > > > if (strcmp(event_str, "power:power_end") == 0) > > - c_state_end(pe->cpu_id, data.time); > > + c_state_end(data.cpu, data.time); > > On which tree is this based of? Linus master (2.6.37-rc8).
> What I have by looking at tip/master is: Yes, this commit includes the patch (should have been a seperate one...): 20c457b8587bee4644d998331d9e13be82e05b4c perf timechart: Adjust perf timechart to the new power events
Thomas
> > else if (strcmp(event_str, "power:power_end") == 0) > c_state_end(sample->cpu, sample->time); > > > > > if (strcmp(event_str, "power:power_frequency") == 0) > > p_state_change(pe->cpu_id, data.time, pe->value); > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html >
| |