[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 11 Nov 2020 14:44:08 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: André Przywara <andre.przywara@....com>
Cc: Leo Yan <leo.yan@...aro.org>, Dave Martin <Dave.Martin@....com>,
James Clark <james.clark@....com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Al Grant <Al.Grant@....com>, Wei Li <liwei391@...wei.com>,
John Garry <john.garry@...wei.com>,
Will Deacon <will@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 00/22] perf arm-spe: Refactor decoding & dumping flow
Em Wed, Nov 11, 2020 at 04:20:26PM +0000, André Przywara escreveu:
> On 11/11/2020 16:15, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Nov 11, 2020 at 01:10:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Wed, Nov 11, 2020 at 03:11:27PM +0800, Leo Yan escreveu:
> >>> This is patch set v8 for refactoring Arm SPE trace decoding and dumping.
> >>>
> >>> This version addresses Andre's comment to pass parameter '&buf_len' at
> >>> the last call arm_spe_pkt_snprintf() in the function arm_spe_pkt_desc().
> >>>
> >>> This patch set is cleanly applied on the top of perf/core branch
> >>> with commit 644bf4b0f7ac ("perf jevents: Add test for arch std events").
> >>>
> >>> I retested this patch set on Hisilicon D06 platform with commands
> >>> "perf report -D" and "perf script", compared the decoding results
> >>> between with this patch set and without this patch set, "diff" tool
> >>> shows the result as expected.
> >>
> >> With the patches I applied I'm getting:
> >>
> >> util/arm-spe-decoder/arm-spe-pkt-decoder.c: In function 'arm_spe_pkt_desc':
> >> util/arm-spe-decoder/arm-spe-pkt-decoder.c:410:3: error: left shift count >= width of type [-Werror]
> >> case 1: ns = !!(packet->payload & NS_FLAG);
> >> ^
> >> util/arm-spe-decoder/arm-spe-pkt-decoder.c:411:4: error: left shift count >= width of type [-Werror]
> >> el = (packet->payload & EL_FLAG) >> 61;
> >> ^
> >> util/arm-spe-decoder/arm-spe-pkt-decoder.c:411:4: error: left shift count >= width of type [-Werror]
> >> util/arm-spe-decoder/arm-spe-pkt-decoder.c:416:3: error: left shift count >= width of type [-Werror]
> >> case 3: ns = !!(packet->payload & NS_FLAG);
> >> ^
> >> CC /tmp/build/perf/util/arm-spe-decoder/arm-spe-decoder.o
> >>
> >>
> >> On:
> >>
> >> 16 11.70 android-ndk:r12b-arm : FAIL arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
> >> 17 11.32 android-ndk:r15c-arm : FAIL arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
> >>
> >> That were building ok before, builds still under way, perhaps its just
> >> on these old systems...
> >
> > [acme@...e perf]$ git bisect good
> > cc6fa07fb1458cca3741919774eb050976471000 is the first bad commit
> > commit cc6fa07fb1458cca3741919774eb050976471000
> > Author: Leo Yan <leo.yan@...aro.org>
> > Date: Wed Nov 11 15:11:28 2020 +0800
> >
> > perf arm-spe: Include bitops.h for BIT() macro
> >
> > Include header linux/bitops.h, directly use its BIT() macro and remove
> > the self defined macros.
> >
> > Signed-off-by: Leo Yan <leo.yan@...aro.org>
> > Reviewed-by: Andre Przywara <andre.przywara@....com>
> > Link: https://lore.kernel.org/r/20201111071149.815-2-leo.yan@linaro.org
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> >
> > tools/perf/util/arm-spe-decoder/arm-spe-decoder.c | 5 +----
> > tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c | 3 +--
> > 2 files changed, 2 insertions(+), 6 deletions(-)
>
>
> Ah, thanks! I think I mentioned the missing usage of BIT_ULL() in an
> earlier review, and thought this was fixed. Possibly this gets fixed in
> a later patch in this series, and is a temporary regression?
you mean this on that patch that ditches the local BIT() macro, right?
[acme@...e perf]$ vim tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
[acme@...e perf]$ git diff
diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
index 46ddb53a645714bb..5f65a3a70c577207 100644
--- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
+++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
@@ -12,8 +12,8 @@
#include "arm-spe-pkt-decoder.h"
-#define NS_FLAG BIT(63)
-#define EL_FLAG (BIT(62) | BIT(61))
+#define NS_FLAG BIT_ULL(63)
+#define EL_FLAG (BIT_ULL(62) | BIT_ULL(61))
#define SPE_HEADER0_PAD 0x0
#define SPE_HEADER0_END 0x1
[acme@...e perf]$
> How do you want to handle this? Shall Leo resend, amending this patch
> (and merging 06 and 07 on the way ;-)?
Powered by blists - more mailing lists