| The patch has minor conflict resolutions due to the missing branch stack |
| for regular events feature and name changes in ToT. |
| Particularly perf_evsel and perf_evlist were renamed to evsel and evlist. |
| |
| From 1c756cd429d8f3da33d31f2a970284b9d5260534 Mon Sep 17 00:00:00 2001 |
| From: Al Grant <al.grant@foss.arm.com> |
| Date: Fri, 13 Nov 2020 20:38:26 +0000 |
| Subject: [PATCH] perf inject: Fix file corruption due to event deletion |
| |
| "perf inject" can create corrupt files when synthesizing sample events from AUX |
| data. This happens when in the input file, the first event (for the AUX data) |
| has a different sample_type from the second event (generally dummy). |
| |
| Specifically, they differ in the bits that indicate the standard fields |
| appended to perf records in the mmap buffer. "perf inject" deletes the first |
| event and moves up the second event to first position. |
| |
| The problem is with the synthetic PERF_RECORD_MMAP (etc.) events created |
| by "perf record". |
| |
| Since these are synthetic versions of events which are normally produced |
| by the kernel, they have to have the standard fields appended as |
| described by sample_type. |
| |
| "perf record" fills these in with zeroes, including the IDENTIFIER |
| field; perf readers interpret records with zero IDENTIFIER using the |
| descriptor for the first event in the file. |
| |
| Since "perf inject" changes the first event, these synthetic records are |
| then processed with the wrong value of sample_type, and the perf reader |
| reads bad data, reports on incorrect length records etc. |
| |
| Mismatching sample_types are seen with "perf record -e cs_etm//", where the AUX |
| event has TID|TIME|CPU|IDENTIFIER and the dummy event has TID|TIME|IDENTIFIER. |
| |
| Perhaps they could be the same, but it isn't normally a problem if they aren't |
| - perf has no problems reading the file. |
| |
| The sample_types have to agree on the position of IDENTIFIER, because |
| that's how perf finds the right event descriptor in the first place, but |
| they don't normally have to agree on other fields, and perf doesn't |
| check that they do. |
| |
| The problem is specific to the way "perf inject" reorganizes the events |
| and the way synthetic MMAP events are recorded with a zero identifier. A |
| simple solution is to stop "perf inject" deleting the tracing event. |
| |
| Committer testing |
| |
| Removed the now unused 'evsel' variable, update the comment about the |
| evsel removal not being performed anymore, and apply the patch manually |
| as it failed with this warning: |
| |
| warning: Patch sent with format=flowed; space at the end of lines might be lost. |
| |
| Testing it with: |
| |
| $ perf bench internals inject-build-id |
| # Running 'internals/inject-build-id' benchmark: |
| Average build-id injection took: 8.543 msec (+- 0.130 msec) |
| Average time per event: 0.838 usec (+- 0.013 usec) |
| Average memory usage: 12717 KB (+- 9 KB) |
| Average build-id-all injection took: 5.710 msec (+- 0.058 msec) |
| Average time per event: 0.560 usec (+- 0.006 usec) |
| Average memory usage: 12079 KB (+- 7 KB) |
| $ |
| |
| Signed-off-by: Al Grant <al.grant@arm.com> |
| Acked-by: Adrian Hunter <adrian.hunter@intel.com> |
| Acked-by: Namhyung Kim <namhyung@kernel.org> |
| Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> |
| Cc: Jiri Olsa <jolsa@redhat.com> |
| Cc: Mark Rutland <mark.rutland@arm.com> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| LPU-Reference: b9cf5611-daae-2390-3439-6617f8f0a34b@foss.arm.com |
| Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> |
| --- |
| tools/perf/builtin-inject.c | 12 +----------- |
| 1 file changed, 1 insertion(+), 11 deletions(-) |
| |
| diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c |
| index 452a75fe68e5..0462dc8db2e3 100644 |
| --- a/tools/perf/builtin-inject.c |
| +++ b/tools/perf/builtin-inject.c |
| @@ -779,27 +779,14 @@ static int __cmd_inject(struct perf_inject *inject) |
| dsos__hit_all(session); |
| /* |
| * The AUX areas have been removed and replaced with |
| - * synthesized hardware events, so clear the feature flag and |
| - * remove the evsel. |
| + * synthesized hardware events, so clear the feature flag. |
| */ |
| if (inject->itrace_synth_opts.set) { |
| - struct perf_evsel *evsel; |
| - |
| perf_header__clear_feat(&session->header, |
| HEADER_AUXTRACE); |
| if (inject->itrace_synth_opts.last_branch) |
| perf_header__set_feat(&session->header, |
| HEADER_BRANCH_STACK); |
| - evsel = perf_evlist__id2evsel_strict(session->evlist, |
| - inject->aux_id); |
| - if (evsel) { |
| - pr_debug("Deleting %s\n", |
| - perf_evsel__name(evsel)); |
| - perf_evlist__remove(session->evlist, evsel); |
| - perf_evsel__delete(evsel); |
| - } |
| - if (inject->strip) |
| - strip_fini(inject); |
| } |
| session->header.data_offset = output_data_offset; |
| session->header.data_size = inject->bytes_written; |
| -- |
| 2.30.0.478.g8a0d178c01-goog |
| |