* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).