Messages in this thread | | | Date | Tue, 7 Dec 2010 08:54:46 -0200 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 3/3] perf record/report: Process events in order |
| |
Em Tue, Dec 07, 2010 at 11:47:35AM +0100, Thomas Gleixner escreveu: > On Tue, 7 Dec 2010, Ian Munsie wrote: > > Makes sense. I did something similar in the report layer that I was > > about to send when I saw this email, but this way we have a generic > > solution for other parts of perf that might want this. > > The problem here is that we only get the PERF_RECORD_HEADER_ATTR if perf > > record has been piped somewhere, so running perf record <load>; perf > > report on an unpatched kernel results in the COMM, MMAP, etc events > > being processed last (timestamp -1ULL) and no userspace samples are > > attributed at all: > > Ok. We need to treat timestamp ~0ULL the same as timestamp 0ULL then.
Right.
> > > + event__parse_sample(event, session, &sample); > > > + if (dump_trace) > > > + perf_session__print_tstamp(session, event, &sample); > > > > Moving this here after the dump_printf("%#Lx [%#x]: PERF_RECORD_%s"... > > changes the output of perf report -D in a bad way. Changing the spacing > > in dump_printf can make up for it, or juggle the code around some more. > > Crap. I wanted to restrict the sample parsing to the real events w/o > having this magic comparison in place as we filter out the synth stuff > in the switch case already. > > > How do you want to proceed? At this point either version of the patches > > are pretty functionally equivelant. Mine does the perf report -D > > Hmm. Arnaldo merged my version already. > > > reordering as well, but that isn't really necessary to solve the bug. > > Either way we only have a few minor fixups left. > > Having time ordered output of -D needs more than fixing the time stamp > issue. The dump_printf/dump_trace stuff is scattered all over the > place. So that needs more code churn, as you want to output the non > synth events when the ordered queue is drained.
We can fix that, but then it was supposed to be a dump, something as it comes from the perf.data file.
Perhaps we need something new, that does ordered dumps, I think we can just move all the dump_fprintf stuff to one place, and have calls for those where it is now (unordered) and another one from the ordered place, but looking at different debug variables?
- Arnaldo
| |