linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] perf jevents: Enable build warnings
@ 2021-10-21  9:16 John Garry
  2021-10-21  9:16 ` [PATCH v2 1/2] perf jevents: Fix some would-be warnings John Garry
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: John Garry @ 2021-10-21  9:16 UTC (permalink / raw)
  To: peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo
  Cc: irogers, linux-perf-users, linux-kernel, kjain, james.clark, John Garry

Currently jevents builds without any complier warning flags enabled. So
use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.

Changes for v2:
- Add Werror, Wall, and Wextra (James Clark suggestion)

Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
Free the sys_event_tables list after processing entries"):

680453e63302 perf jevents: Free the sys_event_tables list after processing entries
be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols

John Garry (2):
  perf jevents: Fix some would-be warnings
  perf jevents: Enable warnings through HOSTCFLAGS

 tools/perf/Makefile.config      |  5 +++++
 tools/perf/Makefile.perf        |  2 +-
 tools/perf/pmu-events/Build     |  2 +-
 tools/perf/pmu-events/jevents.c | 10 ++++------
 4 files changed, 11 insertions(+), 8 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/2] perf jevents: Fix some would-be warnings
  2021-10-21  9:16 [PATCH v2 0/2] perf jevents: Enable build warnings John Garry
@ 2021-10-21  9:16 ` John Garry
  2021-10-21  9:16 ` [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry
  2021-10-21 12:55 ` [PATCH v2 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 15+ messages in thread
From: John Garry @ 2021-10-21  9:16 UTC (permalink / raw)
  To: peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo
  Cc: irogers, linux-perf-users, linux-kernel, kjain, james.clark, 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 b7c8aae6bcf1..70eacc22dfef 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.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-21  9:16 [PATCH v2 0/2] perf jevents: Enable build warnings John Garry
  2021-10-21  9:16 ` [PATCH v2 1/2] perf jevents: Fix some would-be warnings John Garry
@ 2021-10-21  9:16 ` John Garry
  2021-10-21 12:48   ` Jiri Olsa
  2021-10-21 12:55 ` [PATCH v2 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 15+ messages in thread
From: John Garry @ 2021-10-21  9:16 UTC (permalink / raw)
  To: peterz, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo
  Cc: irogers, linux-perf-users, linux-kernel, kjain, james.clark, 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  | 5 +++++
 tools/perf/Makefile.perf    | 2 +-
 tools/perf/pmu-events/Build | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0ae2e3d8b832..374f65b52157 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
 
@@ -211,6 +212,7 @@ endif
 ifneq ($(WERROR),0)
   CORE_CFLAGS += -Werror
   CXXFLAGS += -Werror
+  HOSTCFLAGS += -Werror
 endif
 
 ifndef DEBUG
@@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
 CXXFLAGS += -funwind-tables
 CXXFLAGS += -Wno-strict-aliasing
 
+HOSTCFLAGS += -Wall
+HOSTCFLAGS += -Wextra
+
 # Enforce a non-executable stack, as we may regress (again) in the future by
 # adding assembler files missing the .GNU-stack linker note.
 LDFLAGS += -Wl,-z,noexecstack
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.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-21  9:16 ` [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry
@ 2021-10-21 12:48   ` Jiri Olsa
  2021-10-21 15:50     ` John Garry
  2021-10-22  9:42     ` John Garry
  0 siblings, 2 replies; 15+ messages in thread
From: Jiri Olsa @ 2021-10-21 12:48 UTC (permalink / raw)
  To: John Garry
  Cc: peterz, acme, mark.rutland, alexander.shishkin, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On Thu, Oct 21, 2021 at 05:16:45PM +0800, 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.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/Makefile.config  | 5 +++++
>  tools/perf/Makefile.perf    | 2 +-
>  tools/perf/pmu-events/Build | 2 +-
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 0ae2e3d8b832..374f65b52157 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
>  
> @@ -211,6 +212,7 @@ endif
>  ifneq ($(WERROR),0)
>    CORE_CFLAGS += -Werror
>    CXXFLAGS += -Werror
> +  HOSTCFLAGS += -Werror
>  endif
>  
>  ifndef DEBUG
> @@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
>  CXXFLAGS += -funwind-tables
>  CXXFLAGS += -Wno-strict-aliasing
>  
> +HOSTCFLAGS += -Wall
> +HOSTCFLAGS += -Wextra
> +
>  # Enforce a non-executable stack, as we may regress (again) in the future by
>  # adding assembler files missing the .GNU-stack linker note.
>  LDFLAGS += -Wl,-z,noexecstack
> 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)

so the the host cflags are made of:

host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))

can't you use KBUILD_HOSTCFLAGS?

also perhaps we could rename KBUILD_HOSTCFLAGS to HOSTCFLAGS?
the name seems like leftover from kbuild move

jirka

>  pmu-events-y	+= pmu-events.o
>  JDIR		=  pmu-events/arch/$(SRCARCH)
>  JSON		=  $(shell [ -d $(JDIR) ] &&				\
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] perf jevents: Enable build warnings
  2021-10-21  9:16 [PATCH v2 0/2] perf jevents: Enable build warnings John Garry
  2021-10-21  9:16 ` [PATCH v2 1/2] perf jevents: Fix some would-be warnings John Garry
  2021-10-21  9:16 ` [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry
@ 2021-10-21 12:55 ` Arnaldo Carvalho de Melo
  2021-10-21 13:02   ` John Garry
  2 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-10-21 12:55 UTC (permalink / raw)
  To: John Garry
  Cc: peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

Em Thu, Oct 21, 2021 at 05:16:43PM +0800, John Garry escreveu:
> Currently jevents builds without any complier warning flags enabled. So
> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
> 
> Changes for v2:
> - Add Werror, Wall, and Wextra (James Clark suggestion)

Thanks, applied.

- Arnaldo

 
> Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
> Free the sys_event_tables list after processing entries"):
> 
> 680453e63302 perf jevents: Free the sys_event_tables list after processing entries
> be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
> 2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
> 
> John Garry (2):
>   perf jevents: Fix some would-be warnings
>   perf jevents: Enable warnings through HOSTCFLAGS
> 
>  tools/perf/Makefile.config      |  5 +++++
>  tools/perf/Makefile.perf        |  2 +-
>  tools/perf/pmu-events/Build     |  2 +-
>  tools/perf/pmu-events/jevents.c | 10 ++++------
>  4 files changed, 11 insertions(+), 8 deletions(-)
> 
> -- 
> 2.17.1

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] perf jevents: Enable build warnings
  2021-10-21 12:55 ` [PATCH v2 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo
@ 2021-10-21 13:02   ` John Garry
  2021-10-21 14:36     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: John Garry @ 2021-10-21 13:02 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mark.rutland, alexander.shishkin, jolsa, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On 21/10/2021 13:55, Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 21, 2021 at 05:16:43PM +0800, John Garry escreveu:
>> Currently jevents builds without any complier warning flags enabled. So
>> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
>>
>> Changes for v2:
>> - Add Werror, Wall, and Wextra (James Clark suggestion)
> 
> Thanks, applied.
> 

Hi Arnaldo,

Can you please wait until I check the review response from jirka?

Thanks

> - Arnaldo
> 
>   
>> Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
>> Free the sys_event_tables list after processing entries"):
>>
>> 680453e63302 perf jevents: Free the sys_event_tables list after processing entries
>> be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
>> 2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
>>
>> John Garry (2):
>>    perf jevents: Fix some would-be warnings
>>    perf jevents: Enable warnings through HOSTCFLAGS
>>
>>   tools/perf/Makefile.config      |  5 +++++
>>   tools/perf/Makefile.perf        |  2 +-
>>   tools/perf/pmu-events/Build     |  2 +-
>>   tools/perf/pmu-events/jevents.c | 10 ++++------
>>   4 files changed, 11 insertions(+), 8 deletions(-)
>>
>> -- 
>> 2.17.1
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/2] perf jevents: Enable build warnings
  2021-10-21 13:02   ` John Garry
@ 2021-10-21 14:36     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-10-21 14:36 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, kjain, james.clark



On October 21, 2021 10:02:41 AM GMT-03:00, John Garry <john.garry@huawei.com> wrote:
>On 21/10/2021 13:55, Arnaldo Carvalho de Melo wrote:
>> Em Thu, Oct 21, 2021 at 05:16:43PM +0800, John Garry escreveu:
>>> Currently jevents builds without any complier warning flags enabled. So
>>> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
>>>
>>> Changes for v2:
>>> - Add Werror, Wall, and Wextra (James Clark suggestion)
>> 
>> Thanks, applied.
>> 
>
>Hi Arnaldo,
>
>Can you please wait until I check the review response from jirka?

Sure, it's not published, and will not be until some fixes are made. Having it in my local tree gets it tested together with the whole shebang in lots of containers.

I collect reviewed-by, etc as they appear.

- Arnaldo

>> 
>>   
>>> Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
>>> Free the sys_event_tables list after processing entries"):
>>>
>>> 680453e63302 perf jevents: Free the sys_event_tables list after processing entries
>>> be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
>>> 2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
>>>
>>> John Garry (2):
>>>    perf jevents: Fix some would-be warnings
>>>    perf jevents: Enable warnings through HOSTCFLAGS
>>>
>>>   tools/perf/Makefile.config      |  5 +++++
>>>   tools/perf/Makefile.perf        |  2 +-
>>>   tools/perf/pmu-events/Build     |  2 +-
>>>   tools/perf/pmu-events/jevents.c | 10 ++++------
>>>   4 files changed, 11 insertions(+), 8 deletions(-)
>>>
>>> -- 
>>> 2.17.1
>> 
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-21 12:48   ` Jiri Olsa
@ 2021-10-21 15:50     ` John Garry
  2021-10-21 15:58       ` Jiri Olsa
  2021-10-22  9:42     ` John Garry
  1 sibling, 1 reply; 15+ messages in thread
From: John Garry @ 2021-10-21 15:50 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: peterz, acme, mark.rutland, alexander.shishkin, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On 21/10/2021 13:48, Jiri Olsa wrote:
>> +HOSTCFLAGS += -Wall
>> +HOSTCFLAGS += -Wextra
>> +
>>   # Enforce a non-executable stack, as we may regress (again) in the future by
>>   # adding assembler files missing the .GNU-stack linker note.
>>   LDFLAGS += -Wl,-z,noexecstack
>> 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)
> so the the host cflags are made of:
> 
> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> 
> can't you use KBUILD_HOSTCFLAGS?

Maybe. However, it seems to be empty when building perf/pmu-events. Even 
in building tools/objtool - which currently references it - it is empty 
AFAICS. I'm not sure if it's being imported properly.

Thanks,
John

> 
> also perhaps we could rename KBUILD_HOSTCFLAGS to HOSTCFLAGS?
> the name seems like leftover from kbuild move


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-21 15:50     ` John Garry
@ 2021-10-21 15:58       ` Jiri Olsa
  0 siblings, 0 replies; 15+ messages in thread
From: Jiri Olsa @ 2021-10-21 15:58 UTC (permalink / raw)
  To: John Garry
  Cc: peterz, acme, mark.rutland, alexander.shishkin, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On Thu, Oct 21, 2021 at 04:50:09PM +0100, John Garry wrote:
> On 21/10/2021 13:48, Jiri Olsa wrote:
> > > +HOSTCFLAGS += -Wall
> > > +HOSTCFLAGS += -Wextra
> > > +
> > >   # Enforce a non-executable stack, as we may regress (again) in the future by
> > >   # adding assembler files missing the .GNU-stack linker note.
> > >   LDFLAGS += -Wl,-z,noexecstack
> > > 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)
> > so the the host cflags are made of:
> > 
> > host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> > 
> > can't you use KBUILD_HOSTCFLAGS?
> 
> Maybe. However, it seems to be empty when building perf/pmu-events. Even in
> building tools/objtool - which currently references it - it is empty AFAICS.
> I'm not sure if it's being imported properly.
> 

I meant change like this (on top of yours)

jirka


---
diff --git a/tools/build/Build.include b/tools/build/Build.include
index 2cf3b1bde86e..c2a95ab47379 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -99,7 +99,7 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
 ###
 ## HOSTCC C flags
 
-host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
+host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
 
 # output directory for tests below
 TMPOUT = .tmp_$$$$
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0ae2e3d8b832..374f65b52157 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
 
@@ -211,6 +212,7 @@ endif
 ifneq ($(WERROR),0)
   CORE_CFLAGS += -Werror
   CXXFLAGS += -Werror
+  HOSTCFLAGS += -Werror
 endif
 
 ifndef DEBUG
@@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
 CXXFLAGS += -funwind-tables
 CXXFLAGS += -Wno-strict-aliasing
 
+HOSTCFLAGS += -Wall
+HOSTCFLAGS += -Wextra
+
 # Enforce a non-executable stack, as we may regress (again) in the future by
 # adding assembler files missing the .GNU-stack linker note.
 LDFLAGS += -Wl,-z,noexecstack
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
 


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-21 12:48   ` Jiri Olsa
  2021-10-21 15:50     ` John Garry
@ 2021-10-22  9:42     ` John Garry
  2021-10-25 11:40       ` Jiri Olsa
  1 sibling, 1 reply; 15+ messages in thread
From: John Garry @ 2021-10-22  9:42 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: peterz, acme, mark.rutland, alexander.shishkin, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On 21/10/2021 13:48, Jiri Olsa wrote:
>> +HOSTCFLAGS += -Wall
>> +HOSTCFLAGS += -Wextra
>> +
>>   # Enforce a non-executable stack, as we may regress (again) in the future by
>>   # adding assembler files missing the .GNU-stack linker note.
>>   LDFLAGS += -Wl,-z,noexecstack
>> 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)
> so the the host cflags are made of:
> 
> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> 

ok, so IIRC, then the rule for building .o from .c in 
tools/build/Makefile.build will pick up HOSTCFLAGS through this 
variable, so we then don't need to explicitly mention it in the 
per-target rule, so can have this as before in pmu-events/Build

HOSTCFLAGS_jevents.o	= -I$(srctree)/tools/include

right?

(Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)

Thanks,
John


> can't you use KBUILD_HOSTCFLAGS?
> 
> also perhaps we could rename KBUILD_HOSTCFLAGS to HOSTCFLAGS?
> the name seems like leftover from kbuild move
> 
> jirka
> 
>>   pmu-events-y	+= pmu-events.o
>>   JDIR		=  pmu-events/arch/$(SRCARCH)
>>   JSON		=  $(shell [ -d $(JDIR) ] &&	




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-22  9:42     ` John Garry
@ 2021-10-25 11:40       ` Jiri Olsa
  2021-10-25 16:20         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2021-10-25 11:40 UTC (permalink / raw)
  To: John Garry
  Cc: peterz, acme, mark.rutland, alexander.shishkin, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
> On 21/10/2021 13:48, Jiri Olsa wrote:
> > > +HOSTCFLAGS += -Wall
> > > +HOSTCFLAGS += -Wextra
> > > +
> > >   # Enforce a non-executable stack, as we may regress (again) in the future by
> > >   # adding assembler files missing the .GNU-stack linker note.
> > >   LDFLAGS += -Wl,-z,noexecstack
> > > 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)
> > so the the host cflags are made of:
> > 
> > host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> > 
> 
> ok, so IIRC, then the rule for building .o from .c in
> tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
> we then don't need to explicitly mention it in the per-target rule, so can
> have this as before in pmu-events/Build
> 
> HOSTCFLAGS_jevents.o	= -I$(srctree)/tools/include
> 
> right?
> 
> (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)

hm, the -I.. should stay no? I don't see that
it's being added soem other way

jirka


---
diff --git a/tools/build/Build.include b/tools/build/Build.include
index 2cf3b1bde86e..c2a95ab47379 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -99,7 +99,7 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
 ###
 ## HOSTCC C flags
 
-host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
+host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
 
 # output directory for tests below
 TMPOUT = .tmp_$$$$
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0ae2e3d8b832..374f65b52157 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
 
@@ -211,6 +212,7 @@ endif
 ifneq ($(WERROR),0)
   CORE_CFLAGS += -Werror
   CXXFLAGS += -Werror
+  HOSTCFLAGS += -Werror
 endif
 
 ifndef DEBUG
@@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
 CXXFLAGS += -funwind-tables
 CXXFLAGS += -Wno-strict-aliasing
 
+HOSTCFLAGS += -Wall
+HOSTCFLAGS += -Wextra
+
 # Enforce a non-executable stack, as we may regress (again) in the future by
 # adding assembler files missing the .GNU-stack linker note.
 LDFLAGS += -Wl,-z,noexecstack
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
 


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-25 11:40       ` Jiri Olsa
@ 2021-10-25 16:20         ` Arnaldo Carvalho de Melo
  2021-10-28  8:23           ` John Garry
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-10-25 16:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: John Garry, peterz, mark.rutland, alexander.shishkin, namhyung,
	mingo, irogers, linux-perf-users, linux-kernel, kjain,
	james.clark

Em Mon, Oct 25, 2021 at 01:40:44PM +0200, Jiri Olsa escreveu:
> On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
> > On 21/10/2021 13:48, Jiri Olsa wrote:
> > > > +HOSTCFLAGS += -Wall
> > > > +HOSTCFLAGS += -Wextra
> > > > +
> > > >   # Enforce a non-executable stack, as we may regress (again) in the future by
> > > >   # adding assembler files missing the .GNU-stack linker note.
> > > >   LDFLAGS += -Wl,-z,noexecstack
> > > > 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)
> > > so the the host cflags are made of:
> > > 
> > > host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> > > 
> > 
> > ok, so IIRC, then the rule for building .o from .c in
> > tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
> > we then don't need to explicitly mention it in the per-target rule, so can
> > have this as before in pmu-events/Build
> > 
> > HOSTCFLAGS_jevents.o	= -I$(srctree)/tools/include
> > 
> > right?
> > 
> > (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
> 
> hm, the -I.. should stay no? I don't see that
> it's being added soem other way
> 
> jirka
> 

Probably this change from KBUILD_HOSTCFLAGS back to HOSTCFLAGS should
come with this;

Cc: Laura Abbott <labbott@redhat.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Fixes: 96f14fe738b69dd9 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS")

Right?

- Arnaldo
 
> ---
> diff --git a/tools/build/Build.include b/tools/build/Build.include
> index 2cf3b1bde86e..c2a95ab47379 100644
> --- a/tools/build/Build.include
> +++ b/tools/build/Build.include
> @@ -99,7 +99,7 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
>  ###
>  ## HOSTCC C flags
>  
> -host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> +host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
>  
>  # output directory for tests below
>  TMPOUT = .tmp_$$$$
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 0ae2e3d8b832..374f65b52157 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
>  
> @@ -211,6 +212,7 @@ endif
>  ifneq ($(WERROR),0)
>    CORE_CFLAGS += -Werror
>    CXXFLAGS += -Werror
> +  HOSTCFLAGS += -Werror
>  endif
>  
>  ifndef DEBUG
> @@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
>  CXXFLAGS += -funwind-tables
>  CXXFLAGS += -Wno-strict-aliasing
>  
> +HOSTCFLAGS += -Wall
> +HOSTCFLAGS += -Wextra
> +
>  # Enforce a non-executable stack, as we may regress (again) in the future by
>  # adding assembler files missing the .GNU-stack linker note.
>  LDFLAGS += -Wl,-z,noexecstack
> 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
>  

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-25 16:20         ` Arnaldo Carvalho de Melo
@ 2021-10-28  8:23           ` John Garry
  2021-10-28 11:17             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: John Garry @ 2021-10-28  8:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: peterz, mark.rutland, alexander.shishkin, namhyung, mingo,
	irogers, linux-perf-users, linux-kernel, kjain, james.clark

On 25/10/2021 17:20, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 25, 2021 at 01:40:44PM +0200, Jiri Olsa escreveu:
>> On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
>>> On 21/10/2021 13:48, Jiri Olsa wrote:
>>>>> +HOSTCFLAGS += -Wall
>>>>> +HOSTCFLAGS += -Wextra
>>>>> +
>>>>>    # Enforce a non-executable stack, as we may regress (again) in the future by
>>>>>    # adding assembler files missing the .GNU-stack linker note.
>>>>>    LDFLAGS += -Wl,-z,noexecstack
>>>>> 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)
>>>> so the the host cflags are made of:
>>>>
>>>> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
>>>>
>>>
>>> ok, so IIRC, then the rule for building .o from .c in
>>> tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
>>> we then don't need to explicitly mention it in the per-target rule, so can
>>> have this as before in pmu-events/Build
>>>
>>> HOSTCFLAGS_jevents.o	= -I$(srctree)/tools/include
>>>
>>> right?
>>>
>>> (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
>>
>> hm, the -I.. should stay no? I don't see that
>> it's being added soem other way
>>
>> jirka
>>
> 
> Probably this change from KBUILD_HOSTCFLAGS back to HOSTCFLAGS should
> come with this;
> 
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Fixes: 96f14fe738b69dd9 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS")
> 
> Right?

Maybe, but then renaming back from KBUILD_HOSTCFLAGS -> HOSTCFLAGS seems 
odd as a fix

Anyway, now that this original series is in perf/core, I'll send patches 
on top with this change, cc'ing Laura and Masahiro

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-28  8:23           ` John Garry
@ 2021-10-28 11:17             ` Arnaldo Carvalho de Melo
  2021-10-28 11:40               ` John Garry
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-10-28 11:17 UTC (permalink / raw)
  To: John Garry
  Cc: Jiri Olsa, peterz, mark.rutland, alexander.shishkin, namhyung,
	mingo, irogers, linux-perf-users, linux-kernel, kjain,
	james.clark

Em Thu, Oct 28, 2021 at 09:23:58AM +0100, John Garry escreveu:
> On 25/10/2021 17:20, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Oct 25, 2021 at 01:40:44PM +0200, Jiri Olsa escreveu:
> > > On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
> > > > On 21/10/2021 13:48, Jiri Olsa wrote:
> > > > ok, so IIRC, then the rule for building .o from .c in
> > > > tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
> > > > we then don't need to explicitly mention it in the per-target rule, so can
> > > > have this as before in pmu-events/Build

> > > > HOSTCFLAGS_jevents.o	= -I$(srctree)/tools/include

> > > > right?

> > > > (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)

> > > hm, the -I.. should stay no? I don't see that
> > > it's being added soem other way

> > Probably this change from KBUILD_HOSTCFLAGS back to HOSTCFLAGS should
> > come with this;

> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Fixes: 96f14fe738b69dd9 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS")

> > Right?
 
> Maybe, but then renaming back from KBUILD_HOSTCFLAGS -> HOSTCFLAGS seems odd
> as a fix
 
> Anyway, now that this original series is in perf/core,

Nope, just this one landed:

commit 342cb7ebf5e29fff4dc09ab2c8f37d710f8f5206
Author: John Garry <john.garry@huawei.com>
Date:   Thu Oct 21 17:16:44 2021 +0800

    perf jevents: Fix some would-be warnings

---

The one you asked to wait for further discussion wasn't merged.

- Arnaldo

> I'll send patches on top with this change, cc'ing Laura and Masahiro

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS
  2021-10-28 11:17             ` Arnaldo Carvalho de Melo
@ 2021-10-28 11:40               ` John Garry
  0 siblings, 0 replies; 15+ messages in thread
From: John Garry @ 2021-10-28 11:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, peterz, mark.rutland, alexander.shishkin, namhyung,
	mingo, irogers, linux-perf-users, linux-kernel, kjain,
	james.clark

On 28/10/2021 12:17, Arnaldo Carvalho de Melo wrote:
>> Maybe, but then renaming back from KBUILD_HOSTCFLAGS -> HOSTCFLAGS seems odd
>> as a fix
>   
>> Anyway, now that this original series is in perf/core,
> Nope, just this one landed:
> 
> commit 342cb7ebf5e29fff4dc09ab2c8f37d710f8f5206
> Author: John Garry<john.garry@huawei.com>
> Date:   Thu Oct 21 17:16:44 2021 +0800
> 
>      perf jevents: Fix some would-be warnings
> 
> ---
> 
> The one you asked to wait for further discussion wasn't merged.

ah, at I glance I saw my name and this patch and assumed both were in.

OK, so I'll put together a new series soon and post that.

Thanks

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-10-28 11:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21  9:16 [PATCH v2 0/2] perf jevents: Enable build warnings John Garry
2021-10-21  9:16 ` [PATCH v2 1/2] perf jevents: Fix some would-be warnings John Garry
2021-10-21  9:16 ` [PATCH v2 2/2] perf jevents: Enable warnings through HOSTCFLAGS John Garry
2021-10-21 12:48   ` Jiri Olsa
2021-10-21 15:50     ` John Garry
2021-10-21 15:58       ` Jiri Olsa
2021-10-22  9:42     ` John Garry
2021-10-25 11:40       ` Jiri Olsa
2021-10-25 16:20         ` Arnaldo Carvalho de Melo
2021-10-28  8:23           ` John Garry
2021-10-28 11:17             ` Arnaldo Carvalho de Melo
2021-10-28 11:40               ` John Garry
2021-10-21 12:55 ` [PATCH v2 0/2] perf jevents: Enable build warnings Arnaldo Carvalho de Melo
2021-10-21 13:02   ` John Garry
2021-10-21 14:36     ` Arnaldo Carvalho de Melo

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).