From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31E28C388F9 for ; Wed, 11 Nov 2020 17:51:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CA6D820756 for ; Wed, 11 Nov 2020 17:51:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726985AbgKKRv1 (ORCPT ); Wed, 11 Nov 2020 12:51:27 -0500 Received: from foss.arm.com ([217.140.110.172]:59078 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725933AbgKKRv0 (ORCPT ); Wed, 11 Nov 2020 12:51:26 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 18690139F; Wed, 11 Nov 2020 09:51:26 -0800 (PST) Received: from [192.168.2.22] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 090A83F718; Wed, 11 Nov 2020 09:51:23 -0800 (PST) Subject: Re: [PATCH v8 00/22] perf arm-spe: Refactor decoding & dumping flow To: Arnaldo Carvalho de Melo Cc: Leo Yan , Dave Martin , James Clark , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Al Grant , Wei Li , John Garry , Will Deacon , Mathieu Poirier , linux-kernel@vger.kernel.org References: <20201111071149.815-1-leo.yan@linaro.org> <20201111161051.GH355344@kernel.org> <20201111161535.GI355344@kernel.org> <20201111174408.GC380127@kernel.org> From: =?UTF-8?Q?Andr=c3=a9_Przywara?= Organization: ARM Ltd. Message-ID: <159cb569-c64d-0685-bcfc-0103dd4de703@arm.com> Date: Wed, 11 Nov 2020 17:51:19 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20201111174408.GC380127@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/11/2020 17:44, Arnaldo Carvalho de Melo wrote: > 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@five perf]$ git bisect good >>> cc6fa07fb1458cca3741919774eb050976471000 is the first bad commit >>> commit cc6fa07fb1458cca3741919774eb050976471000 >>> Author: Leo Yan >>> 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 >>> Reviewed-by: Andre Przywara >>> Link: https://lore.kernel.org/r/20201111071149.815-2-leo.yan@linaro.org >>> Signed-off-by: Arnaldo Carvalho de Melo >>> >>> 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@five perf]$ vim tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c > [acme@five 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 Yes, that basically happens in patch 10/22, so this will then (trivially) clash when you rebase. Thanks! Andre. > [acme@five perf]$ > >> How do you want to handle this? Shall Leo resend, amending this patch >> (and merging 06 and 07 on the way ;-)?