From: James Clark <james.clark@linaro.org>
To: linux-perf-users@vger.kernel.org,
gankulkarni@os.amperecomputing.com, coresight@lists.linaro.org,
leo.yan@arm.com, scclevenger@os.amperecomputing.com
Cc: James Clark <james.clark@linaro.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
"Liang, Kan" <kan.liang@linux.intel.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Mike Leach <mike.leach@linaro.org>,
John Garry <john.g.garry@oracle.com>,
Will Deacon <will@kernel.org>, Leo Yan <leo.yan@linux.dev>,
Ben Gainey <ben.gainey@arm.com>,
Ruidong Tian <tianruidong@linux.alibaba.com>,
Benjamin Gray <bgray@linux.ibm.com>,
Mathieu Poirier <mathieu.poirier@linaro.org>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/7] perf cs-etm: Don't flush when packet_queue fills up
Date: Thu, 12 Sep 2024 16:11:32 +0100 [thread overview]
Message-ID: <20240912151143.1264483-2-james.clark@linaro.org> (raw)
In-Reply-To: <20240912151143.1264483-1-james.clark@linaro.org>
cs_etm__flush(), like cs_etm__sample() is an operation that generates a
sample and then swaps the current with the previous packet. Calling
flush after processing the queues results in two swaps which corrupts
the next sample. Therefore it wasn't appropriate to call flush here so
remove it.
Flushing is still done on a discontinuity to explicitly clear the last
branch buffer, but when the packet_queue fills up before reaching a
timestamp, that's not a discontinuity and the call to
cs_etm__process_traceid_queue() already generated samples and drained
the buffers correctly.
This is visible by looking for a branch that has the same target as the
previous branch and the following source is before the address of the
last target, which is impossible as execution would have had to have
gone backwards:
ffff800080849d40 _find_next_and_bit+0x78 => ffff80008011cadc update_sg_lb_stats+0x94
(packet_queue fills here before a timestamp, resulting in a flush and
branch target ffff80008011cadc is duplicated.)
ffff80008011cb1c update_sg_lb_stats+0xd4 => ffff80008011cadc update_sg_lb_stats+0x94
ffff8000801117c4 cpu_util+0x24 => ffff8000801117d4 cpu_util+0x34
After removing the flush the correct branch target is used for the
second sample, and ffff8000801117c4 is no longer before the previous
address:
ffff800080849d40 _find_next_and_bit+0x78 => ffff80008011cadc update_sg_lb_stats+0x94
ffff80008011cb1c update_sg_lb_stats+0xd4 => ffff8000801117a0 cpu_util+0x0
ffff8000801117c4 cpu_util+0x24 => ffff8000801117d4 cpu_util+0x34
Make sure that a final branch stack is output at the end of the trace
by calling cs_etm__end_block(). This is already done for both the
timeless decode paths.
Fixes: 21fe8dc1191a ("perf cs-etm: Add support for CPU-wide trace scenarios")
Reported-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>
Closes: https://lore.kernel.org/all/20240719092619.274730-1-gankulkarni@os.amperecomputing.com/
Signed-off-by: James Clark <james.clark@linaro.org>
---
tools/perf/util/cs-etm.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 90f32f327b9b..242788ac9625 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2490,12 +2490,6 @@ static void cs_etm__clear_all_traceid_queues(struct cs_etm_queue *etmq)
/* Ignore return value */
cs_etm__process_traceid_queue(etmq, tidq);
-
- /*
- * Generate an instruction sample with the remaining
- * branchstack entries.
- */
- cs_etm__flush(etmq, tidq);
}
}
@@ -2638,7 +2632,7 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm)
while (1) {
if (!etm->heap.heap_cnt)
- goto out;
+ break;
/* Take the entry at the top of the min heap */
cs_queue_nr = etm->heap.heap_array[0].queue_nr;
@@ -2721,6 +2715,23 @@ static int cs_etm__process_timestamped_queues(struct cs_etm_auxtrace *etm)
ret = auxtrace_heap__add(&etm->heap, cs_queue_nr, cs_timestamp);
}
+ for (i = 0; i < etm->queues.nr_queues; i++) {
+ struct int_node *inode;
+
+ etmq = etm->queues.queue_array[i].priv;
+ if (!etmq)
+ continue;
+
+ intlist__for_each_entry(inode, etmq->traceid_queues_list) {
+ int idx = (int)(intptr_t)inode->priv;
+
+ /* Flush any remaining branch stack entries */
+ tidq = etmq->traceid_queues[idx];
+ ret = cs_etm__end_block(etmq, tidq);
+ if (ret)
+ return ret;
+ }
+ }
out:
return ret;
}
--
2.34.1
next prev parent reply other threads:[~2024-09-12 15:12 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-12 15:11 [PATCH v2 0/7] perf: cs-etm: Coresight decode and disassembly improvements James Clark
2024-09-12 15:11 ` James Clark [this message]
2024-09-13 11:17 ` [PATCH v2 1/7] perf cs-etm: Don't flush when packet_queue fills up Leo Yan
2024-09-12 15:11 ` [PATCH v2 2/7] perf cs-etm: Use new OpenCSD consistency checks James Clark
2024-09-13 11:54 ` Leo Yan
2024-09-13 12:09 ` James Clark
2024-09-13 13:03 ` Leo Yan
2024-09-12 15:11 ` [PATCH v2 3/7] perf scripting python: Add function to get a config value James Clark
2024-09-13 13:40 ` Leo Yan
2024-09-12 15:11 ` [PATCH v2 4/7] perf scripts python cs-etm: Update to use argparse James Clark
2024-09-13 12:44 ` Leo Yan
2024-09-12 15:11 ` [PATCH v2 5/7] perf scripts python cs-etm: Improve arguments James Clark
2024-09-13 13:01 ` Leo Yan
2024-09-12 15:11 ` [PATCH v2 6/7] perf scripts python cs-etm: Add start and stop arguments James Clark
2024-09-13 13:20 ` Leo Yan
2024-09-16 10:41 ` James Clark
2024-09-12 15:11 ` [PATCH v2 7/7] perf test: cs-etm: Test Coresight disassembly script James Clark
2024-09-13 13:35 ` Leo Yan
2024-09-16 13:25 ` James Clark
2024-09-12 19:23 ` [PATCH v2 0/7] perf: cs-etm: Coresight decode and disassembly improvements Arnaldo Carvalho de Melo
2024-09-17 8:15 ` James Clark
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=20240912151143.1264483-2-james.clark@linaro.org \
--to=james.clark@linaro.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=ben.gainey@arm.com \
--cc=bgray@linux.ibm.com \
--cc=coresight@lists.linaro.org \
--cc=gankulkarni@os.amperecomputing.com \
--cc=irogers@google.com \
--cc=john.g.garry@oracle.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=leo.yan@arm.com \
--cc=leo.yan@linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=scclevenger@os.amperecomputing.com \
--cc=suzuki.poulose@arm.com \
--cc=tianruidong@linux.alibaba.com \
--cc=will@kernel.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).