* [PATCH 0/2] perf jevents: Enable build warnings
@ 2021-10-15 16:48 John Garry
2021-10-15 16:48 ` [PATCH 1/2] perf jevents: Fix some would-be warnings John Garry
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: John Garry @ 2021-10-15 16:48 UTC (permalink / raw)
To: peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo
Cc: irogers, linux-perf-users, linux-kernel, kjain, John Garry
Currently jevents builds without any complier warning flags enabled. So
use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS. I am not
100% confident that this is the best way, but sending out for review.
Baseline is be8ecc57f180 (HEAD, acme/perf/core) perf srcline: Use
long-running addr2line per DSO
Thanks!
John Garry (2):
perf jevents: Fix some would-be warnings
perf jevents: Enable warnings through HOSTCFLAGS
tools/perf/Makefile.config | 1 +
tools/perf/Makefile.perf | 2 +-
tools/perf/pmu-events/Build | 2 +-
tools/perf/pmu-events/jevents.c | 10 ++++------
4 files changed, 7 insertions(+), 8 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] perf jevents: Fix some would-be warnings 2021-10-15 16:48 [PATCH 0/2] perf jevents: Enable build warnings John Garry @ 2021-10-15 16:48 ` John Garry 2021-10-15 16:48 ` [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry 2021-10-20 14:31 ` [PATCH 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo 2 siblings, 0 replies; 11+ messages in thread From: John Garry @ 2021-10-15 16:48 UTC (permalink / raw) To: peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo Cc: irogers, linux-perf-users, linux-kernel, kjain, John Garry Before enabling warnings through HOSTCFLAGS, fix the would-be warnings: HOSTCC pmu-events/jevents.o pmu-events/jevents.c:74:22: warning: no previous prototype for ‘convert’ [-Wmissing-prototypes] 74 | enum aggr_mode_class convert(const char *aggr_mode) | ^~~~~~~ pmu-events/jevents.c: In function ‘print_events_table_entry’: pmu-events/jevents.c:373:8: warning: declaration of ‘topic’ shadows a global declaration [-Wshadow] 373 | char *topic = pd->topic; | ^~~~~ pmu-events/jevents.c:316:14: note: shadowed declaration is here 316 | static char *topic; | ^~~~~ pmu-events/jevents.c: In function ‘json_events’: pmu-events/jevents.c:554:9: warning: declaration of ‘func’ shadows a global declaration [-Wshadow] 554 | int (*func)(void *data, struct json_event *je), | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ pmu-events/jevents.c:85:15: note: shadowed declaration is here 85 | typedef int (*func)(void *data, struct json_event *je); | ^~~~ pmu-events/jevents.c: In function ‘main’: pmu-events/jevents.c:1211:25: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 1211 | char *err_string_ext = ""; | ^~ pmu-events/jevents.c:1304:17: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 1304 | err_string_ext = " for std arch event"; | ^ Signed-off-by: John Garry <john.garry@huawei.com> --- tools/perf/pmu-events/jevents.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c index 19497e4f8a86..b6bd33d85066 100644 --- a/tools/perf/pmu-events/jevents.c +++ b/tools/perf/pmu-events/jevents.c @@ -71,7 +71,7 @@ struct json_event { char *metric_constraint; }; -enum aggr_mode_class convert(const char *aggr_mode) +static enum aggr_mode_class convert(const char *aggr_mode) { if (!strcmp(aggr_mode, "PerCore")) return PerCore; @@ -82,8 +82,6 @@ enum aggr_mode_class convert(const char *aggr_mode) return -1; } -typedef int (*func)(void *data, struct json_event *je); - static LIST_HEAD(sys_event_tables); struct sys_event_table { @@ -370,7 +368,7 @@ static int print_events_table_entry(void *data, struct json_event *je) { struct perf_entry_data *pd = data; FILE *outfp = pd->outfp; - char *topic = pd->topic; + char *topic_local = pd->topic; /* * TODO: Remove formatting chars after debugging to reduce @@ -385,7 +383,7 @@ static int print_events_table_entry(void *data, struct json_event *je) fprintf(outfp, "\t.desc = \"%s\",\n", je->desc); if (je->compat) fprintf(outfp, "\t.compat = \"%s\",\n", je->compat); - fprintf(outfp, "\t.topic = \"%s\",\n", topic); + fprintf(outfp, "\t.topic = \"%s\",\n", topic_local); if (je->long_desc && je->long_desc[0]) fprintf(outfp, "\t.long_desc = \"%s\",\n", je->long_desc); if (je->pmu) @@ -1208,7 +1206,7 @@ int main(int argc, char *argv[]) const char *arch; const char *output_file; const char *start_dirname; - char *err_string_ext = ""; + const char *err_string_ext = ""; struct stat stbuf; prog = basename(argv[0]); -- 2.26.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS 2021-10-15 16:48 [PATCH 0/2] perf jevents: Enable build warnings John Garry 2021-10-15 16:48 ` [PATCH 1/2] perf jevents: Fix some would-be warnings John Garry @ 2021-10-15 16:48 ` John Garry 2021-10-18 10:41 ` James Clark 2021-10-20 14:31 ` [PATCH 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo 2 siblings, 1 reply; 11+ messages in thread From: John Garry @ 2021-10-15 16:48 UTC (permalink / raw) To: peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo Cc: irogers, linux-perf-users, linux-kernel, kjain, John Garry Currently no compiler warnings at all are enabled for building jevents, so help catch bugs at compile time by enabling through HOSTCFLAGS. Signed-off-by: John Garry <john.garry@huawei.com> --- tools/perf/Makefile.config | 1 + tools/perf/Makefile.perf | 2 +- tools/perf/pmu-events/Build | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index 0ae2e3d8b832..65934984f032 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected) detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected) CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) +HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) include $(srctree)/tools/scripts/Makefile.arch diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index 7df13e74450c..118bcdc70bb4 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -226,7 +226,7 @@ else endif export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK -export HOSTCC HOSTLD HOSTAR +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS include $(srctree)/tools/build/Makefile.include diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build index a055dee6a46a..d5c287f069a2 100644 --- a/tools/perf/pmu-events/Build +++ b/tools/perf/pmu-events/Build @@ -1,7 +1,7 @@ hostprogs := jevents jevents-y += json.o jsmn.o jevents.o -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS) pmu-events-y += pmu-events.o JDIR = pmu-events/arch/$(SRCARCH) JSON = $(shell [ -d $(JDIR) ] && \ -- 2.26.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS 2021-10-15 16:48 ` [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry @ 2021-10-18 10:41 ` James Clark 2021-10-19 8:37 ` John Garry 0 siblings, 1 reply; 11+ messages in thread From: James Clark @ 2021-10-18 10:41 UTC (permalink / raw) To: John Garry, peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo Cc: irogers, linux-perf-users, linux-kernel, kjain On 15/10/2021 17:48, John Garry wrote: > Currently no compiler warnings at all are enabled for building jevents, > so help catch bugs at compile time by enabling through HOSTCFLAGS. Is there any reason to not enable -Wall and -Werror so that it builds like the main project? Or if HOSTCFLAGS ends up being the same as CORE_CFLAGS then why not use CORE_CFLAGS instead? I added them like this and only one unused function needs to be removed to make it build successfully: diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index 65934984f032..b2edcedf01db 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -18,6 +18,8 @@ detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected) CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) +HOSTCFLAGS += -Wall +HOSTCFLAGS += -Wextra include $(srctree)/tools/scripts/Makefile.arch @@ -212,6 +214,7 @@ endif ifneq ($(WERROR),0) CORE_CFLAGS += -Werror CXXFLAGS += -Werror + HOSTCFLAGS += -Werror endif ifndef DEBUG > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > tools/perf/Makefile.config | 1 + > tools/perf/Makefile.perf | 2 +- > tools/perf/pmu-events/Build | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > index 0ae2e3d8b832..65934984f032 100644 > --- a/tools/perf/Makefile.config > +++ b/tools/perf/Makefile.config > @@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected) > detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected) > > CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) > +HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) > > include $(srctree)/tools/scripts/Makefile.arch > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf > index 7df13e74450c..118bcdc70bb4 100644 > --- a/tools/perf/Makefile.perf > +++ b/tools/perf/Makefile.perf > @@ -226,7 +226,7 @@ else > endif > > export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK > -export HOSTCC HOSTLD HOSTAR > +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS > > include $(srctree)/tools/build/Makefile.include > > diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build > index a055dee6a46a..d5c287f069a2 100644 > --- a/tools/perf/pmu-events/Build > +++ b/tools/perf/pmu-events/Build > @@ -1,7 +1,7 @@ > hostprogs := jevents > > jevents-y += json.o jsmn.o jevents.o > -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include > +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS) > pmu-events-y += pmu-events.o > JDIR = pmu-events/arch/$(SRCARCH) > JSON = $(shell [ -d $(JDIR) ] && \ > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS 2021-10-18 10:41 ` James Clark @ 2021-10-19 8:37 ` John Garry 0 siblings, 0 replies; 11+ messages in thread From: John Garry @ 2021-10-19 8:37 UTC (permalink / raw) To: James Clark, peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo Cc: irogers, linux-perf-users, linux-kernel, kjain On 18/10/2021 11:41, James Clark wrote: > > > On 15/10/2021 17:48, John Garry wrote: >> Currently no compiler warnings at all are enabled for building jevents, >> so help catch bugs at compile time by enabling through HOSTCFLAGS. > Hi James, > Is there any reason to not enable -Wall and -Werror so that it builds like > the main project? Or if HOSTCFLAGS ends up being the same as CORE_CFLAGS > then why not use CORE_CFLAGS instead? I am not sure that we really want that, as CORE_CFLAGS brings with it things like _LARGEFILE64_SOURCE, which I doubt we want. > > I added them like this and only one unused function needs to be removed to > make it build successfully: > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > index 65934984f032..b2edcedf01db 100644 > --- a/tools/perf/Makefile.config > +++ b/tools/perf/Makefile.config > @@ -18,6 +18,8 @@ detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected) > > CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) > HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) > +HOSTCFLAGS += -Wall > +HOSTCFLAGS += -Wextra > > include $(srctree)/tools/scripts/Makefile.arch > > @@ -212,6 +214,7 @@ endif > ifneq ($(WERROR),0) > CORE_CFLAGS += -Werror > CXXFLAGS += -Werror > + HOSTCFLAGS += -Werror These seem fine to add. Actually what I have in HOSTCFLAGS doesn't seem to detect unused functions, with one example fixed in b94729919db2. Thanks, John ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] perf jevents: Enable build warnings 2021-10-15 16:48 [PATCH 0/2] perf jevents: Enable build warnings John Garry 2021-10-15 16:48 ` [PATCH 1/2] perf jevents: Fix some would-be warnings John Garry 2021-10-15 16:48 ` [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry @ 2021-10-20 14:31 ` Arnaldo Carvalho de Melo 2021-10-20 14:41 ` John Garry 2 siblings, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-20 14:31 UTC (permalink / raw) To: John Garry Cc: peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo, irogers, linux-perf-users, linux-kernel, kjain Em Sat, Oct 16, 2021 at 12:48:25AM +0800, John Garry escreveu: > Currently jevents builds without any complier warning flags enabled. So > use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS. I am not > 100% confident that this is the best way, but sending out for review. > > Baseline is be8ecc57f180 (HEAD, acme/perf/core) perf srcline: Use > long-running addr2line per DSO Thanks, applied. - Arnaldo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] perf jevents: Enable build warnings 2021-10-20 14:31 ` [PATCH 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo @ 2021-10-20 14:41 ` John Garry 2021-10-20 16:34 ` Arnaldo Carvalho de Melo 2021-10-20 17:25 ` Arnaldo Carvalho de Melo 0 siblings, 2 replies; 11+ messages in thread From: John Garry @ 2021-10-20 14:41 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo, irogers, linux-perf-users, linux-kernel, James Clark On 20/10/2021 15:31, Arnaldo Carvalho de Melo wrote: > Em Sat, Oct 16, 2021 at 12:48:25AM +0800, John Garry escreveu: >> Currently jevents builds without any complier warning flags enabled. So >> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS. I am not >> 100% confident that this is the best way, but sending out for review. >> >> Baseline is be8ecc57f180 (HEAD, acme/perf/core) perf srcline: Use >> long-running addr2line per DSO > > Thanks, applied. > Hi Arnaldo, I was going to send a v2, with changes according to James Clark's review - that was to add -Wall & -Werror, but they caused a problem on your perf/core branch as they triggered the warn fixed in commit b94729919db2. I suppose the best thing now is to send a patch on top once perf/core contains that commit. Let me know otherwise. Thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] perf jevents: Enable build warnings 2021-10-20 14:41 ` John Garry @ 2021-10-20 16:34 ` Arnaldo Carvalho de Melo 2021-10-20 17:25 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-20 16:34 UTC (permalink / raw) To: John Garry, Arnaldo Carvalho de Melo Cc: peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo, irogers, linux-perf-users, linux-kernel, James Clark On October 20, 2021 11:41:01 AM GMT-03:00, John Garry <john.garry@huawei.com> wrote: >On 20/10/2021 15:31, Arnaldo Carvalho de Melo wrote: >> Em Sat, Oct 16, 2021 at 12:48:25AM +0800, John Garry escreveu: >>> Currently jevents builds without any complier warning flags enabled. So >>> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS. I am not >>> 100% confident that this is the best way, but sending out for review. >>> >>> Baseline is be8ecc57f180 (HEAD, acme/perf/core) perf srcline: Use >>> long-running addr2line per DSO >> >> Thanks, applied. >> > >Hi Arnaldo, > >I was going to send a v2, with changes according to James Clark's review > - that was to add -Wall & -Werror, but they caused a problem on your >perf/core branch as they triggered the warn fixed in commit b94729919db2. > >I suppose the best thing now is to send a patch on top once perf/core >contains that commit. Let me know otherwise. Tests are running, if something fails, then I'll have to fix it and will not make my perf/core public, so I can replace what I have with a v2, otherwise you please send a patch on top after l make it public. - Arnaldo > >Thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] perf jevents: Enable build warnings 2021-10-20 14:41 ` John Garry 2021-10-20 16:34 ` Arnaldo Carvalho de Melo @ 2021-10-20 17:25 ` Arnaldo Carvalho de Melo 2021-10-21 20:31 ` [RFC] Support Intel-PT code build in 32-bit arches Arnaldo Carvalho de Melo 1 sibling, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-20 17:25 UTC (permalink / raw) To: John Garry, Adrian Hunter Cc: peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo, irogers, linux-perf-users, linux-kernel, James Clark Em Wed, Oct 20, 2021 at 03:41:01PM +0100, John Garry escreveu: > On 20/10/2021 15:31, Arnaldo Carvalho de Melo wrote: > > Em Sat, Oct 16, 2021 at 12:48:25AM +0800, John Garry escreveu: > > > Currently jevents builds without any complier warning flags enabled. So > > > use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS. I am not > > > 100% confident that this is the best way, but sending out for review. > > > > > > Baseline is be8ecc57f180 (HEAD, acme/perf/core) perf srcline: Use > > > long-running addr2line per DSO > > > > Thanks, applied. > I was going to send a v2, with changes according to James Clark's review - > that was to add -Wall & -Werror, but they caused a problem on your perf/core > branch as they triggered the warn fixed in commit b94729919db2. > I suppose the best thing now is to send a patch on top once perf/core > contains that commit. Let me know otherwise. You can send a v2, as: 29 8.60 debian:experimental-x-mipsel : FAIL gcc version 11.2.0 (Debian 11.2.0-9) util/intel-pt.c: In function 'intel_pt_synth_pebs_sample': util/intel-pt.c:2146:33: error: passing argument 1 of 'find_first_bit' from incompatible pointer type [-Werror=incompatible-pointer-types] 2146 | for_each_set_bit(hw_id, &items->applicable_counters, INTEL_PT_MAX_PEBS) { /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:37:38: note: in definition of macro 'for_each_set_bit' 37 | for ((bit) = find_first_bit((addr), (size)); \ | ^~~~ In file included from /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops.h:21, from /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:34, from /git/perf-5.15.0-rc4/tools/include/linux/bitmap.h:6, from util/header.h:10, from util/session.h:7, from util/intel-pt.c:16: /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops/find.h:109:51: note: expected 'const long unsigned int *' but argument is of type 'const uint64_t *' {aka 'const long long unsigned int *'} Adrian, this is on: commit 803a3c9233990e1adac8ea2421e3759c2d380cf8 Author: Adrian Hunter <adrian.hunter@intel.com> Date: Tue Sep 7 19:39:03 2021 +0300 perf intel-pt: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID Originally, software only supported redirecting at most one PEBS event to Intel PT (PEBS-via-PT) because it was not able to differentiate one event from another. To overcome that, add support for the PERF_RECORD_AUX_OUTPUT_HW_ID side-band event. Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> Reviewed-by: Andi Kleen <ak@linux.intel.com> That is still just on tmp.perf/core, so we can fix it, probably its just making that uint64_t into a unsigned long, will check later if you don't beat me to it. - Arnaldo ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC] Support Intel-PT code build in 32-bit arches 2021-10-20 17:25 ` Arnaldo Carvalho de Melo @ 2021-10-21 20:31 ` Arnaldo Carvalho de Melo 2021-10-22 12:23 ` Adrian Hunter 0 siblings, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-21 20:31 UTC (permalink / raw) To: Adrian Hunter Cc: John Garry, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo, irogers, linux-perf-users, linux-kernel, James Clark Em Wed, Oct 20, 2021 at 02:25:43PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, Oct 20, 2021 at 03:41:01PM +0100, John Garry escreveu: > > I suppose the best thing now is to send a patch on top once perf/core > > contains that commit. Let me know otherwise. > You can send a v2, as: > 29 8.60 debian:experimental-x-mipsel : FAIL gcc version 11.2.0 (Debian 11.2.0-9) > util/intel-pt.c: In function 'intel_pt_synth_pebs_sample': > util/intel-pt.c:2146:33: error: passing argument 1 of 'find_first_bit' from incompatible pointer type [-Werror=incompatible-pointer-types] > 2146 | for_each_set_bit(hw_id, &items->applicable_counters, INTEL_PT_MAX_PEBS) { > /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:37:38: note: in definition of macro 'for_each_set_bit' > 37 | for ((bit) = find_first_bit((addr), (size)); \ > | ^~~~ > In file included from /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops.h:21, > from /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:34, > from /git/perf-5.15.0-rc4/tools/include/linux/bitmap.h:6, > from util/header.h:10, > from util/session.h:7, > from util/intel-pt.c:16: > /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops/find.h:109:51: note: expected 'const long unsigned int *' but argument is of type 'const uint64_t *' {aka 'const long long unsigned int *'} > > Adrian, this is on: > > commit 803a3c9233990e1adac8ea2421e3759c2d380cf8 > Author: Adrian Hunter <adrian.hunter@intel.com> > Date: Tue Sep 7 19:39:03 2021 +0300 > > perf intel-pt: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID > > Originally, software only supported redirecting at most one PEBS event to > Intel PT (PEBS-via-PT) because it was not able to differentiate one event > from another. To overcome that, add support for the > PERF_RECORD_AUX_OUTPUT_HW_ID side-band event. > > Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Reviewed-by: Andi Kleen <ak@linux.intel.com> > > > That is still just on tmp.perf/core, so we can fix it, probably its just > making that uint64_t into a unsigned long, will check later if you don't > beat me to it. Adrian, Probably we should just disable Intel PT support from being built on 32-bit arches, right? I don't know if anybody is interested on that, my tests just try to figure out if it continues to build, and if fixing any problem isn't costly, which in this case is more than my threshold, wdyt? - Arnaldo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] Support Intel-PT code build in 32-bit arches 2021-10-21 20:31 ` [RFC] Support Intel-PT code build in 32-bit arches Arnaldo Carvalho de Melo @ 2021-10-22 12:23 ` Adrian Hunter 0 siblings, 0 replies; 11+ messages in thread From: Adrian Hunter @ 2021-10-22 12:23 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: John Garry, peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo, irogers, linux-perf-users, linux-kernel, James Clark On 21/10/2021 23:31, Arnaldo Carvalho de Melo wrote: > Em Wed, Oct 20, 2021 at 02:25:43PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Wed, Oct 20, 2021 at 03:41:01PM +0100, John Garry escreveu: >>> I suppose the best thing now is to send a patch on top once perf/core >>> contains that commit. Let me know otherwise. > >> You can send a v2, as: > >> 29 8.60 debian:experimental-x-mipsel : FAIL gcc version 11.2.0 (Debian 11.2.0-9) >> util/intel-pt.c: In function 'intel_pt_synth_pebs_sample': >> util/intel-pt.c:2146:33: error: passing argument 1 of 'find_first_bit' from incompatible pointer type [-Werror=incompatible-pointer-types] >> 2146 | for_each_set_bit(hw_id, &items->applicable_counters, INTEL_PT_MAX_PEBS) { >> /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:37:38: note: in definition of macro 'for_each_set_bit' >> 37 | for ((bit) = find_first_bit((addr), (size)); \ >> | ^~~~ >> In file included from /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops.h:21, >> from /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:34, >> from /git/perf-5.15.0-rc4/tools/include/linux/bitmap.h:6, >> from util/header.h:10, >> from util/session.h:7, >> from util/intel-pt.c:16: >> /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops/find.h:109:51: note: expected 'const long unsigned int *' but argument is of type 'const uint64_t *' {aka 'const long long unsigned int *'} >> >> Adrian, this is on: >> >> commit 803a3c9233990e1adac8ea2421e3759c2d380cf8 >> Author: Adrian Hunter <adrian.hunter@intel.com> >> Date: Tue Sep 7 19:39:03 2021 +0300 >> >> perf intel-pt: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID >> >> Originally, software only supported redirecting at most one PEBS event to >> Intel PT (PEBS-via-PT) because it was not able to differentiate one event >> from another. To overcome that, add support for the >> PERF_RECORD_AUX_OUTPUT_HW_ID side-band event. >> >> Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> >> Reviewed-by: Andi Kleen <ak@linux.intel.com> >> >> >> That is still just on tmp.perf/core, so we can fix it, probably its just >> making that uint64_t into a unsigned long, will check later if you don't >> beat me to it. > > Adrian, > > Probably we should just disable Intel PT support from being > built on 32-bit arches, right? I don't know if anybody is interested on > that, my tests just try to figure out if it continues to build, and if > fixing any problem isn't costly, which in this case is more than my > threshold, wdyt? It feels like bad form not to compile on 32-bit. In this case we could just cast it, and I see that has been done in other places for for_each_set_bit(), but (and maybe I've got it wrong) that wouldn't work for a 32-bit big endian system? The following might cover all cases, do you think? diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c index 1073c56a512c..7c979ffefade 100644 --- a/tools/perf/util/intel-pt.c +++ b/tools/perf/util/intel-pt.c @@ -2129,9 +2129,17 @@ static int intel_pt_synth_single_pebs_sample(struct intel_pt_queue *ptq) return intel_pt_do_synth_pebs_sample(ptq, evsel, id); } +/* For 32-bit big endian, put the longs from u64 in correct order */ +#define U64_BITS(var, val) \ + unsigned long var[] = { \ + [0] = (val), \ + [1] = (val) >> 32, \ + } + static int intel_pt_synth_pebs_sample(struct intel_pt_queue *ptq) { const struct intel_pt_blk_items *items = &ptq->state->items; + U64_BITS(ac_bits, items->applicable_counters); struct intel_pt_pebs_event *pe; struct intel_pt *pt = ptq->pt; int err = -EINVAL; @@ -2143,7 +2151,7 @@ static int intel_pt_synth_pebs_sample(struct intel_pt_queue *ptq) return intel_pt_synth_single_pebs_sample(ptq); } - for_each_set_bit(hw_id, &items->applicable_counters, INTEL_PT_MAX_PEBS) { + for_each_set_bit(hw_id, ac_bits, INTEL_PT_MAX_PEBS) { pe = &ptq->pebs[hw_id]; if (!pe->evsel) { if (!pt->single_pebs) ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-22 12:24 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-15 16:48 [PATCH 0/2] perf jevents: Enable build warnings John Garry 2021-10-15 16:48 ` [PATCH 1/2] perf jevents: Fix some would-be warnings John Garry 2021-10-15 16:48 ` [PATCH 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry 2021-10-18 10:41 ` James Clark 2021-10-19 8:37 ` John Garry 2021-10-20 14:31 ` [PATCH 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo 2021-10-20 14:41 ` John Garry 2021-10-20 16:34 ` Arnaldo Carvalho de Melo 2021-10-20 17:25 ` Arnaldo Carvalho de Melo 2021-10-21 20:31 ` [RFC] Support Intel-PT code build in 32-bit arches Arnaldo Carvalho de Melo 2021-10-22 12:23 ` Adrian Hunter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.