On Fri, 7 Jun 2019 at 12:20, Arnaldo Carvalho de Melo arnaldo.melo@gmail.com wrote:
Em Fri, Jun 07, 2019 at 10:21:36AM +0100, Suzuki K Poulose escreveu:
Hi Mathieu,
On 24/05/2019 18:34, Mathieu Poirier wrote:
When operating in CPU-wide mode being notified of contextID changes is required so that the decoding mechanic is aware of the process context switch.
Signed-off-by: Mathieu Poirier mathieu.poirier@linaro.org
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
I am sorry but, I don't remember reviewing this patch in the previous postings. But here we go.
Can I keep it as is? I addressed one of your concerns below, please check.
- Arnaldo
+++ b/tools/perf/util/cs-etm.h @@ -103,6 +103,18 @@ struct intlist *traceid_list; #define KiB(x) ((x) * 1024) #define MiB(x) ((x) * 1024 * 1024) +/*
- Create a contiguous bitmask starting at bit position @l and ending at
- position @h. For example
- GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
- Carbon copy of implementation found in $KERNEL/include/linux/bitops.h
- */
+#define GENMASK(h, l) \
- (((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
minor nit: Could this be placed in a more generic header file for the other parts of the perf tool to consume ?
Yeah, since we have:
Good catch, we have it already:
[acme@quaco perf]$ tail tools/include/linux/bits.h
- GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
*/ #define GENMASK(h, l) \ (((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
#define GENMASK_ULL(h, l) \ (((~0ULL) - (1ULL << (l)) + 1) & \ (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
#endif /* __LINUX_BITS_H */ [acme@quaco perf]$ [acme@quaco perf]$
So I'm adding this to the pile with a Suggested-by: Suzuki, ok?
commit 3217a621248824fbff8563d8447fdafe69c5316d Author: Arnaldo Carvalho de Melo acme@redhat.com Date: Fri Jun 7 15:14:27 2019 -0300
perf cs-etm: Remove duplicate GENMASK() define, use linux/bits.h instead Suzuki noticed that this should be more useful in a generic header, and after looking I noticed we have it already in our copy of include/linux/bits.h in tools/include, so just use it, test built on x86-64 and ubuntu 19.04 with: perfbuilder@46646c9e848e:/$ aarch64-linux-gnu-gcc --version |& head -1 aarch64-linux-gnu-gcc (Ubuntu/Linaro 8.3.0-6ubuntu1) 8.3.0 perfbuilder@46646c9e848e:/$ Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com> Link: https://lkml.kernel.org/r/68c1c548-33cd-31e8-100d-7ffad008c7b2@arm.com Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Leo Yan <leo.yan@linaro.org> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org, Link: https://lkml.kernel.org/n/tip-69pd3mqvxdlh2shddsc7yhyv@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h index 33b57e748c3d..bc848fd095f4 100644 --- a/tools/perf/util/cs-etm.h +++ b/tools/perf/util/cs-etm.h @@ -9,6 +9,7 @@
#include "util/event.h" #include "util/session.h" +#include <linux/bits.h>
/* Versionning header in case things need tro change in the future. That way
- decoding of old snapshot is still possible.
@@ -161,16 +162,6 @@ struct cs_etm_packet_queue {
#define CS_ETM_INVAL_ADDR 0xdeadbeefdeadbeefUL
-/*
- Create a contiguous bitmask starting at bit position @l and ending at
- position @h. For example
- GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
- Carbon copy of implementation found in $KERNEL/include/linux/bitops.h
- */
-#define GENMASK(h, l) \
(((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
#define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_0_MAX * sizeof(u64))