* [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events
@ 2021-09-28 10:29 Like Xu
2021-09-28 11:52 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 11+ messages in thread
From: Like Xu @ 2021-09-28 10:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, John Garry
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel
From: Like Xu <likexu@tencent.com>
The compiler reports that free_sys_event_tables() is dead code. But
according to the semantics, the "LIST_HEAD(arch_std_events)" should
also be released, just like we do with 'arch_std_events' in the main().
Fixes: e9d32c1bf0cd7a98 ("perf vendor events: Add support for arch standard events")
Signed-off-by: Like Xu <likexu@tencent.com>
---
tools/perf/pmu-events/jevents.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 6731b3cf0c2f..7c887d37b893 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -1285,6 +1285,7 @@ int main(int argc, char *argv[])
}
free_arch_std_events();
+ free_sys_event_tables();
free(mapfile);
return 0;
@@ -1306,6 +1307,7 @@ int main(int argc, char *argv[])
create_empty_mapping(output_file);
err_out:
free_arch_std_events();
+ free_sys_event_tables();
free(mapfile);
return ret;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 10:29 [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events Like Xu @ 2021-09-28 11:52 ` Arnaldo Carvalho de Melo 2021-09-28 11:53 ` Like Xu 2021-09-28 12:49 ` John Garry 0 siblings, 2 replies; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-09-28 11:52 UTC (permalink / raw) To: Like Xu Cc: John Garry, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu: > From: Like Xu <likexu@tencent.com> > > The compiler reports that free_sys_event_tables() is dead code. But > according to the semantics, the "LIST_HEAD(arch_std_events)" should > also be released, just like we do with 'arch_std_events' in the main(). Thanks, applied. - Arnaldo > Fixes: e9d32c1bf0cd7a98 ("perf vendor events: Add support for arch standard events") > Signed-off-by: Like Xu <likexu@tencent.com> > --- > tools/perf/pmu-events/jevents.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c > index 6731b3cf0c2f..7c887d37b893 100644 > --- a/tools/perf/pmu-events/jevents.c > +++ b/tools/perf/pmu-events/jevents.c > @@ -1285,6 +1285,7 @@ int main(int argc, char *argv[]) > } > > free_arch_std_events(); > + free_sys_event_tables(); > free(mapfile); > return 0; > > @@ -1306,6 +1307,7 @@ int main(int argc, char *argv[]) > create_empty_mapping(output_file); > err_out: > free_arch_std_events(); > + free_sys_event_tables(); > free(mapfile); > return ret; > } > -- > 2.32.0 -- - Arnaldo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 11:52 ` Arnaldo Carvalho de Melo @ 2021-09-28 11:53 ` Like Xu 2021-09-28 12:37 ` Arnaldo Carvalho de Melo 2021-09-28 12:49 ` John Garry 1 sibling, 1 reply; 11+ messages in thread From: Like Xu @ 2021-09-28 11:53 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: John Garry, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel On 28/9/2021 7:52 pm, Arnaldo Carvalho de Melo wrote: > Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu: >> From: Like Xu <likexu@tencent.com> >> >> The compiler reports that free_sys_event_tables() is dead code. But >> according to the semantics, the "LIST_HEAD(arch_std_events)" should Sorry, s/arch_std_events/sys_event_tables/, please --amend. >> also be released, just like we do with 'arch_std_events' in the main(). > > Thanks, applied. > > - Arnaldo > > >> Fixes: e9d32c1bf0cd7a98 ("perf vendor events: Add support for arch standard events") >> Signed-off-by: Like Xu <likexu@tencent.com> >> --- >> tools/perf/pmu-events/jevents.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c >> index 6731b3cf0c2f..7c887d37b893 100644 >> --- a/tools/perf/pmu-events/jevents.c >> +++ b/tools/perf/pmu-events/jevents.c >> @@ -1285,6 +1285,7 @@ int main(int argc, char *argv[]) >> } >> >> free_arch_std_events(); >> + free_sys_event_tables(); >> free(mapfile); >> return 0; >> >> @@ -1306,6 +1307,7 @@ int main(int argc, char *argv[]) >> create_empty_mapping(output_file); >> err_out: >> free_arch_std_events(); >> + free_sys_event_tables(); >> free(mapfile); >> return ret; >> } >> -- >> 2.32.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 11:53 ` Like Xu @ 2021-09-28 12:37 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-09-28 12:37 UTC (permalink / raw) To: Like Xu Cc: Arnaldo Carvalho de Melo, John Garry, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel Em Tue, Sep 28, 2021 at 07:53:54PM +0800, Like Xu escreveu: > On 28/9/2021 7:52 pm, Arnaldo Carvalho de Melo wrote: > > Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu: > > > From: Like Xu <likexu@tencent.com> > > > > > > The compiler reports that free_sys_event_tables() is dead code. But > > > according to the semantics, the "LIST_HEAD(arch_std_events)" should > Sorry, s/arch_std_events/sys_event_tables/, please --amend. sure > > > also be released, just like we do with 'arch_std_events' in the main(). > > > > Thanks, applied. > > > > - Arnaldo > > > > > Fixes: e9d32c1bf0cd7a98 ("perf vendor events: Add support for arch standard events") > > > Signed-off-by: Like Xu <likexu@tencent.com> > > > --- > > > tools/perf/pmu-events/jevents.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c > > > index 6731b3cf0c2f..7c887d37b893 100644 > > > --- a/tools/perf/pmu-events/jevents.c > > > +++ b/tools/perf/pmu-events/jevents.c > > > @@ -1285,6 +1285,7 @@ int main(int argc, char *argv[]) > > > } > > > free_arch_std_events(); > > > + free_sys_event_tables(); > > > free(mapfile); > > > return 0; > > > @@ -1306,6 +1307,7 @@ int main(int argc, char *argv[]) > > > create_empty_mapping(output_file); > > > err_out: > > > free_arch_std_events(); > > > + free_sys_event_tables(); > > > free(mapfile); > > > return ret; > > > } > > > -- > > > 2.32.0 > > -- - Arnaldo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 11:52 ` Arnaldo Carvalho de Melo 2021-09-28 11:53 ` Like Xu @ 2021-09-28 12:49 ` John Garry 2021-09-28 13:16 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 11+ messages in thread From: John Garry @ 2021-09-28 12:49 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Like Xu Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel On 28/09/2021 12:52, Arnaldo Carvalho de Melo wrote: > Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu: >> From: Like Xu <likexu@tencent.com> >> >> The compiler reports that free_sys_event_tables() is dead code. But >> according to the semantics, the "LIST_HEAD(arch_std_events)" should >> also be released, just like we do with 'arch_std_events' in the main(). > > Thanks, applied. > > - Arnaldo > If not too late: Reviewed-by: John Garry <john.garry@huawei.com> I think that it could be a good idea to raise gcc warning level to detect unused static functions, like this was thanks > >> Fixes: e9d32c1bf0cd7a98 ("perf vendor events: Add support for arch standard events") >> Signed-off-by: Like Xu <likexu@tencent.com> >> --- >> tools/perf/pmu-events/jevents.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c >> index 6731b3cf0c2f..7c887d37b893 100644 >> --- a/tools/perf/pmu-events/jevents.c >> +++ b/tools/perf/pmu-events/jevents.c >> @@ -1285,6 +1285,7 @@ int main(int argc, char *argv[]) >> } >> >> free_arch_std_events(); >> + free_sys_event_tables(); >> free(mapfile); >> return 0; >> >> @@ -1306,6 +1307,7 @@ int main(int argc, char *argv[]) >> create_empty_mapping(output_file); >> err_out: >> free_arch_std_events(); >> + free_sys_event_tables(); >> free(mapfile); >> return ret; >> } >> -- >> 2.32.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 12:49 ` John Garry @ 2021-09-28 13:16 ` Arnaldo Carvalho de Melo 2021-09-28 13:22 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-09-28 13:16 UTC (permalink / raw) To: John Garry Cc: Like Xu, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel Em Tue, Sep 28, 2021 at 01:49:20PM +0100, John Garry escreveu: > On 28/09/2021 12:52, Arnaldo Carvalho de Melo wrote: > > Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu: > > > From: Like Xu <likexu@tencent.com> > > > > > > The compiler reports that free_sys_event_tables() is dead code. But > > > according to the semantics, the "LIST_HEAD(arch_std_events)" should > > > also be released, just like we do with 'arch_std_events' in the main(). > > > > Thanks, applied. > > > > - Arnaldo > > > > If not too late: > Reviewed-by: John Garry <john.garry@huawei.com> Not too late, collected. > I think that it could be a good idea to raise gcc warning level to detect > unused static functions, like this was Agreed, but we already have: CORE_CFLAGS += -Wall CORE_CFLAGS += -Wextra We can se it for this specific case with: $ make V=1 -k BUILD_BPF_SKEL=1 CORESIGHT=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin | grep jevents make -f /var/home/acme/git/perf/tools/build/Makefile.build dir=pmu-events obj=jevents gcc -Wp,-MD,/tmp/build/perf/pmu-events/.jevents.o.d -Wp,-MT,/tmp/build/perf/pmu-events/jevents.o -D"BUILD_STR(s)=#s" -I/var/home/acme/git/perf/tools/include -c -o /tmp/build/perf/pmu-events/jevents.o pmu-events/jevents.c ld -r -o /tmp/build/perf/pmu-events/jevents-in.o /tmp/build/perf/pmu-events/json.o /tmp/build/perf/pmu-events/jsmn.o /tmp/build/perf/pmu-events/jevents.o gcc /tmp/build/perf/pmu-events/jevents-in.o -o /tmp/build/perf/pmu-events/jevents /tmp/build/perf/pmu-events/jevents x86 pmu-events/arch /tmp/build/perf/pmu-events/pmu-events.c 1 jevents: Processing mapfile pmu-events/arch/x86/mapfile.csv Humm... no "-Wall -Wextra" there... lemme try to fix it - Arnaldo > thanks > > > > Fixes: e9d32c1bf0cd7a98 ("perf vendor events: Add support for arch standard events") > > > Signed-off-by: Like Xu <likexu@tencent.com> > > > --- > > > tools/perf/pmu-events/jevents.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c > > > index 6731b3cf0c2f..7c887d37b893 100644 > > > --- a/tools/perf/pmu-events/jevents.c > > > +++ b/tools/perf/pmu-events/jevents.c > > > @@ -1285,6 +1285,7 @@ int main(int argc, char *argv[]) > > > } > > > free_arch_std_events(); > > > + free_sys_event_tables(); > > > free(mapfile); > > > return 0; > > > @@ -1306,6 +1307,7 @@ int main(int argc, char *argv[]) > > > create_empty_mapping(output_file); > > > err_out: > > > free_arch_std_events(); > > > + free_sys_event_tables(); > > > free(mapfile); > > > return ret; > > > } > > > -- > > > 2.32.0 > > -- - Arnaldo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 13:16 ` Arnaldo Carvalho de Melo @ 2021-09-28 13:22 ` Arnaldo Carvalho de Melo 2021-09-28 13:32 ` John Garry 0 siblings, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-09-28 13:22 UTC (permalink / raw) To: John Garry Cc: Like Xu, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel Em Tue, Sep 28, 2021 at 10:16:48AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Tue, Sep 28, 2021 at 01:49:20PM +0100, John Garry escreveu: > > On 28/09/2021 12:52, Arnaldo Carvalho de Melo wrote: > > > Em Tue, Sep 28, 2021 at 06:29:38PM +0800, Like Xu escreveu: > > > > From: Like Xu <likexu@tencent.com> > > > > > > > > The compiler reports that free_sys_event_tables() is dead code. But > > > > according to the semantics, the "LIST_HEAD(arch_std_events)" should > > > > also be released, just like we do with 'arch_std_events' in the main(). > > > > > > Thanks, applied. > > > > > > - Arnaldo > > > > > > > If not too late: > > Reviewed-by: John Garry <john.garry@huawei.com> > > Not too late, collected. > > > I think that it could be a good idea to raise gcc warning level to detect > > unused static functions, like this was > > Agreed, but we already have: > > CORE_CFLAGS += -Wall > CORE_CFLAGS += -Wextra > > We can se it for this specific case with: > > $ make V=1 -k BUILD_BPF_SKEL=1 CORESIGHT=1 PYTHON=python3 O=/tmp/build/perf -C tools/perf install-bin | grep jevents > make -f /var/home/acme/git/perf/tools/build/Makefile.build dir=pmu-events obj=jevents > gcc -Wp,-MD,/tmp/build/perf/pmu-events/.jevents.o.d -Wp,-MT,/tmp/build/perf/pmu-events/jevents.o -D"BUILD_STR(s)=#s" -I/var/home/acme/git/perf/tools/include -c -o /tmp/build/perf/pmu-events/jevents.o pmu-events/jevents.c > ld -r -o /tmp/build/perf/pmu-events/jevents-in.o /tmp/build/perf/pmu-events/json.o /tmp/build/perf/pmu-events/jsmn.o /tmp/build/perf/pmu-events/jevents.o > gcc /tmp/build/perf/pmu-events/jevents-in.o -o /tmp/build/perf/pmu-events/jevents > /tmp/build/perf/pmu-events/jevents x86 pmu-events/arch /tmp/build/perf/pmu-events/pmu-events.c 1 > jevents: Processing mapfile pmu-events/arch/x86/mapfile.csv > > Humm... no "-Wall -Wextra" there... lemme try to fix it With this: ⬢[acme@toolbox perf]$ git diff diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build index a055dee6a46af77e..ea7107630bf4327f 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 -Wall -Wextra pmu-events-y += pmu-events.o JDIR = pmu-events/arch/$(SRCARCH) JSON = $(shell [ -d $(JDIR) ] && \ ⬢[acme@toolbox perf]$ I get this before applying Xu's patch: LINK /tmp/build/perf/libbpf.a pmu-events/jevents.c: In function ‘save_arch_std_events’: pmu-events/jevents.c:473:39: warning: unused parameter ‘data’ [-Wunused-parameter] 473 | static int save_arch_std_events(void *data, struct json_event *je) | ~~~~~~^~~~ At top level: pmu-events/jevents.c:93:13: warning: ‘free_sys_event_tables’ defined but not used [-Wunused-function] 93 | static void free_sys_event_tables(void) | ^~~~~~~~~~~~~~~~~~~~~ ------------------------------------- I'll add this to perf/core, as this isn't a strict fix, so can wait for v5.16. - Arnaldo ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 13:22 ` Arnaldo Carvalho de Melo @ 2021-09-28 13:32 ` John Garry 2021-09-28 17:56 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 11+ messages in thread From: John Garry @ 2021-09-28 13:32 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Like Xu, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel On 28/09/2021 14:22, Arnaldo Carvalho de Melo wrote: > jevents-y += json.o jsmn.o jevents.o > -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include > +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include -Wall -Wextra > pmu-events-y += pmu-events.o > JDIR = pmu-events/arch/$(SRCARCH) > JSON = $(shell [ -d $(JDIR) ] && \ > ⬢[acme@toolbox perf]$ > > I get this before applying Xu's patch: > > LINK /tmp/build/perf/libbpf.a > pmu-events/jevents.c: In function ‘save_arch_std_events’: > pmu-events/jevents.c:473:39: warning: unused parameter ‘data’ [-Wunused-parameter] > 473 | static int save_arch_std_events(void *data, struct json_event *je) > | ~~~~~~^~~~ > At top level: > pmu-events/jevents.c:93:13: warning: ‘free_sys_event_tables’ defined but not used [-Wunused-function] > 93 | static void free_sys_event_tables(void) > | ^~~~~~~~~~~~~~~~~~~~~ > > > ------------------------------------- > > I'll add this to perf/core, as this isn't a strict fix, so can wait for > v5.16. Hi Arnaldo, OK, would you also consider reusing CFLAGS: --- a/tools/perf/pmu-events/Build +++ b/tools/perf/pmu-events/Build @@ -9,10 +9,12 @@ JSON = $(shell [ -d $(JDIR) ] && \ JDIR_TEST = pmu-events/arch/test JSON_TEST = $(shell [ -d $(JDIR_TEST) ] && \ find $(JDIR_TEST) -name '*.json') - +HOSTCFLAGS_jevents += $(CFLAGS) I tried it, and there are more things to fix for jevents.o. Let me know your preference and if any help required to fix any errors up. Thanks! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 13:32 ` John Garry @ 2021-09-28 17:56 ` Arnaldo Carvalho de Melo 2021-09-28 20:30 ` John Garry 0 siblings, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-09-28 17:56 UTC (permalink / raw) To: John Garry Cc: Like Xu, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel Em Tue, Sep 28, 2021 at 02:32:02PM +0100, John Garry escreveu: > On 28/09/2021 14:22, Arnaldo Carvalho de Melo wrote: > > jevents-y += json.o jsmn.o jevents.o > > -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include > > +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include -Wall -Wextra > > pmu-events-y += pmu-events.o > > JDIR = pmu-events/arch/$(SRCARCH) > > JSON = $(shell [ -d $(JDIR) ] && \ > > ⬢[acme@toolbox perf]$ > > > > I get this before applying Xu's patch: > > > > LINK /tmp/build/perf/libbpf.a > > pmu-events/jevents.c: In function ‘save_arch_std_events’: > > pmu-events/jevents.c:473:39: warning: unused parameter ‘data’ [-Wunused-parameter] > > 473 | static int save_arch_std_events(void *data, struct json_event *je) > > | ~~~~~~^~~~ > > At top level: > > pmu-events/jevents.c:93:13: warning: ‘free_sys_event_tables’ defined but not used [-Wunused-function] > > 93 | static void free_sys_event_tables(void) > > | ^~~~~~~~~~~~~~~~~~~~~ > > > > > > ------------------------------------- > > > > I'll add this to perf/core, as this isn't a strict fix, so can wait for > > v5.16. > > Hi Arnaldo, > > OK, would you also consider reusing CFLAGS: > > --- a/tools/perf/pmu-events/Build > +++ b/tools/perf/pmu-events/Build > @@ -9,10 +9,12 @@ JSON = $(shell [ -d $(JDIR) ] && > \ > JDIR_TEST = pmu-events/arch/test > JSON_TEST = $(shell [ -d $(JDIR_TEST) ] && \ > find $(JDIR_TEST) -name '*.json') > - > +HOSTCFLAGS_jevents += $(CFLAGS) Humm, we have to check if CFLAGS doesn't come with cross-build options, i.e. IIRC we have to use HOSTCFLAGS instead. Unsure if there is some *CFLAGS variable that gets the common part, where these -Wall and -Wextra, -Werror could go. > I tried it, and there are more things to fix for jevents.o. Let me know your > preference and if any help required to fix any errors up. I fixed the one I found, see below, I'll test build what I have in perf/core and push it, then you can continue from there, after checking this HOSTCFLAGS/CFLAGS issue. - Arnaldo From 0e46c8307574a8e2dac8d7ba97e0f6f4bbee67a5 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo <acme@redhat.com> Date: Tue, 28 Sep 2021 14:15:01 -0300 Subject: [PATCH 1/1] perf jevents: Add __maybe_unused attribute to unused function arg The tools/perf/pmu-events/jevents.c file isn't being compiled with -Werror and -Wextra, which will be the case soon, so before we turn those compiler flags on, fix what it would flag. Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Like Xu <like.xu.linux@gmail.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> To: John Garry <john.garry@huawei.com> --- tools/perf/pmu-events/jevents.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c index 6731b3cf0c2fc9b7..323e1dfe2436c049 100644 --- a/tools/perf/pmu-events/jevents.c +++ b/tools/perf/pmu-events/jevents.c @@ -45,6 +45,7 @@ #include <sys/resource.h> /* getrlimit */ #include <ftw.h> #include <sys/stat.h> +#include <linux/compiler.h> #include <linux/list.h> #include "jsmn.h" #include "json.h" @@ -470,7 +471,7 @@ static void free_arch_std_events(void) } } -static int save_arch_std_events(void *data, struct json_event *je) +static int save_arch_std_events(void *data __maybe_unused, struct json_event *je) { struct event_struct *es; -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events 2021-09-28 17:56 ` Arnaldo Carvalho de Melo @ 2021-09-28 20:30 ` John Garry 2021-10-11 17:03 ` perf tools jevents build flags (was Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events) John Garry 0 siblings, 1 reply; 11+ messages in thread From: John Garry @ 2021-09-28 20:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Like Xu, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel On 28/09/2021 18:56, Arnaldo Carvalho de Melo wrote: >>> ‘save_arch_std_events’: >>> pmu-events/jevents.c:473:39: warning: unused parameter ‘data’ [-Wunused-parameter] >>> 473 | static int save_arch_std_events(void *data, struct json_event *je) >>> | ~~~~~~^~~~ >>> At top level: >>> pmu-events/jevents.c:93:13: warning: ‘free_sys_event_tables’ defined but not used [-Wunused-function] >>> 93 | static void free_sys_event_tables(void) >>> | ^~~~~~~~~~~~~~~~~~~~~ >>> >>> >>> ------------------------------------- >>> >>> I'll add this to perf/core, as this isn't a strict fix, so can wait for >>> v5.16. >> Hi Arnaldo, >> >> OK, would you also consider reusing CFLAGS: >> >> --- a/tools/perf/pmu-events/Build >> +++ b/tools/perf/pmu-events/Build >> @@ -9,10 +9,12 @@ JSON = $(shell [ -d $(JDIR) ] && >> \ >> JDIR_TEST = pmu-events/arch/test >> JSON_TEST = $(shell [ -d $(JDIR_TEST) ] && \ >> find $(JDIR_TEST) -name '*.json') >> - >> +HOSTCFLAGS_jevents += $(CFLAGS) > Humm, we have to check if CFLAGS doesn't come with cross-build options, > i.e. IIRC we have to use HOSTCFLAGS instead. Unsure if there is some > *CFLAGS variable that gets the common part, where these -Wall and > -Wextra, -Werror could go. not sure. As I see, the bulk of flags we have in CFLAGS comes from EXTRA_WARNINGS in scripts/Makefile.include; but CFLAGS seems to also include EXTRA_CLAGS, which are for cross-builds (see perf/Makefile.config) So maybe we just need to use EXTRA_WARNINGS for HOST_CFLAGS > >> I tried it, and there are more things to fix for jevents.o. Let me know your >> preference and if any help required to fix any errors up. > I fixed the one I found, see below, I'll test build what I have in > perf/core and push it, then you can continue from there, after checking > this HOSTCFLAGS/CFLAGS issue. > > - Arnaldo > >>From 0e46c8307574a8e2dac8d7ba97e0f6f4bbee67a5 Mon Sep 17 00:00:00 2001 > From: Arnaldo Carvalho de Melo<acme@redhat.com> > Date: Tue, 28 Sep 2021 14:15:01 -0300 > Subject: [PATCH 1/1] perf jevents: Add __maybe_unused attribute to unused > function arg > > The tools/perf/pmu-events/jevents.c file isn't being compiled with > -Werror and -Wextra, which will be the case soon, so before we turn > those compiler flags on, fix what it would flag. > > Cc: Alexander Shishkin<alexander.shishkin@linux.intel.com> > Cc: Ingo Molnar<mingo@redhat.com> > Cc: Jiri Olsa<jolsa@redhat.com> > Cc: Like Xu<like.xu.linux@gmail.com> > Cc: Mark Rutland<mark.rutland@arm.com> > Cc: Namhyung Kim<namhyung@kernel.org> > Cc: Peter Zijlstra<peterz@infradead.org> > Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com> > To: John Garry<john.garry@huawei.com> thanks. Reviewed-by: John Garry <john.garry@huawei.com> > --- > tools/perf/pmu-events/jevents.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c > index 6731b3cf0c2fc9b7..323e1dfe2436c049 100644 > --- a/tools/perf/pmu-events/jevents.c > +++ b/tools/perf/pmu-events/jevents.c > @@ -45,6 +45,7 @@ > #include <sys/resource.h> /* getrlimit */ > #include <ftw.h> > #include <sys/stat.h> > +#include <linux/compiler.h> > #include <linux/list.h> > #include "jsmn.h" > #include "json.h" > @@ -470,7 +471,7 @@ static void free_arch_std_events(void) > } > } > > -static int save_arch_std_events(void *data, struct json_event *je) > +static int save_arch_std_events(void *data __maybe_unused, struct json_event *je) > { > struct event_struct *es; ^ permalink raw reply [flat|nested] 11+ messages in thread
* perf tools jevents build flags (was Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events) 2021-09-28 20:30 ` John Garry @ 2021-10-11 17:03 ` John Garry 0 siblings, 0 replies; 11+ messages in thread From: John Garry @ 2021-10-11 17:03 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Like Xu, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel On 28/09/2021 21:30, John Garry wrote: >>> >>> OK, would you also consider reusing CFLAGS: >>> >>> --- a/tools/perf/pmu-events/Build >>> +++ b/tools/perf/pmu-events/Build >>> @@ -9,10 +9,12 @@ JSON = $(shell [ -d $(JDIR) ] && >>> \ >>> JDIR_TEST = pmu-events/arch/test >>> JSON_TEST = $(shell [ -d $(JDIR_TEST) ] && \ >>> find $(JDIR_TEST) -name '*.json') >>> - >>> +HOSTCFLAGS_jevents += $(CFLAGS) >> Humm, we have to check if CFLAGS doesn't come with cross-build options, >> i.e. IIRC we have to use HOSTCFLAGS instead. Unsure if there is some >> *CFLAGS variable that gets the common part, where these -Wall and >> -Wextra, -Werror could go. > > not sure. As I see, the bulk of flags we have in CFLAGS comes from > EXTRA_WARNINGS in scripts/Makefile.include; but CFLAGS seems to also > include EXTRA_CLAGS, which are for cross-builds (see perf/Makefile.config) > Hi Arnaldo, I'm just looking at enabling warning cflags for jevents again. So how about this: --->8---- Subject: [PATCH] perf pmu-events: Enable jevents warnings through HOSTCFLAGS 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 ---8<--- The newly generated warnings in jevents.c are pretty straightforward to tidy up. Thanks, John ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-11 17:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-28 10:29 [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events Like Xu 2021-09-28 11:52 ` Arnaldo Carvalho de Melo 2021-09-28 11:53 ` Like Xu 2021-09-28 12:37 ` Arnaldo Carvalho de Melo 2021-09-28 12:49 ` John Garry 2021-09-28 13:16 ` Arnaldo Carvalho de Melo 2021-09-28 13:22 ` Arnaldo Carvalho de Melo 2021-09-28 13:32 ` John Garry 2021-09-28 17:56 ` Arnaldo Carvalho de Melo 2021-09-28 20:30 ` John Garry 2021-10-11 17:03 ` perf tools jevents build flags (was Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events) John Garry
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.