All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers
@ 2023-01-16 21:57 Ian Rogers
  2023-01-16 21:57 ` [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced Ian Rogers
  2023-01-17 22:08 ` [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers Jiri Olsa
  0 siblings, 2 replies; 8+ messages in thread
From: Ian Rogers @ 2023-01-16 21:57 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Connor OBrien,
	Nathan Chancellor, Ian Rogers, Kumar Kartikeya Dwivedi, bpf,
	linux-kernel

Previously tools/lib/subcmd was added to the include path, switch to
installing the headers and then including from that directory. This
avoids dependencies on headers internal to tools/lib/subcmd. Add the
missing subcmd directory to the affected #include.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
 tools/bpf/resolve_btfids/main.c   |  2 +-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index 19a3112e271a..76b737b2560d 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
 BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
 LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
 SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
+SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
 
 LIBBPF_DESTDIR := $(LIBBPF_OUT)
 LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
 
+SUBCMD_DESTDIR := $(SUBCMD_OUT)
+SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
+
 BINARY     := $(OUTPUT)/resolve_btfids
 BINARY_IN  := $(BINARY)-in.o
 
 all: $(BINARY)
 
+prepare: $(BPFOBJ) $(SUBCMDOBJ)
+
 $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
 	$(call msg,MKDIR,,$@)
 	$(Q)mkdir -p $(@)
 
 $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
-	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
+	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
+		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
+		    $(abspath $@) install_headers
 
 $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
 	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
@@ -60,14 +68,14 @@ CFLAGS += -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
           -I$(LIBBPF_INCLUDE) \
-          -I$(SUBCMD_SRC)
+          -I$(SUBCMD_INCLUDE)
 
 LIBS = -lelf -lz
 
 export srctree OUTPUT CFLAGS Q
 include $(srctree)/tools/build/Makefile.include
 
-$(BINARY_IN): $(BPFOBJ) fixdep FORCE | $(OUTPUT)
+$(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
 	$(Q)$(MAKE) $(build)=resolve_btfids
 
 $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
@@ -79,7 +87,8 @@ clean_objects := $(wildcard $(OUTPUT)/*.o                \
                             $(OUTPUT)/.*.o.d             \
                             $(LIBBPF_OUT)                \
                             $(LIBBPF_DESTDIR)            \
-                            $(OUTPUT)/libsubcmd          \
+                            $(SUBCMD_OUT)                \
+                            $(SUBCMD_DESTDIR)            \
                             $(OUTPUT)/resolve_btfids)
 
 ifneq ($(clean_objects),)
@@ -96,4 +105,4 @@ tags:
 
 FORCE:
 
-.PHONY: all FORCE clean tags
+.PHONY: all FORCE clean tags prepare
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 80cd7843c677..77058174082d 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -75,7 +75,7 @@
 #include <linux/err.h>
 #include <bpf/btf.h>
 #include <bpf/libbpf.h>
-#include <parse-options.h>
+#include <subcmd/parse-options.h>
 
 #define BTF_IDS_SECTION	".BTF_ids"
 #define BTF_ID		"__BTF_ID__"
-- 
2.39.0.314.g84b9a713c41-goog


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

* [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced
  2023-01-16 21:57 [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers Ian Rogers
@ 2023-01-16 21:57 ` Ian Rogers
  2023-01-17 22:07   ` Jiri Olsa
  2023-01-19 14:51   ` Jiri Olsa
  2023-01-17 22:08 ` [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers Jiri Olsa
  1 sibling, 2 replies; 8+ messages in thread
From: Ian Rogers @ 2023-01-16 21:57 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Connor OBrien,
	Nathan Chancellor, Ian Rogers, Kumar Kartikeya Dwivedi, bpf,
	linux-kernel

HOSTCC is always wanted when building. Setting CC to HOSTCC happens
after tools/scripts/Makefile.include is included, meaning flags are
set assuming say CC is gcc, but then it can be later set to HOSTCC
which may be clang. tools/scripts/Makefile.include is needed for host
set up and common macros in objtool's Makefile. Rather than override
CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
builds and the linkage step. This means the Makefiles don't see things
like CC changing and tool flag determination, and similar, work
properly.

Also, clear the passed subdir as otherwise an outer build may break by
inadvertently passing an inappropriate value.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index 76b737b2560d..515d87b32fb8 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -18,14 +18,11 @@ else
 endif
 
 # always use the host compiler
-AR       = $(HOSTAR)
-CC       = $(HOSTCC)
-LD       = $(HOSTLD)
-ARCH     = $(HOSTARCH)
+HOST_OVERRIDES := AR=$(HOSTAR) CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
+		  ARCH=$(HOSTARCH) EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
+
 RM      ?= rm
 CROSS_COMPILE =
-CFLAGS  := $(KBUILD_HOSTCFLAGS)
-LDFLAGS := $(KBUILD_HOSTLDFLAGS)
 
 OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
 
@@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
 
 $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
 	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
-		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
+		    DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
 		    $(abspath $@) install_headers
 
 $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
 	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
-		    DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
+		    DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
 		    $(abspath $@) install_headers
 
 CFLAGS += -g \
@@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
 include $(srctree)/tools/build/Makefile.include
 
 $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
-	$(Q)$(MAKE) $(build)=resolve_btfids
+	$(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
 
 $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
 	$(call msg,LINK,$@)
-	$(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
+	$(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
 
 clean_objects := $(wildcard $(OUTPUT)/*.o                \
                             $(OUTPUT)/.*.o.cmd           \
-- 
2.39.0.314.g84b9a713c41-goog


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

* Re: [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced
  2023-01-16 21:57 ` [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced Ian Rogers
@ 2023-01-17 22:07   ` Jiri Olsa
  2023-01-17 23:20     ` Ian Rogers
  2023-01-19 14:51   ` Jiri Olsa
  1 sibling, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2023-01-17 22:07 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Mon, Jan 16, 2023 at 01:57:51PM -0800, Ian Rogers wrote:
> HOSTCC is always wanted when building. Setting CC to HOSTCC happens
> after tools/scripts/Makefile.include is included, meaning flags are
> set assuming say CC is gcc, but then it can be later set to HOSTCC
> which may be clang. tools/scripts/Makefile.include is needed for host
> set up and common macros in objtool's Makefile. Rather than override
> CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
> builds and the linkage step. This means the Makefiles don't see things
> like CC changing and tool flag determination, and similar, work
> properly.
> 
> Also, clear the passed subdir as otherwise an outer build may break by
> inadvertently passing an inappropriate value.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

lgtm did you try cross build and build with clang?

jirka

> ---
>  tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index 76b737b2560d..515d87b32fb8 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -18,14 +18,11 @@ else
>  endif
>  
>  # always use the host compiler
> -AR       = $(HOSTAR)
> -CC       = $(HOSTCC)
> -LD       = $(HOSTLD)
> -ARCH     = $(HOSTARCH)
> +HOST_OVERRIDES := AR=$(HOSTAR) CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
> +		  ARCH=$(HOSTARCH) EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
> +
>  RM      ?= rm
>  CROSS_COMPILE =
> -CFLAGS  := $(KBUILD_HOSTCFLAGS)
> -LDFLAGS := $(KBUILD_HOSTLDFLAGS)
>  
>  OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
>  
> @@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
>  
>  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
>  	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> -		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> +		    DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
>  		    $(abspath $@) install_headers
>  
>  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
>  	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> -		    DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
> +		    DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
>  		    $(abspath $@) install_headers
>  
>  CFLAGS += -g \
> @@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
>  include $(srctree)/tools/build/Makefile.include
>  
>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> -	$(Q)$(MAKE) $(build)=resolve_btfids
> +	$(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
>  
>  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
>  	$(call msg,LINK,$@)
> -	$(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> +	$(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
>  
>  clean_objects := $(wildcard $(OUTPUT)/*.o                \
>                              $(OUTPUT)/.*.o.cmd           \
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 

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

* Re: [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers
  2023-01-16 21:57 [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers Ian Rogers
  2023-01-16 21:57 ` [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced Ian Rogers
@ 2023-01-17 22:08 ` Jiri Olsa
  1 sibling, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2023-01-17 22:08 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Mon, Jan 16, 2023 at 01:57:50PM -0800, Ian Rogers wrote:
> Previously tools/lib/subcmd was added to the include path, switch to
> installing the headers and then including from that directory. This
> avoids dependencies on headers internal to tools/lib/subcmd. Add the
> missing subcmd directory to the affected #include.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: iri Olsa <jolsa@kernel.org>

jirka

> ---
>  tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
>  tools/bpf/resolve_btfids/main.c   |  2 +-
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index 19a3112e271a..76b737b2560d 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
>  BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
>  LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
>  SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
> +SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
>  
>  LIBBPF_DESTDIR := $(LIBBPF_OUT)
>  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
>  
> +SUBCMD_DESTDIR := $(SUBCMD_OUT)
> +SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
> +
>  BINARY     := $(OUTPUT)/resolve_btfids
>  BINARY_IN  := $(BINARY)-in.o
>  
>  all: $(BINARY)
>  
> +prepare: $(BPFOBJ) $(SUBCMDOBJ)
> +
>  $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
>  	$(call msg,MKDIR,,$@)
>  	$(Q)mkdir -p $(@)
>  
>  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> -	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> +	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> +		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> +		    $(abspath $@) install_headers
>  
>  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
>  	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> @@ -60,14 +68,14 @@ CFLAGS += -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> -          -I$(SUBCMD_SRC)
> +          -I$(SUBCMD_INCLUDE)
>  
>  LIBS = -lelf -lz
>  
>  export srctree OUTPUT CFLAGS Q
>  include $(srctree)/tools/build/Makefile.include
>  
> -$(BINARY_IN): $(BPFOBJ) fixdep FORCE | $(OUTPUT)
> +$(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
>  	$(Q)$(MAKE) $(build)=resolve_btfids
>  
>  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> @@ -79,7 +87,8 @@ clean_objects := $(wildcard $(OUTPUT)/*.o                \
>                              $(OUTPUT)/.*.o.d             \
>                              $(LIBBPF_OUT)                \
>                              $(LIBBPF_DESTDIR)            \
> -                            $(OUTPUT)/libsubcmd          \
> +                            $(SUBCMD_OUT)                \
> +                            $(SUBCMD_DESTDIR)            \
>                              $(OUTPUT)/resolve_btfids)
>  
>  ifneq ($(clean_objects),)
> @@ -96,4 +105,4 @@ tags:
>  
>  FORCE:
>  
> -.PHONY: all FORCE clean tags
> +.PHONY: all FORCE clean tags prepare
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index 80cd7843c677..77058174082d 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -75,7 +75,7 @@
>  #include <linux/err.h>
>  #include <bpf/btf.h>
>  #include <bpf/libbpf.h>
> -#include <parse-options.h>
> +#include <subcmd/parse-options.h>
>  
>  #define BTF_IDS_SECTION	".BTF_ids"
>  #define BTF_ID		"__BTF_ID__"
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 

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

* Re: [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced
  2023-01-17 22:07   ` Jiri Olsa
@ 2023-01-17 23:20     ` Ian Rogers
  2023-01-18 22:30       ` Jiri Olsa
  0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2023-01-17 23:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Tue, Jan 17, 2023 at 2:07 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jan 16, 2023 at 01:57:51PM -0800, Ian Rogers wrote:
> > HOSTCC is always wanted when building. Setting CC to HOSTCC happens
> > after tools/scripts/Makefile.include is included, meaning flags are
> > set assuming say CC is gcc, but then it can be later set to HOSTCC
> > which may be clang. tools/scripts/Makefile.include is needed for host
> > set up and common macros in objtool's Makefile. Rather than override
> > CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
> > builds and the linkage step. This means the Makefiles don't see things
> > like CC changing and tool flag determination, and similar, work
> > properly.
> >
> > Also, clear the passed subdir as otherwise an outer build may break by
> > inadvertently passing an inappropriate value.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> lgtm did you try cross build and build with clang?

Hmm.. I don't have a cross build but I checked clang. Any chance you
could check the cross build?

Thanks,
Ian

> jirka
>
> > ---
> >  tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index 76b737b2560d..515d87b32fb8 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -18,14 +18,11 @@ else
> >  endif
> >
> >  # always use the host compiler
> > -AR       = $(HOSTAR)
> > -CC       = $(HOSTCC)
> > -LD       = $(HOSTLD)
> > -ARCH     = $(HOSTARCH)
> > +HOST_OVERRIDES := AR=$(HOSTAR) CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
> > +               ARCH=$(HOSTARCH) EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
> > +
> >  RM      ?= rm
> >  CROSS_COMPILE =
> > -CFLAGS  := $(KBUILD_HOSTCFLAGS)
> > -LDFLAGS := $(KBUILD_HOSTLDFLAGS)
> >
> >  OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
> >
> > @@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> >
> >  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> >       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> > -                 DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> > +                 DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> >                   $(abspath $@) install_headers
> >
> >  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> >       $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> > -                 DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
> > +                 DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> >                   $(abspath $@) install_headers
> >
> >  CFLAGS += -g \
> > @@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
> >  include $(srctree)/tools/build/Makefile.include
> >
> >  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> > -     $(Q)$(MAKE) $(build)=resolve_btfids
> > +     $(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
> >
> >  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> >       $(call msg,LINK,$@)
> > -     $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> > +     $(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> >
> >  clean_objects := $(wildcard $(OUTPUT)/*.o                \
> >                              $(OUTPUT)/.*.o.cmd           \
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >

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

* Re: [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced
  2023-01-17 23:20     ` Ian Rogers
@ 2023-01-18 22:30       ` Jiri Olsa
  0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2023-01-18 22:30 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Tue, Jan 17, 2023 at 03:20:38PM -0800, Ian Rogers wrote:
> On Tue, Jan 17, 2023 at 2:07 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, Jan 16, 2023 at 01:57:51PM -0800, Ian Rogers wrote:
> > > HOSTCC is always wanted when building. Setting CC to HOSTCC happens
> > > after tools/scripts/Makefile.include is included, meaning flags are
> > > set assuming say CC is gcc, but then it can be later set to HOSTCC
> > > which may be clang. tools/scripts/Makefile.include is needed for host
> > > set up and common macros in objtool's Makefile. Rather than override
> > > CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
> > > builds and the linkage step. This means the Makefiles don't see things
> > > like CC changing and tool flag determination, and similar, work
> > > properly.
> > >
> > > Also, clear the passed subdir as otherwise an outer build may break by
> > > inadvertently passing an inappropriate value.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> >
> > lgtm did you try cross build and build with clang?
> 
> Hmm.. I don't have a cross build but I checked clang. Any chance you
> could check the cross build?

ok, I'll try tomorrow

jirka

> 
> Thanks,
> Ian
> 
> > jirka
> >
> > > ---
> > >  tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
> > >  1 file changed, 7 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > > index 76b737b2560d..515d87b32fb8 100644
> > > --- a/tools/bpf/resolve_btfids/Makefile
> > > +++ b/tools/bpf/resolve_btfids/Makefile
> > > @@ -18,14 +18,11 @@ else
> > >  endif
> > >
> > >  # always use the host compiler
> > > -AR       = $(HOSTAR)
> > > -CC       = $(HOSTCC)
> > > -LD       = $(HOSTLD)
> > > -ARCH     = $(HOSTARCH)
> > > +HOST_OVERRIDES := AR=$(HOSTAR) CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
> > > +               ARCH=$(HOSTARCH) EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
> > > +
> > >  RM      ?= rm
> > >  CROSS_COMPILE =
> > > -CFLAGS  := $(KBUILD_HOSTCFLAGS)
> > > -LDFLAGS := $(KBUILD_HOSTLDFLAGS)
> > >
> > >  OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
> > >
> > > @@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> > >
> > >  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> > >       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> > > -                 DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> > > +                 DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> > >                   $(abspath $@) install_headers
> > >
> > >  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> > >       $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> > > -                 DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
> > > +                 DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> > >                   $(abspath $@) install_headers
> > >
> > >  CFLAGS += -g \
> > > @@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
> > >  include $(srctree)/tools/build/Makefile.include
> > >
> > >  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> > > -     $(Q)$(MAKE) $(build)=resolve_btfids
> > > +     $(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
> > >
> > >  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> > >       $(call msg,LINK,$@)
> > > -     $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> > > +     $(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> > >
> > >  clean_objects := $(wildcard $(OUTPUT)/*.o                \
> > >                              $(OUTPUT)/.*.o.cmd           \
> > > --
> > > 2.39.0.314.g84b9a713c41-goog
> > >

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

* Re: [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced
  2023-01-16 21:57 ` [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced Ian Rogers
  2023-01-17 22:07   ` Jiri Olsa
@ 2023-01-19 14:51   ` Jiri Olsa
  2023-01-19 18:13     ` Ian Rogers
  1 sibling, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2023-01-19 14:51 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Mon, Jan 16, 2023 at 01:57:51PM -0800, Ian Rogers wrote:
> HOSTCC is always wanted when building. Setting CC to HOSTCC happens
> after tools/scripts/Makefile.include is included, meaning flags are
> set assuming say CC is gcc, but then it can be later set to HOSTCC
> which may be clang. tools/scripts/Makefile.include is needed for host
> set up and common macros in objtool's Makefile. Rather than override
> CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
> builds and the linkage step. This means the Makefiles don't see things
> like CC changing and tool flag determination, and similar, work
> properly.
> 
> Also, clear the passed subdir as otherwise an outer build may break by
> inadvertently passing an inappropriate value.

I tested with cross builds for s390/ppc/arm64 and it was ok

some comments below

thanks,
jirka


> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index 76b737b2560d..515d87b32fb8 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -18,14 +18,11 @@ else
>  endif
>  
>  # always use the host compiler
> -AR       = $(HOSTAR)
> -CC       = $(HOSTCC)
> -LD       = $(HOSTLD)
> -ARCH     = $(HOSTARCH)

I wonder all the tools should use HOSTCC in the first place?
seems more clear than forcing it from other makefiles

subcmd even has:

CC ?= $(CROSS_COMPILE)gcc
LD ?= $(CROSS_COMPILE)ld
AR ?= $(CROSS_COMPILE)ar

which seems wrong unless I'm missing something.. should be always
the host compiler, right?

> +HOST_OVERRIDES := AR=$(HOSTAR) CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
> +		  ARCH=$(HOSTARCH) EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"

there's extra AR set and ARCH value is not in ""

> +
>  RM      ?= rm
>  CROSS_COMPILE =
> -CFLAGS  := $(KBUILD_HOSTCFLAGS)
> -LDFLAGS := $(KBUILD_HOSTLDFLAGS)
>  
>  OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
>  
> @@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
>  
>  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
>  	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> -		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> +		    DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
>  		    $(abspath $@) install_headers
>  
>  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
>  	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> -		    DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
> +		    DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
>  		    $(abspath $@) install_headers
>  
>  CFLAGS += -g \
> @@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
>  include $(srctree)/tools/build/Makefile.include
>  
>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> -	$(Q)$(MAKE) $(build)=resolve_btfids
> +	$(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
>  
>  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
>  	$(call msg,LINK,$@)
> -	$(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> +	$(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
>  
>  clean_objects := $(wildcard $(OUTPUT)/*.o                \
>                              $(OUTPUT)/.*.o.cmd           \
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 

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

* Re: [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced
  2023-01-19 14:51   ` Jiri Olsa
@ 2023-01-19 18:13     ` Ian Rogers
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2023-01-19 18:13 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Thu, Jan 19, 2023 at 6:51 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jan 16, 2023 at 01:57:51PM -0800, Ian Rogers wrote:
> > HOSTCC is always wanted when building. Setting CC to HOSTCC happens
> > after tools/scripts/Makefile.include is included, meaning flags are
> > set assuming say CC is gcc, but then it can be later set to HOSTCC
> > which may be clang. tools/scripts/Makefile.include is needed for host
> > set up and common macros in objtool's Makefile. Rather than override
> > CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
> > builds and the linkage step. This means the Makefiles don't see things
> > like CC changing and tool flag determination, and similar, work
> > properly.
> >
> > Also, clear the passed subdir as otherwise an outer build may break by
> > inadvertently passing an inappropriate value.
>
> I tested with cross builds for s390/ppc/arm64 and it was ok
>
> some comments below
>
> thanks,
> jirka
>
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index 76b737b2560d..515d87b32fb8 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -18,14 +18,11 @@ else
> >  endif
> >
> >  # always use the host compiler
> > -AR       = $(HOSTAR)
> > -CC       = $(HOSTCC)
> > -LD       = $(HOSTLD)
> > -ARCH     = $(HOSTARCH)
>
> I wonder all the tools should use HOSTCC in the first place?
> seems more clear than forcing it from other makefiles
>
> subcmd even has:
>
> CC ?= $(CROSS_COMPILE)gcc
> LD ?= $(CROSS_COMPILE)ld
> AR ?= $(CROSS_COMPILE)ar
>
> which seems wrong unless I'm missing something.. should be always
> the host compiler, right?

Hmm.. it seems like a feature to be able to cross compile things like
the perf tool.

I agree the way this is all done is muddling and we should try to keep
the way it is done consistent. The pattern of always early including
Makefile.include, that sets flags based on CC, but then overriding CC
was what I was after cleaning up here. Let's work on migrating to
something better.

> > +HOST_OVERRIDES := AR=$(HOSTAR) CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)" \
> > +               ARCH=$(HOSTARCH) EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
>
> there's extra AR set and ARCH value is not in ""

Ack. Will fix in v3.

Thanks,
Ian


> > +
> >  RM      ?= rm
> >  CROSS_COMPILE =
> > -CFLAGS  := $(KBUILD_HOSTCFLAGS)
> > -LDFLAGS := $(KBUILD_HOSTLDFLAGS)
> >
> >  OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
> >
> > @@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> >
> >  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> >       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> > -                 DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> > +                 DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> >                   $(abspath $@) install_headers
> >
> >  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> >       $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> > -                 DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
> > +                 DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> >                   $(abspath $@) install_headers
> >
> >  CFLAGS += -g \
> > @@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
> >  include $(srctree)/tools/build/Makefile.include
> >
> >  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> > -     $(Q)$(MAKE) $(build)=resolve_btfids
> > +     $(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
> >
> >  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> >       $(call msg,LINK,$@)
> > -     $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> > +     $(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> >
> >  clean_objects := $(wildcard $(OUTPUT)/*.o                \
> >                              $(OUTPUT)/.*.o.cmd           \
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >

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

end of thread, other threads:[~2023-01-19 18:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-16 21:57 [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers Ian Rogers
2023-01-16 21:57 ` [PATCH v2 2/2] tools/resolve_btfids: Alter how HOSTCC is forced Ian Rogers
2023-01-17 22:07   ` Jiri Olsa
2023-01-17 23:20     ` Ian Rogers
2023-01-18 22:30       ` Jiri Olsa
2023-01-19 14:51   ` Jiri Olsa
2023-01-19 18:13     ` Ian Rogers
2023-01-17 22:08 ` [PATCH v2 1/2] tools/resolve_btfids: Install subcmd headers Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.