Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v8 00/22] perf arm-spe: Refactor decoding & dumping flow
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Leo Yan <leo.yan@linaro.org>, Dave Martin <Dave.Martin@arm.com>,
        James Clark <james.clark@arm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Al Grant <Al.Grant@arm.com>, Wei Li <liwei391@huawei.com>,
        John Garry <john.garry@huawei.com>,
        Will Deacon <will@kernel.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        linux-kernel@vger.kernel.org
References: <20201111071149.815-1-leo.yan@linaro.org>
 <20201111161051.GH355344@kernel.org> <20201111161535.GI355344@kernel.org>
 <c22d9fa3-51bb-d86f-2a03-390c6844256a@arm.com>
 <20201111174408.GC380127@kernel.org>
From:   =?UTF-8?Q?Andr=c3=a9_Przywara?= <andre.przywara@arm.com>
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

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 <leo.yan@linaro.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@linaro.org>
>>>     Reviewed-by: Andre Przywara <andre.przywara@arm.com>
>>>     Link: https://lore.kernel.org/r/20201111071149.815-2-leo.yan@linaro.org
>>>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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@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 ;-)?