Thread (22 messages) 22 messages, 5 authors, 2021-03-06

Re: [PATCH v3 4/8] perf cs-etm: Fix bitmap for option

From: Arnaldo Carvalho de Melo <acme@kernel.org>
Date: 2021-03-05 17:30:25
Also in: linux-doc, lkml

Em Tue, Feb 09, 2021 at 09:58:55AM +0800, Leo Yan escreveu:
On Mon, Feb 08, 2021 at 01:46:41PM -0700, Mathieu Poirier wrote:
quoted
On Sat, Feb 06, 2021 at 11:08:29PM +0800, Leo Yan wrote:
quoted
From: Suzuki K Poulose <suzuki.poulose@arm.com>

When set option with macros ETM_OPT_CTXTID and ETM_OPT_TS, it wrongly
takes these two values (14 and 28 prespectively) as bit masks, but
actually both are the offset for bits.  But this doesn't lead to
further failure due to the AND logic operation will be always true for
ETM_OPT_CTXTID / ETM_OPT_TS.

This patch defines new independent macros (rather than using the
"config" bits) for requesting the "contextid" and "timestamp" for
cs_etm_set_option().

[leoy: Extract the change as a separate patch for easier review]
This should go just above your name - see below.
I fixed this up and added this patch to my perf/urgent branch, for
v5.12, since the kernel bits are upstream and this is a fix.

Looking at the other patches in the series.

- Arnaldo
 
quoted
quoted
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
Reviewed-by: Mike Leach <redacted>
 Signed-off-by: Suzuki K Poulose [off-list ref]
 [Extract the change as a separate patch for easier review]
 Signed-off-by: Leo Yan [off-list ref]
 Reviewed-by: Mike Leach [off-list ref]
quoted
---
 tools/perf/arch/arm/util/cs-etm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index bd446aba64f7..c25c878fd06c 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -156,6 +156,10 @@ static int cs_etm_set_timestamp(struct auxtrace_record *itr,
 	return err;
 }
 
+#define ETM_SET_OPT_CTXTID	(1 << 0)
+#define ETM_SET_OPT_TS		(1 << 1)
+#define ETM_SET_OPT_MASK	(ETM_SET_OPT_CTXTID | ETM_SET_OPT_TS)
+
I would much rather see this fixed with the BIT() macro as it is done in the
rest of this set than defining new constant.

With the above:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

I have picked up the kernel portion of this set.  I suggest you fix the above
and send another revision to Arnaldo with my RBs.
Will do this.  Thanks for suggestion, Mathieu.

Leo

[...]
-- 

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help