Messages in this thread | | | Date | Mon, 28 Jun 2021 14:01:32 -0600 | From | Mathieu Poirier <> | Subject | Re: [PATCH v7 2/2] perf cs-etm: Split --dump-raw-trace by AUX records |
| |
On Mon, Jun 28, 2021 at 08:08:02PM +0800, Leo Yan wrote: > On Mon, Jun 28, 2021 at 11:38:34AM +0100, James Clark wrote: > > [...] > > > >> static int cs_etm__process_auxtrace_event(struct perf_session *session, > > >> union perf_event *event, > > >> struct perf_tool *tool __maybe_unused) > > >> @@ -2462,7 +2478,8 @@ static int cs_etm__process_auxtrace_event(struct perf_session *session, > > >> cs_etm__dump_event(etm, buffer); > > >> auxtrace_buffer__put_data(buffer); > > >> } > > >> - } > > >> + } else if (dump_trace) > > >> + dump_queued_data(etm, &event->auxtrace); > > > > > > IIUC, in the function cs_etm__process_auxtrace_event(), since > > > "etm->data_queued" is always true, below flow will never run: > > > > > > if (!etm->data_queued) { > > > ...... > > > > > > if (dump_trace) > > > if (auxtrace_buffer__get_data(buffer, fd)) { > > > cs_etm__dump_event(etm, buffer); > > > auxtrace_buffer__put_data(buffer); > > > } > > > } > > > > > > If so, it's better to use a new patch to polish the code. > > > > > > > Hi Leo, > > > > I think this is not true in piped mode because there is no auxtrace index. > > In that mode, events are processed only in file order and cs_etm__process_auxtrace_event() > > is called for each buffer. > > > > You can reproduce this with something like this: > > > > ./perf record -o - ls > stdio.data > > cat stdio.data | ./perf report -i - > > You are right! I tried these two commands with cs_etm event, just as > you said, in this case, the AUX trace data is not queued; so the flow > for "if (!etm->data_queued)" should be kept. If so, I am very fine > for current change. Thanks for sharing the knowledge. > > > There are some other Coresight features that don't work as expected in this mode, like > > sorting timestamps between CPUs. The aux split patchset won't work either because random > > access isn't possible. And the TRBE patch that I'm working on now won't work, because it > > also requires the random access to lookup the flags on the AUX record to configure the > > decoder for unformatted trace. >
There is a lot of things happening in this area. Based on the above should I still plan to review this set or should I wait for another revision?
Thanks, Mathieu
> Cool, looking forward for the patches :) > > Leo
| |