[PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Like Xu]

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

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Like Xu]

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
> 

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[John Garry]

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
> 

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[John Garry]

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!

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[Arnaldo Carvalho de Melo]

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

Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events

[John Garry]

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;

perf tools jevents build flags (was Re: [PATCH] perf jevents: Fix sys_event_tables to be freed like arch_std_events)

[John Garry]

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