kbuild/resolve_btfids: Invoke resolve_btfids clean in root Makefile

PATCH Applied Jiri Olsa 7 participants 28 messages Feb 5, 2021 – Mar 10, 2021

Subsystems (4)

Networking Network drivers Kbuild BPF

lore

Share & tools

lore

Jiri Olsa

author reviewer

2021-02-05 12:43 UTC #0 lore raw

v2 cover kbuild/resolve_btfids: Invoke resolve_btfids clean in root Makefile

hi,
resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.

v2 changes:
- add Song's acks on patches 1 and 4 (others changed) [Song]
- add missing / [Andrii]
- change srctree variable initialization [Andrii]
- shifted ifdef for clean target [Andrii]

thanks,
jirka

---
Jiri Olsa (4):
tools/resolve_btfids: Build libbpf and libsubcmd in separate directories tools/resolve_btfids: Check objects before removing tools/resolve_btfids: Set srctree variable unconditionally
kbuild: Add resolve_btfids clean to root clean target

 Makefile                            |  7 ++++++-
 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 44 ++++++++++++++++++++++----------------------
 3 files changed, 28 insertions(+), 25 deletions(-)

Jiri Olsa

author reviewer

2021-02-05 12:43 UTC #1 lore raw

PATCH v2 1/4 tools/resolve_btfids: Build libbpf and libsubcmd in separate directories

Setting up separate build directories for libbpf and libpsubcmd, so it's separated from other objects and we don't get them mixed in the future.

It also simplifies cleaning, which is now simple rm -rf.

Also there's no need for FEATURE-DUMP.libbpf and bpf_helper_defs.h files in .gitignore anymore.

Acked-by Song Liu <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 26 +++++++++++---------------
 2 files changed, 11 insertions(+), 17 deletions(-)

tools/bpf/resolve_btfids/.gitignore +0 -2

index a026df7dc280..25f308c933cc 100644
@@ -1,4 +1,2 @@
-/FEATURE-DUMP.libbpf
-/bpf_helper_defs.h
 /fixdep
 /resolve_btfids

tools/bpf/resolve_btfids/Makefile +11 ~~-15~~

index bf656432ad73..1d46a247ec95 100644
@@ -28,22 +28,22 @@ OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
 LIBBPF_SRC := $(srctree)/tools/lib/bpf/
 SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
 
-BPFOBJ     := $(OUTPUT)/libbpf.a
-SUBCMDOBJ  := $(OUTPUT)/libsubcmd.a
+BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
+SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
 
 BINARY     := $(OUTPUT)/resolve_btfids
 BINARY_IN  := $(BINARY)-in.o
 
 all: $(BINARY)
 
-$(OUTPUT):
+$(OUTPUT) $(OUTPUT)/libbpf $(OUTPUT)/libsubcmd:
 	$(call msg,MKDIR,,$@)
-	$(Q)mkdir -p $(OUTPUT)
+	$(Q)mkdir -p $(@)
 
-$(SUBCMDOBJ): fixdep FORCE
-	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT)
+$(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
+	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
-$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)/libbpf
 	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)  OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
 CFLAGS := -g \

@@ -57,23 +57,19 @@ LIBS = -lelf -lz
 export srctree OUTPUT CFLAGS Q
 include $(srctree)/tools/build/Makefile.include
 
-$(BINARY_IN): fixdep FORCE
+$(BINARY_IN): fixdep FORCE | $(OUTPUT)
 	$(Q)$(MAKE) $(build)=resolve_btfids
 
 $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
 	$(call msg,LINK,$@)
 	$(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
 
-libsubcmd-clean:
-	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT) clean
-
-libbpf-clean:
-	$(Q)$(MAKE) -C $(LIBBPF_SRC) OUTPUT=$(OUTPUT) clean
-
-clean: libsubcmd-clean libbpf-clean fixdep-clean
+clean: fixdep-clean
 	$(call msg,CLEAN,$(BINARY))
 	$(Q)$(RM) -f $(BINARY); \
 	$(RM) -rf $(if $(OUTPUT),$(OUTPUT),.)/feature; \
+	$(RM) -rf $(OUTPUT)/libbpf; \
+	$(RM) -rf $(OUTPUT)/libsubcmd; \
 	find $(if $(OUTPUT),$(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
 
 tags:

Signature · 1 line

2.26.2

Andrii Nakryiko

Acked-by on v2 1/4

2021-02-06 02:44 UTC #2 lore raw

Re: [PATCH bpf-next 1/4] tools/resolve_btfids: Build libbpf and libsubcmd in separate directories

Quoted 5 lines · Jiri Olsa

Setting up separate build directories for libbpf and libpsubcmd, so it's separated from other objects and we don't get them mixed in the future.

Quoted 8 lines

It also simplifies cleaning, which is now simple rm -rf.
Also there's no need for FEATURE-DUMP.libbpf and bpf_helper_defs.h files in .gitignore anymore.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---

Acked-by Andrii Nakryiko <[email protected]>

Quoted 78 lines

 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 26 +++++++++++---------------
 2 files changed, 11 insertions(+), 17 deletions(-)

tools/bpf/resolve_btfids/.gitignore +0 -2

index a026df7dc280..25f308c933cc 100644
@@ -1,4 +1,2 @@
-/FEATURE-DUMP.libbpf
-/bpf_helper_defs.h
 /fixdep
 /resolve_btfids

tools/bpf/resolve_btfids/Makefile +11 ~~-16~~

index bf656432ad73..1d46a247ec95 100644
@@ -28,22 +28,22 @@ OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
 LIBBPF_SRC := $(srctree)/tools/lib/bpf/
 SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
 
-BPFOBJ     := $(OUTPUT)/libbpf.a
-SUBCMDOBJ  := $(OUTPUT)/libsubcmd.a
+BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
+SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
 
 BINARY     := $(OUTPUT)/resolve_btfids
 BINARY_IN  := $(BINARY)-in.o
 
 all: $(BINARY)
 
-$(OUTPUT):
+$(OUTPUT) $(OUTPUT)/libbpf $(OUTPUT)/libsubcmd:
        $(call msg,MKDIR,,$@)
-       $(Q)mkdir -p $(OUTPUT)
+       $(Q)mkdir -p $(@)
 
-$(SUBCMDOBJ): fixdep FORCE
-       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT)
+$(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
+       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
-$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)/libbpf
        $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)  OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
 CFLAGS := -g \

@@ -57,23 +57,19 @@ LIBS = -lelf -lz
 export srctree OUTPUT CFLAGS Q
 include $(srctree)/tools/build/Makefile.include
 
-$(BINARY_IN): fixdep FORCE
+$(BINARY_IN): fixdep FORCE | $(OUTPUT)
        $(Q)$(MAKE) $(build)=resolve_btfids
 
 $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
        $(call msg,LINK,$@)
        $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
 
-libsubcmd-clean:
-       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT) clean
-
-libbpf-clean:
-       $(Q)$(MAKE) -C $(LIBBPF_SRC) OUTPUT=$(OUTPUT) clean
-
-clean: libsubcmd-clean libbpf-clean fixdep-clean
+clean: fixdep-clean
        $(call msg,CLEAN,$(BINARY))
        $(Q)$(RM) -f $(BINARY); \
        $(RM) -rf $(if $(OUTPUT),$(OUTPUT),.)/feature; \
+       $(RM) -rf $(OUTPUT)/libbpf; \
+       $(RM) -rf $(OUTPUT)/libsubcmd; \
        find $(if $(OUTPUT),$(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
 
 tags:
--

2.26.2

Viktor Jägersküpper

2021-03-10 17:35 UTC #3 lore raw

Re: [PATCH bpf-next 1/4] tools/resolve_btfids: Build libbpf and libsubcmd in separate directories

In reply to Jiri Olsa

Hi,

Quoted 4 lines

Setting up separate build directories for libbpf and libpsubcmd, so it's separated from other objects and we don't get them mixed in the future.

Quoted 8 lines

It also simplifies cleaning, which is now simple rm -rf.
Also there's no need for FEATURE-DUMP.libbpf and bpf_helper_defs.h files in .gitignore anymore.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---

when I invoke 'git status' on the master branch of my local git repository (cloned from stable/linux.git), which I have used to compile several kernels, it lists two untracked files:

tools/bpf/resolve_btfids/FEATURE-DUMP.libbpf tools/bpf/resolve_btfids/bpf_helper_defs.h

'git status' doesn't complain about these files with v5.11, and I can't get rid of them by 'make clean' with v5.11 or v5.12-rc1/rc2. So I used 'git bisect' and found that this is caused by commit fc6b48f692f89cc48bfb7fd1aa65454dfe9b2d77, which links to this thread.

Looking at the diff it's obvious because of the change in the .gitignore file, but I don't know why these files are there and I have never touched anything in the 'tools' directory.

Can I savely delete the files? Do I even have to delete them before I compile v5.12-rcX?

Thanks,
Viktor

Quoted 76 lines

 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 26 +++++++++++---------------
 2 files changed, 11 insertions(+), 17 deletions(-)

tools/bpf/resolve_btfids/.gitignore +0 -2

index a026df7dc280..25f308c933cc 100644
@@ -1,4 +1,2 @@
-/FEATURE-DUMP.libbpf
-/bpf_helper_defs.h
 /fixdep
 /resolve_btfids

tools/bpf/resolve_btfids/Makefile +11 ~~-15~~

index bf656432ad73..1d46a247ec95 100644
@@ -28,22 +28,22 @@ OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
 LIBBPF_SRC := $(srctree)/tools/lib/bpf/
 SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
 
-BPFOBJ     := $(OUTPUT)/libbpf.a
-SUBCMDOBJ  := $(OUTPUT)/libsubcmd.a
+BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
+SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
 
 BINARY     := $(OUTPUT)/resolve_btfids
 BINARY_IN  := $(BINARY)-in.o
 
 all: $(BINARY)
 
-$(OUTPUT):
+$(OUTPUT) $(OUTPUT)/libbpf $(OUTPUT)/libsubcmd:
 	$(call msg,MKDIR,,$@)
-	$(Q)mkdir -p $(OUTPUT)
+	$(Q)mkdir -p $(@)
 
-$(SUBCMDOBJ): fixdep FORCE
-	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT)
+$(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
+	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
-$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)/libbpf
 	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)  OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
 CFLAGS := -g \

@@ -57,23 +57,19 @@ LIBS = -lelf -lz
 export srctree OUTPUT CFLAGS Q
 include $(srctree)/tools/build/Makefile.include
 
-$(BINARY_IN): fixdep FORCE
+$(BINARY_IN): fixdep FORCE | $(OUTPUT)
 	$(Q)$(MAKE) $(build)=resolve_btfids
 
 $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
 	$(call msg,LINK,$@)
 	$(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
 
-libsubcmd-clean:
-	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT) clean
-
-libbpf-clean:
-	$(Q)$(MAKE) -C $(LIBBPF_SRC) OUTPUT=$(OUTPUT) clean
-
-clean: libsubcmd-clean libbpf-clean fixdep-clean
+clean: fixdep-clean
 	$(call msg,CLEAN,$(BINARY))
 	$(Q)$(RM) -f $(BINARY); \
 	$(RM) -rf $(if $(OUTPUT),$(OUTPUT),.)/feature; \
+	$(RM) -rf $(OUTPUT)/libbpf; \
+	$(RM) -rf $(OUTPUT)/libsubcmd; \
 	find $(if $(OUTPUT),$(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
 
 tags:

Andrii Nakryiko

2021-03-10 19:28 UTC #4 lore raw

Re: [PATCH bpf-next 1/4] tools/resolve_btfids: Build libbpf and libsubcmd in separate directories

Quoted 28 lines · Viktor Jägersküpper

Hi,
Quoted 4 lines
Setting up separate build directories for libbpf and libpsubcmd, so it's separated from other objects and we don't get them mixed in the future.
Quoted 8 lines
It also simplifies cleaning, which is now simple rm -rf.
Also there's no need for FEATURE-DUMP.libbpf and bpf_helper_defs.h files in .gitignore anymore.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
when I invoke 'git status' on the master branch of my local git repository (cloned from stable/linux.git), which I have used to compile several kernels, it lists two untracked files:
tools/bpf/resolve_btfids/FEATURE-DUMP.libbpf tools/bpf/resolve_btfids/bpf_helper_defs.h
'git status' doesn't complain about these files with v5.11, and I can't get rid of them by 'make clean' with v5.11 or v5.12-rc1/rc2. So I used 'git bisect' and found that this is caused by commit fc6b48f692f89cc48bfb7fd1aa65454dfe9b2d77, which links to this thread.

Quoted 6 lines

Looking at the diff it's obvious because of the change in the .gitignore file, but I don't know why these files are there and I have never touched anything in the 'tools' directory.
Can I savely delete the files? Do I even have to delete them before I compile v5.12-rcX?

yes, those were auto-generated files. You can safely remove them.

Quoted 4 lines

Thanks,
Viktor

[...]

Jiri Olsa

2021-03-10 22:43 UTC #5 lore raw

Re: [PATCH bpf-next 1/4] tools/resolve_btfids: Build libbpf and libsubcmd in separate directories

Quoted 30 lines · Andrii Nakryiko

Quoted 28 lines · Viktor Jägersküpper
Hi,
Quoted 4 lines
Setting up separate build directories for libbpf and libpsubcmd, so it's separated from other objects and we don't get them mixed in the future.
Quoted 8 lines
It also simplifies cleaning, which is now simple rm -rf.
Also there's no need for FEATURE-DUMP.libbpf and bpf_helper_defs.h files in .gitignore anymore.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
when I invoke 'git status' on the master branch of my local git repository (cloned from stable/linux.git), which I have used to compile several kernels, it lists two untracked files:
tools/bpf/resolve_btfids/FEATURE-DUMP.libbpf tools/bpf/resolve_btfids/bpf_helper_defs.h
'git status' doesn't complain about these files with v5.11, and I can't get rid of them by 'make clean' with v5.11 or v5.12-rc1/rc2. So I used 'git bisect' and found that this is caused by commit fc6b48f692f89cc48bfb7fd1aa65454dfe9b2d77, which links to this thread.

Quoted 8 lines

Quoted 6 lines
Looking at the diff it's obvious because of the change in the .gitignore file, but I don't know why these files are there and I have never touched anything in the 'tools' directory.
Can I savely delete the files? Do I even have to delete them before I compile v5.12-rcX?
yes, those were auto-generated files. You can safely remove them.

hm, I answered this email, but for some reason I can't see it on lore.. FWIW, trying once more ;-)

hi,
yes, you can delete them, this patch moved libbpf and libsubcmd into their own build directories, so those 2 files stayed there from your last build without the patch

jirka

Jiri Olsa

author reviewer

2021-02-05 12:44 UTC #6 lore raw

PATCH v2 2/4 tools/resolve_btfids: Check objects before removing

In reply to Jiri Olsa

We want this clean to be called from tree's root clean and that one is silent if there's nothing to clean.

Adding check for all object to clean and display CLEAN messages only if there are objects to remove.

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/bpf/resolve_btfids/Makefile | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

tools/bpf/resolve_btfids/Makefile +12 -5

index 1d46a247ec95..be09ec4f03ff 100644
@@ -64,13 +64,20 @@ $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
 	$(call msg,LINK,$@)
 	$(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
 
+clean_objects := $(wildcard $(OUTPUT)/*.o                \
+                            $(OUTPUT)/.*.o.cmd           \
+                            $(OUTPUT)/.*.o.d             \
+                            $(OUTPUT)/libbpf             \
+                            $(OUTPUT)/libsubcmd          \
+                            $(OUTPUT)/resolve_btfids)
+
+ifneq ($(clean_objects),)
 clean: fixdep-clean
 	$(call msg,CLEAN,$(BINARY))
-	$(Q)$(RM) -f $(BINARY); \
-	$(RM) -rf $(if $(OUTPUT),$(OUTPUT),.)/feature; \
-	$(RM) -rf $(OUTPUT)/libbpf; \
-	$(RM) -rf $(OUTPUT)/libsubcmd; \
-	find $(if $(OUTPUT),$(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
+	$(Q)$(RM) -rf $(clean_objects)
+else
+clean:
+endif
 
 tags:
 	$(call msg,GEN,,tags)

Signature · 1 line

2.26.2

Andrii Nakryiko

Acked-by on v2 2/4

2021-02-06 02:31 UTC #7 lore raw

Re: [PATCH bpf-next 2/4] tools/resolve_btfids: Check objects before removing

Quoted 9 lines · Jiri Olsa

We want this clean to be called from tree's root clean and that one is silent if there's nothing to clean.
Adding check for all object to clean and display CLEAN messages only if there are objects to remove.
Signed-off-by Jiri Olsa <[email protected]>
---

Acked-by Andrii Nakryiko <[email protected]>

Quoted 36 lines

 tools/bpf/resolve_btfids/Makefile | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

tools/bpf/resolve_btfids/Makefile +12 -6

index 1d46a247ec95..be09ec4f03ff 100644
@@ -64,13 +64,20 @@ $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
        $(call msg,LINK,$@)
        $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
 
+clean_objects := $(wildcard $(OUTPUT)/*.o                \
+                            $(OUTPUT)/.*.o.cmd           \
+                            $(OUTPUT)/.*.o.d             \
+                            $(OUTPUT)/libbpf             \
+                            $(OUTPUT)/libsubcmd          \
+                            $(OUTPUT)/resolve_btfids)
+
+ifneq ($(clean_objects),)
 clean: fixdep-clean
        $(call msg,CLEAN,$(BINARY))
-       $(Q)$(RM) -f $(BINARY); \
-       $(RM) -rf $(if $(OUTPUT),$(OUTPUT),.)/feature; \
-       $(RM) -rf $(OUTPUT)/libbpf; \
-       $(RM) -rf $(OUTPUT)/libsubcmd; \
-       find $(if $(OUTPUT),$(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
+       $(Q)$(RM) -rf $(clean_objects)
+else
+clean:
+endif
 
 tags:
        $(call msg,GEN,,tags)
--

2.26.2

Jiri Olsa

author reviewer

2021-02-05 12:45 UTC #8 lore raw

PATCH v2 3/4 tools/resolve_btfids: Set srctree variable unconditionally

In reply to Jiri Olsa

We want this clean to be called from tree's root Makefile, which defines same srctree variable and that will screw the make setup.

We actually do not use srctree being passed from outside, so we can solve this by setting current srctree value directly.

Also changing the way how srctree is initialized as suggested by Andrri.

Also root Makefile does not define the implicit RM variable, so adding RM initialization.

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/bpf/resolve_btfids/Makefile | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

tools/bpf/resolve_btfids/Makefile +2 -5

index be09ec4f03ff..bb9fa8de7e62 100644
@@ -2,11 +2,7 @@
 include ../../scripts/Makefile.include
 include ../../scripts/Makefile.arch
 
-ifeq ($(srctree),)
-srctree := $(patsubst %/,%,$(dir $(CURDIR)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-endif
+srctree := $(abspath $(CURDIR)/../../../)
 
 ifeq ($(V),1)
   Q =

@@ -22,6 +18,7 @@ AR       = $(HOSTAR)
 CC       = $(HOSTCC)
 LD       = $(HOSTLD)
 ARCH     = $(HOSTARCH)
+RM      ?= rm
 
 OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/

Signature · 1 line

2.26.2

Andrii Nakryiko

Acked-by on v2 3/4

2021-02-06 03:01 UTC #9 lore raw

Re: [PATCH bpf-next 3/4] tools/resolve_btfids: Set srctree variable unconditionally

Quoted 9 lines · Jiri Olsa

We want this clean to be called from tree's root Makefile, which defines same srctree variable and that will screw the make setup.
We actually do not use srctree being passed from outside, so we can solve this by setting current srctree value directly.

Quoted 8 lines

Also changing the way how srctree is initialized as suggested by Andrri.
Also root Makefile does not define the implicit RM variable, so adding RM initialization.
Signed-off-by Jiri Olsa <[email protected]>
---

Acked-by Andrii Nakryiko <[email protected]>

Quoted 31 lines

 tools/bpf/resolve_btfids/Makefile | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

tools/bpf/resolve_btfids/Makefile +2 -6

index be09ec4f03ff..bb9fa8de7e62 100644
@@ -2,11 +2,7 @@
 include ../../scripts/Makefile.include
 include ../../scripts/Makefile.arch
 
-ifeq ($(srctree),)
-srctree := $(patsubst %/,%,$(dir $(CURDIR)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-endif
+srctree := $(abspath $(CURDIR)/../../../)
 
 ifeq ($(V),1)
   Q =

@@ -22,6 +18,7 @@ AR       = $(HOSTAR)
 CC       = $(HOSTCC)
 LD       = $(HOSTLD)
 ARCH     = $(HOSTARCH)
+RM      ?= rm
 
 OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
 
--

2.26.2

Jiri Olsa

author reviewer

2021-02-05 12:43 UTC #10 lore raw

PATCH v2 4/4 kbuild: Add resolve_btfids clean to root clean target

In reply to Jiri Olsa

The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.

Invoking the the resolve_btfids clean as part of root 'make clean'.

Acked-by Song Liu <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Makefile +6 -1

index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+	$(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)

@@ -1495,7 +1500,7 @@ vmlinuxclean:
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
 	$(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #

Signature · 1 line

2.26.2

Amy Parker

2021-02-06 05:14 UTC #11 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

Quoted 39 lines · Jiri Olsa

The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.

Invoking the the resolve_btfids clean as part of root 'make clean'.

Acked-by Song Liu <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Makefile +6 -2

index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+       $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)

@@ -1495,7 +1500,7 @@ vmlinuxclean:
        $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
        $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--

Quoted 2 lines

2.26.2

It compiles, looks good to me.

Reviewed by: Amy Parker <[email protected]>

Have a great day, and thank you for this patch!

-Amy IP

Nathan Chancellor

maintainer

2021-02-10 17:46 UTC #12 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

In reply to Jiri Olsa

Quoted 40 lines · Jiri Olsa

The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.

Invoking the the resolve_btfids clean as part of root 'make clean'.

Acked-by Song Liu <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Makefile +6 -1

index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+	$(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)

@@ -1495,7 +1500,7 @@ vmlinuxclean:
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
 	$(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #

Signature · 1 line

2.26.2

This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):

$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.

Cheers,
Nathan

Andrii Nakryiko

2021-02-10 17:54 UTC #13 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

Quoted 46 lines · Nathan Chancellor

Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):

Quoted 3 lines

$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.

Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.

Quoted 2 lines

Cheers,
Nathan

Nathan Chancellor

maintainer

2021-02-10 18:06 UTC #14 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

Quoted 47 lines · Andrii Nakryiko

Quoted 46 lines · Nathan Chancellor
Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):

Quoted 8 lines

Quoted 3 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.

Odd, this reproduces for me on a completely clean checkout of bpf-next:

$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/

$ cd bpf-next

$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.

I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?

Cheers,
Nathan

Jiri Olsa

2021-02-10 18:35 UTC #15 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

Quoted 63 lines · Nathan Chancellor

Quoted 47 lines · Andrii Nakryiko
Quoted 46 lines · Nathan Chancellor
Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):
Quoted 8 lines
Quoted 3 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.
Odd, this reproduces for me on a completely clean checkout of bpf-next:
$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
$ cd bpf-next

Quoted 7 lines

$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?

right, we need to handle the case where tools/bpf/resolve_btfids does not exist, patch below fixes it for me

jirka

---

Makefile +7 -1

index 159d9592b587..ce9685961abe 100644
@@ -1088,8 +1088,14 @@ endif
 
 PHONY += resolve_btfids_clean
 
+resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
+
+# tools/bpf/resolve_btfids directory might not exist
+# in output directory, skip its clean in that case
 resolve_btfids_clean:
-	$(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+ifneq (,$(wildcard $(resolve_btfids_O)))
+	$(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(resolve_btfids_O) clean
+endif
 
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF

Nathan Chancellor

maintainer Tested-by

2021-02-10 18:40 UTC #16 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

Quoted 72 lines · Jiri Olsa

Quoted 63 lines · Nathan Chancellor
Quoted 47 lines · Andrii Nakryiko
Quoted 46 lines · Nathan Chancellor
Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):
Quoted 8 lines
Quoted 3 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.
Odd, this reproduces for me on a completely clean checkout of bpf-next:
$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
$ cd bpf-next
Quoted 7 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?

Quoted 4 lines

right, we need to handle the case where tools/bpf/resolve_btfids does not exist, patch below fixes it for me
jirka

Yes, this seems to work fine for me as well.

Tested-by Nathan Chancellor <[email protected]>

Quoted 22 lines

---

Makefile +7 -1

index 159d9592b587..ce9685961abe 100644
@@ -1088,8 +1088,14 @@ endif
 
 PHONY += resolve_btfids_clean
 
+resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
+
+# tools/bpf/resolve_btfids directory might not exist
+# in output directory, skip its clean in that case
 resolve_btfids_clean:
-	$(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+ifneq (,$(wildcard $(resolve_btfids_O)))
+	$(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(resolve_btfids_O) clean
+endif
 
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF

Andrii Nakryiko

2021-02-10 19:27 UTC #17 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

In reply to Jiri Olsa

Quoted 73 lines · Jiri Olsa

Quoted 63 lines · Nathan Chancellor
Quoted 47 lines · Andrii Nakryiko
Quoted 46 lines · Nathan Chancellor
Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):
Quoted 8 lines
Quoted 3 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.
Odd, this reproduces for me on a completely clean checkout of bpf-next:
$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
$ cd bpf-next
Quoted 7 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?

Quoted 5 lines

right, we need to handle the case where tools/bpf/resolve_btfids does not exist, patch below fixes it for me
jirka

Looks good to me, please send it as a proper patch to bpf-next.

But I'm curious, why is objtool not doing something like that? Is it not doing clean at all? Or does it do it in some different way?

Quoted 7 lines

---

Makefile +0 -0

index 159d9592b587..ce9685961abe 100644

Makefile endif · line 1096

Quoted 10 lines

Makefile +5 -1

@@ -1088,8 +1088,14 @@ endif
 
 PHONY += resolve_btfids_clean
 
+resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
+
+# tools/bpf/resolve_btfids directory might not exist
+# in output directory, skip its clean in that case
 resolve_btfids_clean:
-       $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+ifneq (,$(wildcard $(resolve_btfids_O)))

nit: kind of backwards, usually it's in a ifneq($var,) form

Quoted 6 lines

Makefile +2 -0

+       $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(resolve_btfids_O) clean
+endif
 
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF

Jiri Olsa

2021-02-11 12:21 UTC #18 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

On Wed, Feb 10, 2021 at 11:26:28AM -0800, Andrii Nakryiko wrote:

SNIP

Quoted 22 lines

Quoted 22 lines
Quoted 11 lines
Quoted 4 lines
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.
Odd, this reproduces for me on a completely clean checkout of bpf-next:
$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
$ cd bpf-next
Quoted 7 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?
right, we need to handle the case where tools/bpf/resolve_btfids does not exist, patch below fixes it for me

Quoted 7 lines

Quoted 2 lines
jirka
Looks good to me, please send it as a proper patch to bpf-next.
But I'm curious, why is objtool not doing something like that? Is it not doing clean at all? Or does it do it in some different way?

yes, it's not connected to global make clean

Quoted 11 lines

---

Makefile +0 -0

index 159d9592b587..ce9685961abe 100644
@@ -1088,8 +1088,14 @@ endif
 
 PHONY += resolve_btfids_clean

Quoted 9 lines

Makefile endif · line 1096

Quoted 7 lines

Makefile +5 -1

+resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
+
+# tools/bpf/resolve_btfids directory might not exist
+# in output directory, skip its clean in that case
 resolve_btfids_clean:
-       $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+ifneq (,$(wildcard $(resolve_btfids_O)))

nit: kind of backwards, usually it's in a ifneq($var,) form

thanks,
jirka

Masahiro Yamada

2021-02-12 03:32 UTC #19 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

Quoted 58 lines · Jiri Olsa

On Wed, Feb 10, 2021 at 11:26:28AM -0800, Andrii Nakryiko wrote:
SNIP
Quoted 22 lines
Quoted 22 lines
Quoted 11 lines
Quoted 4 lines
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.
Odd, this reproduces for me on a completely clean checkout of bpf-next:
$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
$ cd bpf-next
Quoted 7 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?
right, we need to handle the case where tools/bpf/resolve_btfids does not exist, patch below fixes it for me
Quoted 7 lines
Quoted 2 lines
jirka
Looks good to me, please send it as a proper patch to bpf-next.
But I'm curious, why is objtool not doing something like that? Is it not doing clean at all? Or does it do it in some different way?
yes, it's not connected to global make clean
Quoted 11 lines
Quoted 11 lines
---
Makefile +0 -0
index 159d9592b587..ce9685961abe 100644
@@ -1088,8 +1088,14 @@ endif
 
 PHONY += resolve_btfids_clean
 
Quoted 9 lines
Makefile endif · line 1096
Quoted 7 lines
Makefile +5 -1
+resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
+
+# tools/bpf/resolve_btfids directory might not exist
+# in output directory, skip its clean in that case
 resolve_btfids_clean:
-       $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+ifneq (,$(wildcard $(resolve_btfids_O)))
nit: kind of backwards, usually it's in a ifneq($var,) form

Quoted 5 lines

ok
thanks,
jirka

I expected this kind of mess
when I saw 33a57ce0a54d498275f432db04850001175dfdfa

The tools/ directory is a completely different world governed by a different build system
(no, not a build system, but a collection of adhoc makefile code)

All the other programs used during the kernel build are located under scripts/, and can be built with a simple syntax, and cleaned up correctly. It is simple, clean and robust.

objtool is the first alien that opt out Kbuild, and this is the second one.

It is scary to mix up two different things, which run in different working directories.

See, this is wired up in the top Makefile in an ugly way, and you are struggling
in suppressing issues, where you can never do it in the right way.

Signature · 2 lines

Best Regards
Masahiro Yamada

Jiri Olsa

2021-02-12 13:37 UTC #20 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

On Fri, Feb 12, 2021 at 12:30:45PM +0900, Masahiro Yamada wrote:

SNIP

Quoted 15 lines

I expected this kind of mess
when I saw 33a57ce0a54d498275f432db04850001175dfdfa
The tools/ directory is a completely different world governed by a different build system
(no, not a build system, but a collection of adhoc makefile code)
All the other programs used during the kernel build are located under scripts/, and can be built with a simple syntax, and cleaned up correctly. It is simple, clean and robust.

Quoted 6 lines

objtool is the first alien that opt out Kbuild, and this is the second one.
It is scary to mix up two different things, which run in different working directories.

would you see any way out? apart from changing resolve_btfids to use Kbuild.. there are some dependencies we'd need to change as well and they are used by other tools.. probably it'd end up with all or nothing scenario

Quoted 5 lines

See, this is wired up in the top Makefile in an ugly way, and you are struggling
in suppressing issues, where you can never do it in the right way.

maybe we could move it out of top makefile into separate one, that would handle all the related mess

jirka

Andrii Nakryiko

2021-02-10 19:24 UTC #21 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

In reply to Nathan Chancellor

Quoted 53 lines · Nathan Chancellor

Quoted 51 lines · Andrii Nakryiko
Quoted 49 lines · Nathan Chancellor
Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.

Quoted 6 lines

Quoted 4 lines
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.
Odd, this reproduces for me on a completely clean checkout of bpf-next:

my bad, I was trying it on a branch that didn't have Jiri's patches, sorry about that.

Quoted 15 lines

$ git clone --depth=1 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
$ cd bpf-next
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/tmp/bpf-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
I do not really see how this could be environment related. It seems like this comes from tools/scripts/Makefile.include, where there is no guarantee that $(O) is created before being used like in the main Makefile?
Cheers,
Nathan

Jiri Olsa

2021-02-10 18:17 UTC #22 lore raw

Re: [PATCH bpf-next 4/4] kbuild: Add resolve_btfids clean to root clean target

In reply to Andrii Nakryiko

Quoted 47 lines · Andrii Nakryiko

Quoted 46 lines · Nathan Chancellor
Quoted 38 lines · Jiri Olsa
The resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Invoking the the resolve_btfids clean as part of root 'make clean'.
Acked-by Song Liu <[email protected]>
Signed-off-by Jiri Olsa <[email protected]>
---
 Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -2
index b0e4767735dc..159d9592b587 100644
@@ -1086,6 +1086,11 @@ ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+PHONY += resolve_btfids_clean
+
+resolve_btfids_clean:
+     $(Q)$(MAKE) -sC $(srctree)/tools/bpf/resolve_btfids O=$(abspath $(objtree))/tools/bpf/resolve_btfids clean
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
   ifeq ($(has_libelf),1)
@@ -1495,7 +1500,7 @@ vmlinuxclean:
      $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
      $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+clean: archclean vmlinuxclean resolve_btfids_clean
 
 # mrproper - Delete all generated files, including .config
 #
--
Quoted 2 lines
2.26.2
This breaks running distclean on a clean tree (my script just unconditionally runs distclean regardless of the tree state):

Quoted 7 lines

Quoted 3 lines
$ make -s O=build distclean
../../scripts/Makefile.include:4: *** O=/home/nathan/cbl/src/linux-next/build/tools/bpf/resolve_btfids does not exist.  Stop.
Can't reproduce it. It works in all kinds of variants (relative and absolute O=, clean and not clean trees, etc). Jiri, please check as well.

I can reproduce if I set O=XXX to directory that does not contain kernel build

	$ mkdir /tmp/krava
	$ make O=/tmp/krava distclean
	make[1]: Entering directory '/tmp/krava'
	../../scripts/Makefile.include:4: *** O=/tmp/krava/tools/bpf/resolve_btfids does not exist.  Stop.
	make[1]: *** [/home/jolsa/linux/Makefile:1092: resolve_btfids_clean] Error 2
	make[1]: Leaving directory '/tmp/krava'
	make: *** [Makefile:185: __sub-make] Error 2

will check on fix

jirka

Andrii Nakryiko

2021-02-06 02:58 UTC #23 lore raw

In reply to Jiri Olsa

Quoted 5 lines · Jiri Olsa

hi,
resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.

Quoted 10 lines

v2 changes:
- add Song's acks on patches 1 and 4 (others changed) [Song]
- add missing / [Andrii]
- change srctree variable initialization [Andrii]
- shifted ifdef for clean target [Andrii]
thanks,
jirka

FYI, your patch #2 didn't make it into the mailing list (see [0]). So maybe wait for a bit and if it doesn't arrive, re-submit?

[0] https://patchwork.kernel.org/user/todo/netdevbpf/?series=428711&delegate=121173&state=*

Quoted 12 lines

---
Jiri Olsa (4):
tools/resolve_btfids: Build libbpf and libsubcmd in separate directories tools/resolve_btfids: Check objects before removing tools/resolve_btfids: Set srctree variable unconditionally
kbuild: Add resolve_btfids clean to root clean target
 Makefile                            |  7 ++++++-
 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 44 ++++++++++++++++++++++----------------------
 3 files changed, 28 insertions(+), 25 deletions(-)

Jiri Olsa

2021-02-06 02:57 UTC #24 lore raw

Quoted 17 lines · Andrii Nakryiko

Quoted 5 lines · Jiri Olsa
hi,
resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Quoted 10 lines
v2 changes:
- add Song's acks on patches 1 and 4 (others changed) [Song]
- add missing / [Andrii]
- change srctree variable initialization [Andrii]
- shifted ifdef for clean target [Andrii]
thanks,
jirka

Quoted 4 lines

FYI, your patch #2 didn't make it into the mailing list (see [0]). So maybe wait for a bit and if it doesn't arrive, re-submit?
[0] https://patchwork.kernel.org/user/todo/netdevbpf/?series=428711&delegate=121173&state=*

hum and lore shows just 1 and 4
https://lore.kernel.org/bpf/[email protected]/

I'll check and resent later

thanks,
jirka

Quoted 14 lines

Quoted 13 lines
---
Jiri Olsa (4):
tools/resolve_btfids: Build libbpf and libsubcmd in separate directories tools/resolve_btfids: Check objects before removing tools/resolve_btfids: Set srctree variable unconditionally
kbuild: Add resolve_btfids clean to root clean target
 Makefile                            |  7 ++++++-
 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 44 ++++++++++++++++++++++----------------------
 3 files changed, 28 insertions(+), 25 deletions(-)

Jiri Olsa

2021-02-06 03:26 UTC #25 lore raw

Quoted 21 lines · Jiri Olsa

Quoted 20 lines · Andrii Nakryiko
Quoted 5 lines · Jiri Olsa
hi,
resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
Quoted 10 lines
v2 changes:
- add Song's acks on patches 1 and 4 (others changed) [Song]
- add missing / [Andrii]
- change srctree variable initialization [Andrii]
- shifted ifdef for clean target [Andrii]
thanks,
jirka
FYI, your patch #2 didn't make it into the mailing list (see [0]). So maybe wait for a bit and if it doesn't arrive, re-submit?

Quoted 6 lines

Quoted 1 line
[0] https://patchwork.kernel.org/user/todo/netdevbpf/?series=428711&delegate=121173&state=*
hum and lore shows just 1 and 4
https://lore.kernel.org/bpf/[email protected]/
I'll check and resent later

it arrived

jirka

Andrii Nakryiko

Applied

2021-02-09 05:37 UTC #26 lore raw

In reply to Jiri Olsa

Quoted 22 lines · Jiri Olsa

hi,
resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
v2 changes:
- add Song's acks on patches 1 and 4 (others changed) [Song]
- add missing / [Andrii]
- change srctree variable initialization [Andrii]
- shifted ifdef for clean target [Andrii]
thanks,
jirka
---
Jiri Olsa (4):
tools/resolve_btfids: Build libbpf and libsubcmd in separate directories tools/resolve_btfids: Check objects before removing tools/resolve_btfids: Set srctree variable unconditionally
kbuild: Add resolve_btfids clean to root clean target

Quoted 5 lines

 Makefile                            |  7 ++++++-
 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 44 ++++++++++++++++++++++----------------------
 3 files changed, 28 insertions(+), 25 deletions(-)

I've applied the changes to the bpf-next tree. Thanks.

Next time please make sure that each patch in the series has a v2 tag in [PATCH] section, it was a bit confusing to figure out which one is the actual v2 version. Our tooling (CI) also expects the format [PATCH v2 bpf-next], so try not to merge v2 with PATCH.

Jiri Olsa

2021-02-09 09:35 UTC #27 lore raw

Quoted 29 lines · Andrii Nakryiko

Quoted 22 lines · Jiri Olsa
hi,
resolve_btfids tool is used during the kernel build, so we should clean it on kernel's make clean.
v2 changes:
- add Song's acks on patches 1 and 4 (others changed) [Song]
- add missing / [Andrii]
- change srctree variable initialization [Andrii]
- shifted ifdef for clean target [Andrii]
thanks,
jirka
---
Jiri Olsa (4):
tools/resolve_btfids: Build libbpf and libsubcmd in separate directories tools/resolve_btfids: Check objects before removing tools/resolve_btfids: Set srctree variable unconditionally
kbuild: Add resolve_btfids clean to root clean target
Quoted 5 lines
 Makefile                            |  7 ++++++-
 tools/bpf/resolve_btfids/.gitignore |  2 --
 tools/bpf/resolve_btfids/Makefile   | 44 ++++++++++++++++++++++----------------------
 3 files changed, 28 insertions(+), 25 deletions(-)

Quoted 7 lines

I've applied the changes to the bpf-next tree. Thanks.
Next time please make sure that each patch in the series has a v2 tag in [PATCH] section, it was a bit confusing to figure out which one is the actual v2 version. Our tooling (CI) also expects the format [PATCH v2 bpf-next], so try not to merge v2 with PATCH.

will do, thanks

jirka