Re: [PATCH] Revert "perf cs-etm: Move definition of 'traceid_list' global variable from header file"

From: Andrey Zhizhikin <andrey.z@gmail.com>
To: Salvatore Bonaccorso <carnil@debian.org>
Cc: stable@vger.kernel.org, Leo Yan <leo.yan@linaro.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Mark Rutland <mark.rutland@arm.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Suzuki Poulouse <suzuki.poulose@arm.com>,
	Tor Jeremiassen <tor@ti.com>,
	linux-arm-kernel@lists.infradead.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] Revert "perf cs-etm: Move definition of 'traceid_list' global variable from header file"
Date: Fri, 20 Nov 2020 15:29:39 +0100	[thread overview]
Message-ID: <CAHtQpK7=hpWLM-ztyTS8vzGDfG_46Qx2vc6q0fm1dDDU3W6+UA@mail.gmail.com> (raw)
In-Reply-To: <20201120133400.GA405401@eldamar.lan>

Hello Salvatore,

On Fri, Nov 20, 2020 at 2:34 PM Salvatore Bonaccorso <carnil@debian.org> wrote:
>
> Hi Andrey,
>
> On Fri, Nov 20, 2020 at 10:54:22AM +0100, Andrey Zhizhikin wrote:
> > On Fri, Nov 20, 2020 at 8:39 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > >
> > > This reverts commit 168200b6d6ea0cb5765943ec5da5b8149701f36a upstream.
> > > (but only from 4.19.y)
> >
> > This revert would fail the build of 4.19.y with gcc10, I believe the
> > original commit was introduced to address exactly this case. If this
> > is intended behavior that 4.19.y is not compiled with newer gcc
> > versions - then this revert is OK.
>
> TTBOMK, this would not regress the build for newer gcc (specifically
> gcc10) as 4.19.158 is failing perf tool builds there as well (without
> the above commit reverted). Just as an example v4.19.y does not have
> cff20b3151cc ("perf tests bp_account: Make global variable static")
> which is there in v5.6-rc6 to fix build failures with 10.0.1.
>
> But it did regress builds with older gcc's as for instance used in
> Debian buster (gcc 8.3.0) since 4.19.152.
>
> Do I possibly miss something? If there is a solution to make it build
> with newer GCCs and *not* regress previously working GCC versions then
> this is surely the best outcome though.

I guess (and from what I understand in Leo's reply), porting of
95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to
traceID-metadata") should solve the issue for both older and newer gcc
versions.

The breakage is now in
[tools/perf/util/cs-etm-decoder/cs-etm-decoder.c] file (which uses
traceid_list inside). This is solved with the above commit, which
concealed traceid_list internally inside [tools/perf/util/cs-etm.c]
file and exposed to [tools/perf/util/cs-etm-decoder/cs-etm-decoder.c]
via cs_etm__get_cpu() call.

Can you try out to port that commit to see if that would solve your regression?

>
> Regards,
> Salvatore

-- 
Regards,
Andrey.