On Fri, 7 Jun 2019 at 12:20, Arnaldo Carvalho de Melo <[email protected]> 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 <[email protected]> > > > > > > > Reviewed-by: Suzuki K Poulose <[email protected]> > > > > 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 <[email protected]> > 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 <[email protected]> > Link: > https://lkml.kernel.org/r/[email protected] > Cc: Alexander Shishkin <[email protected]> > Cc: Jiri Olsa <[email protected]> > Cc: Leo Yan <[email protected]> > Cc: Mathieu Poirier <[email protected]> > Cc: Namhyung Kim <[email protected]> > Cc: Peter Zijlstra <[email protected]> > Cc: [email protected] > Cc: [email protected], > Link: > https://lkml.kernel.org/n/[email protected] > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]> > > 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 <[email protected]> > #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb) > > #define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_0_MAX * sizeof(u64))
