Hi Greg, Sasha and all,
This is a resubmit of the patches already done in https://lore.kernel.org/stable/20201125201215.26455-1-carnil@debian.org/ and https://lore.kernel.org/stable/20201125201215.26455-2-carnil@debian.org/
The issue can be explained as this. In
https://lore.kernel.org/stable/20201014135627.GA3698844@kroah.com/
on request 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header file") was queued back to 4.19.y to fix build failures for perf with more recent GCCs.
But for 4.19.y this was wrong because it missed to pick as well a dependency needed, and in turn it caused build failures with older GCC (8.3.0 as used in Debian stable in that case).
The commit was reverted in a later in 4.19.159.
It as though requested to try to allow as well compilation with more recent GCCs (while obviously not breaking older GCC builds) and found that the cause was just the missing dependency to pick up, namely pick 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata") before 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header file").
Regards, Salvatore
Leo Yan (2): perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata perf cs-etm: Move definition of 'traceid_list' global variable from header file
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 8 ++--- tools/perf/util/cs-etm.c | 29 +++++++++++++++---- tools/perf/util/cs-etm.h | 10 +++++-- 3 files changed, 33 insertions(+), 14 deletions(-)
From: Leo Yan leo.yan@linaro.org
commit 95c6fe970a0160cb770c5dce9f80311b42d030c0 upstream.
If packet processing wants to know the packet is bound with which ETM version, it needs to access metadata to decide that based on metadata magic number; but we cannot simply to use CPU logic ID number as index to access metadata sequential array, especially when system have hotplugged off CPUs, the metadata array are only allocated for online CPUs but not offline CPUs, so the CPU logic number doesn't match with its index in the array.
This patch is to change tuple from traceID-CPU# to traceID-metadata, thus it can use the tuple to retrieve metadata pointer according to traceID.
For safe accessing metadata fields, this patch provides helper function cs_etm__get_cpu() which is used to return CPU number according to traceID; cs_etm_decoder__buffer_packet() is the first consumer for this helper function.
Signed-off-by: Leo Yan leo.yan@linaro.org Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Jiri Olsa jolsa@redhat.com Cc: Mike Leach mike.leach@linaro.org Cc: Namhyung Kim namhyung@kernel.org Cc: Robert Walker robert.walker@arm.com Cc: Suzuki K Poulouse suzuki.poulose@arm.com Cc: coresight ml coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Link: http://lkml.kernel.org/r/20190129122842.32041-6-leo.yan@linaro.org Signed-off-by: Arnaldo Carvalho de Melo acme@redhat.com [Salvatore Bonaccorso: Adjust for context changes in tools/perf/util/cs-etm-decoder/cs-etm-decoder.c] Signed-off-by: Salvatore Bonaccorso carnil@debian.org --- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 8 +++--- tools/perf/util/cs-etm.c | 26 ++++++++++++++----- tools/perf/util/cs-etm.h | 9 ++++++- 3 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c index 938def6d0bb9..f540037eb705 100644 --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c @@ -278,14 +278,12 @@ cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder, enum cs_etm_sample_type sample_type) { u32 et = 0; - struct int_node *inode = NULL; + int cpu;
if (decoder->packet_count >= MAX_BUFFER - 1) return OCSD_RESP_FATAL_SYS_ERR;
- /* Search the RB tree for the cpu associated with this traceID */ - inode = intlist__find(traceid_list, trace_chan_id); - if (!inode) + if (cs_etm__get_cpu(trace_chan_id, &cpu) < 0) return OCSD_RESP_FATAL_SYS_ERR;
et = decoder->tail; @@ -296,7 +294,7 @@ cs_etm_decoder__buffer_packet(struct cs_etm_decoder *decoder, decoder->packet_buffer[et].sample_type = sample_type; decoder->packet_buffer[et].exc = false; decoder->packet_buffer[et].exc_ret = false; - decoder->packet_buffer[et].cpu = *((int *)inode->priv); + decoder->packet_buffer[et].cpu = cpu; decoder->packet_buffer[et].start_addr = CS_ETM_INVAL_ADDR; decoder->packet_buffer[et].end_addr = CS_ETM_INVAL_ADDR;
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 7b5e15cc6b71..5cde3956e19a 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -91,6 +91,20 @@ static int cs_etm__update_queues(struct cs_etm_auxtrace *etm); static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm, pid_t tid, u64 time_);
+int cs_etm__get_cpu(u8 trace_chan_id, int *cpu) +{ + struct int_node *inode; + u64 *metadata; + + inode = intlist__find(traceid_list, trace_chan_id); + if (!inode) + return -EINVAL; + + metadata = inode->priv; + *cpu = (int)metadata[CS_ETM_CPU]; + return 0; +} + static void cs_etm__packet_dump(const char *pkt_string) { const char *color = PERF_COLOR_BLUE; @@ -230,7 +244,7 @@ static void cs_etm__free(struct perf_session *session) cs_etm__free_events(session); session->auxtrace = NULL;
- /* First remove all traceID/CPU# nodes for the RB tree */ + /* First remove all traceID/metadata nodes for the RB tree */ intlist__for_each_entry_safe(inode, tmp, traceid_list) intlist__remove(traceid_list, inode); /* Then the RB tree itself */ @@ -1316,9 +1330,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event, 0xffffffff);
/* - * Create an RB tree for traceID-CPU# tuple. Since the conversion has - * to be made for each packet that gets decoded, optimizing access in - * anything other than a sequential array is worth doing. + * Create an RB tree for traceID-metadata tuple. Since the conversion + * has to be made for each packet that gets decoded, optimizing access + * in anything other than a sequential array is worth doing. */ traceid_list = intlist__new(NULL); if (!traceid_list) { @@ -1384,8 +1398,8 @@ int cs_etm__process_auxtrace_info(union perf_event *event, err = -EINVAL; goto err_free_metadata; } - /* All good, associate the traceID with the CPU# */ - inode->priv = &metadata[j][CS_ETM_CPU]; + /* All good, associate the traceID with the metadata pointer */ + inode->priv = metadata[j]; }
/* diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index 37f8d48179ca..fb5fc6538b7f 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -53,7 +53,7 @@ enum { CS_ETMV4_PRIV_MAX, };
-/* RB tree for quick conversion between traceID and CPUs */ +/* RB tree for quick conversion between traceID and metadata pointers */ struct intlist *traceid_list;
#define KiB(x) ((x) * 1024) @@ -69,6 +69,7 @@ static const u64 __perf_cs_etmv4_magic = 0x4040404040404040ULL; #ifdef HAVE_CSTRACE_SUPPORT int cs_etm__process_auxtrace_info(union perf_event *event, struct perf_session *session); +int cs_etm__get_cpu(u8 trace_chan_id, int *cpu); #else static inline int cs_etm__process_auxtrace_info(union perf_event *event __maybe_unused, @@ -76,6 +77,12 @@ cs_etm__process_auxtrace_info(union perf_event *event __maybe_unused, { return -1; } + +static inline int cs_etm__get_cpu(u8 trace_chan_id __maybe_unused, + int *cpu __maybe_unused) +{ + return -1; +} #endif
#endif
From: Leo Yan leo.yan@linaro.org
commit 168200b6d6ea0cb5765943ec5da5b8149701f36a upstream.
The variable 'traceid_list' is defined in the header file cs-etm.h, if multiple C files include cs-etm.h the compiler might complaint for multiple definition of 'traceid_list'.
To fix multiple definition error, move the definition of 'traceid_list' into cs-etm.c.
Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata") Reported-by: Thomas Backlund tmb@mageia.org Signed-off-by: Leo Yan leo.yan@linaro.org Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org Reviewed-by: Mike Leach mike.leach@linaro.org Tested-by: Mike Leach mike.leach@linaro.org Tested-by: Thomas Backlund tmb@mageia.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Jiri Olsa jolsa@redhat.com Cc: Mark Rutland mark.rutland@arm.com Cc: Namhyung Kim namhyung@kernel.org Cc: Peter Zijlstra peterz@infradead.org Cc: Suzuki Poulouse suzuki.poulose@arm.com Cc: Tor Jeremiassen tor@ti.com Cc: linux-arm-kernel@lists.infradead.org Link: http://lore.kernel.org/lkml/20200505133642.4756-1-leo.yan@linaro.org Signed-off-by: Arnaldo Carvalho de Melo acme@redhat.com Signed-off-by: Salvatore Bonaccorso carnil@debian.org --- tools/perf/util/cs-etm.c | 3 +++ tools/perf/util/cs-etm.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 5cde3956e19a..3275b8dc9344 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -87,6 +87,9 @@ struct cs_etm_queue { struct cs_etm_packet *packet; };
+/* RB tree for quick conversion between traceID and metadata pointers */ +static struct intlist *traceid_list; + static int cs_etm__update_queues(struct cs_etm_auxtrace *etm); static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm, pid_t tid, u64 time_); diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index fb5fc6538b7f..97c3152f5bfd 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -53,9 +53,6 @@ enum { CS_ETMV4_PRIV_MAX, };
-/* RB tree for quick conversion between traceID and metadata pointers */ -struct intlist *traceid_list; - #define KiB(x) ((x) * 1024) #define MiB(x) ((x) * 1024 * 1024)
On Sun, Dec 27, 2020 at 10:27:43AM +0100, Salvatore Bonaccorso wrote:
Hi Greg, Sasha and all,
This is a resubmit of the patches already done in https://lore.kernel.org/stable/20201125201215.26455-1-carnil@debian.org/ and https://lore.kernel.org/stable/20201125201215.26455-2-carnil@debian.org/
The issue can be explained as this. In
https://lore.kernel.org/stable/20201014135627.GA3698844@kroah.com/
on request 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header file") was queued back to 4.19.y to fix build failures for perf with more recent GCCs.
But for 4.19.y this was wrong because it missed to pick as well a dependency needed, and in turn it caused build failures with older GCC (8.3.0 as used in Debian stable in that case).
The commit was reverted in a later in 4.19.159.
It as though requested to try to allow as well compilation with more recent GCCs (while obviously not breaking older GCC builds) and found that the cause was just the missing dependency to pick up, namely pick 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata") before 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header file").
Thanks for these, both now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Salvatore Bonaccorso