linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Grant <al.grant@foss.arm.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	linux-perf-users@vger.kernel.org
Subject: [PATCH] perf inject corrupts file by deleting event
Date: Fri, 13 Nov 2020 20:38:26 +0000	[thread overview]
Message-ID: <b9cf5611-daae-2390-3439-6617f8f0a34b@foss.arm.com> (raw)
In-Reply-To: <83633eb2-04dc-4a13-3ad7-abd3a7459ac1@foss.arm.com>

"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.

Signed-off-by: Al Grant <al.grant@arm.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
  tools/perf/builtin-inject.c | 7 -------
  1 file changed, 7 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 452a75fe68e5..f4968ebf5f3a 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -791,13 +791,6 @@ static int __cmd_inject(struct perf_inject *inject)
                             inject->itrace_synth_opts.add_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", evsel__name(evsel));
-                               evlist__remove(session->evlist, evsel);
-                               evsel__delete(evsel);
-                       }
                 }
                 session->header.data_offset = output_data_offset;
                 session->header.data_size = inject->bytes_written;

       reply	other threads:[~2020-11-13 20:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <83633eb2-04dc-4a13-3ad7-abd3a7459ac1@foss.arm.com>
2020-11-13 20:38 ` Al Grant [this message]
2020-11-16 16:25   ` [PATCH] perf inject corrupts file by deleting event Namhyung Kim
2020-11-16 16:45     ` Arnaldo Carvalho de Melo
2020-11-16 16:48       ` Arnaldo Carvalho de Melo
2020-11-16 16:59         ` Arnaldo Carvalho de Melo
2020-12-15  9:48           ` Al Grant
2020-12-15 13:33             ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9cf5611-daae-2390-3439-6617f8f0a34b@foss.arm.com \
    --to=al.grant@foss.arm.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@redhat.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).