* [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py
@ 2019-02-19 9:33 Masahiro Yamada
2019-02-19 9:33 ` [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild Masahiro Yamada
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-19 9:33 UTC (permalink / raw)
To: linux-kbuild
Cc: Kieran Bingham, Masahiro Yamada, Michal Marek, Heiko Carstens,
linux-kernel
scripts/gdb/linux/constants.py is never used in the kernel build
process. There is no good reason to create it so early.
Get it out of the 'prepare' stage.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Kbuild | 10 ----------
Makefile | 11 +++++++++++
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/Kbuild b/Kbuild
index 65db5be..4cebcc7 100644
--- a/Kbuild
+++ b/Kbuild
@@ -6,7 +6,6 @@
# 2) Generate timeconst.h
# 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
# 4) Check for missing system calls
-# 5) Generate constants.py (may need bounds.h)
#####
# 1) Generate bounds.h
@@ -58,14 +57,5 @@ quiet_cmd_syscalls = CALL $<
missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
$(call cmd,syscalls)
-#####
-# 5) Generate constants for Python GDB integration
-#
-
-extra-$(CONFIG_GDB_SCRIPTS) += build_constants_py
-
-build_constants_py: $(timeconst-file) $(bounds-file)
- @$(MAKE) $(build)=scripts/gdb/linux $@
-
# Keep these three files during make clean
no-clean-files := $(bounds-file) $(offsets-file) $(timeconst-file)
diff --git a/Makefile b/Makefile
index 88db36b..26dbcb7 100644
--- a/Makefile
+++ b/Makefile
@@ -1513,6 +1513,17 @@ PHONY += $(DOC_TARGETS)
$(DOC_TARGETS): scripts_basic FORCE
$(Q)$(MAKE) $(build)=Documentation $@
+# Misc
+# ---------------------------------------------------------------------------
+
+PHONY += scripts_gdb
+scripts_gdb: prepare
+ $(Q)$(MAKE) $(build)=scripts/gdb/linux build_constants_py
+
+ifdef CONFIG_GDB_SCRIPTS
+all: scripts_gdb
+endif
+
else # KBUILD_EXTMOD
###
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
@ 2019-02-19 9:33 ` Masahiro Yamada
2019-02-27 11:33 ` Kieran Bingham
2019-02-19 9:33 ` [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts Masahiro Yamada
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-19 9:33 UTC (permalink / raw)
To: linux-kbuild
Cc: Kieran Bingham, Masahiro Yamada, Heiko Carstens, linux-kernel
Every time we add/remove a target, we need to touch the header part,
including renumbering. This is not so important information.
Numbering targets is rather misleading because they are not necessarily
generated in this order. For example, 1) and 2) can be executed
simultaneously when the -j option is given.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Kbuild | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/Kbuild b/Kbuild
index 4cebcc7..a07bbd6 100644
--- a/Kbuild
+++ b/Kbuild
@@ -1,14 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
#
# Kbuild for top-level directory of the kernel
-# This file takes care of the following:
-# 1) Generate bounds.h
-# 2) Generate timeconst.h
-# 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
-# 4) Check for missing system calls
#####
-# 1) Generate bounds.h
+# Generate bounds.h
bounds-file := include/generated/bounds.h
@@ -19,7 +14,7 @@ $(bounds-file): kernel/bounds.s FORCE
$(call filechk,offsets,__LINUX_BOUNDS_H__)
#####
-# 2) Generate timeconst.h
+# Generate timeconst.h
timeconst-file := include/generated/timeconst.h
@@ -31,8 +26,7 @@ $(timeconst-file): kernel/time/timeconst.bc FORCE
$(call filechk,gentimeconst)
#####
-# 3) Generate asm-offsets.h
-#
+# Generate asm-offsets.h
offsets-file := include/generated/asm-offsets.h
@@ -45,8 +39,7 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
$(call filechk,offsets,__ASM_OFFSETS_H__)
#####
-# 4) Check for missing system calls
-#
+# Check for missing system calls
always += missing-syscalls
targets += missing-syscalls
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
2019-02-19 9:33 ` [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild Masahiro Yamada
@ 2019-02-19 9:33 ` Masahiro Yamada
2019-02-27 11:44 ` Kieran Bingham
2019-02-19 9:33 ` [PATCH 4/5] kbuild: create symlink to vmlinux-gdb.py in scripts_gdb target Masahiro Yamada
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-19 9:33 UTC (permalink / raw)
To: linux-kbuild
Cc: Kieran Bingham, Masahiro Yamada, linux-kernel, Michal Marek, Jan Kiszka
Currently, Kbuild descends from scripts/Makefile to scripts/gdb/Makefile
just for creating symbolic links, but it does not need to do it so early.
Merge the two descending paths to simplify the code.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 2 +-
scripts/Makefile | 3 +--
scripts/gdb/linux/Makefile | 9 +++------
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 26dbcb7..a5762c6 100644
--- a/Makefile
+++ b/Makefile
@@ -1518,7 +1518,7 @@ $(DOC_TARGETS): scripts_basic FORCE
PHONY += scripts_gdb
scripts_gdb: prepare
- $(Q)$(MAKE) $(build)=scripts/gdb/linux build_constants_py
+ $(Q)$(MAKE) $(build)=scripts/gdb
ifdef CONFIG_GDB_SCRIPTS
all: scripts_gdb
diff --git a/scripts/Makefile b/scripts/Makefile
index feb1f71..9d442ee 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -39,7 +39,6 @@ build_unifdef: $(obj)/unifdef
subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
subdir-$(CONFIG_MODVERSIONS) += genksyms
subdir-$(CONFIG_SECURITY_SELINUX) += selinux
-subdir-$(CONFIG_GDB_SCRIPTS) += gdb
# Let clean descend into subdirs
-subdir- += basic dtc kconfig mod package
+subdir- += basic dtc gdb kconfig mod package
diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
index aba23be..7545806 100644
--- a/scripts/gdb/linux/Makefile
+++ b/scripts/gdb/linux/Makefile
@@ -14,11 +14,8 @@ quiet_cmd_gen_constants_py = GEN $@
$(CPP) -E -x c -P $(c_flags) $< > $@ ;\
sed -i '1,/<!-- end-c-headers -->/d;' $@
-targets += constants.py
-$(obj)/constants.py: $(SRCTREE)/$(obj)/constants.py.in FORCE
+extra-y += constants.py
+$(obj)/constants.py: $(src)/constants.py.in FORCE
$(call if_changed_dep,gen_constants_py)
-build_constants_py: $(obj)/constants.py
- @:
-
-clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py) $(obj)/constants.py
+clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] kbuild: create symlink to vmlinux-gdb.py in scripts_gdb target
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
2019-02-19 9:33 ` [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild Masahiro Yamada
2019-02-19 9:33 ` [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts Masahiro Yamada
@ 2019-02-19 9:33 ` Masahiro Yamada
2019-02-27 11:48 ` Kieran Bingham
2019-02-19 9:33 ` [PATCH 5/5] scripts/gdb: refactor rules for symlink creation Masahiro Yamada
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-19 9:33 UTC (permalink / raw)
To: linux-kbuild; +Cc: Kieran Bingham, Masahiro Yamada, Michal Marek, linux-kernel
It is weird to create gdb stuff as a side-effect of vmlinux.
Move it to a more relevant place.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index a5762c6..0459260 100644
--- a/Makefile
+++ b/Makefile
@@ -1015,9 +1015,6 @@ cmd_link-vmlinux = \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
-ifdef CONFIG_GDB_SCRIPTS
- $(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
-endif
+$(call if_changed,link-vmlinux)
targets := vmlinux
@@ -1519,6 +1516,7 @@ $(DOC_TARGETS): scripts_basic FORCE
PHONY += scripts_gdb
scripts_gdb: prepare
$(Q)$(MAKE) $(build)=scripts/gdb
+ $(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
ifdef CONFIG_GDB_SCRIPTS
all: scripts_gdb
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/5] scripts/gdb: refactor rules for symlink creation
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
` (2 preceding siblings ...)
2019-02-19 9:33 ` [PATCH 4/5] kbuild: create symlink to vmlinux-gdb.py in scripts_gdb target Masahiro Yamada
@ 2019-02-19 9:33 ` Masahiro Yamada
2019-02-27 11:55 ` Kieran Bingham
2019-02-27 11:31 ` [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Kieran Bingham
2019-02-27 12:42 ` Masahiro Yamada
5 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-19 9:33 UTC (permalink / raw)
To: linux-kbuild; +Cc: Kieran Bingham, Masahiro Yamada, Jan Kiszka, linux-kernel
gdb-scripts is not a real object, but (ab)used like a phony target.
Rewrite the code in a more Kbuild-ish way. Add symlinks to extra-y
and use if_changed.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/gdb/linux/Makefile | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
index 7545806..3df395a 100644
--- a/scripts/gdb/linux/Makefile
+++ b/scripts/gdb/linux/Makefile
@@ -1,13 +1,17 @@
# SPDX-License-Identifier: GPL-2.0
-always := gdb-scripts
-SRCTREE := $(abspath $(srctree))
-
-$(obj)/gdb-scripts:
ifneq ($(KBUILD_SRC),)
- $(Q)ln -fsn $(SRCTREE)/$(obj)/*.py $(objtree)/$(obj)
+
+symlinks := $(patsubst $(srctree)/$(src)/%,%,$(wildcard $(srctree)/$(src)/*.py))
+
+quiet_cmd_symlink = SYMLINK $@
+ cmd_symlink = ln -fsn $(patsubst $(obj)/%,$(abspath $(srctree))/$(src)/%,$@) $@
+
+extra-y += $(symlinks)
+$(addprefix $(obj)/, $(symlinks)): FORCE
+ $(call if_changed,symlink)
+
endif
- @:
quiet_cmd_gen_constants_py = GEN $@
cmd_gen_constants_py = \
@@ -18,4 +22,4 @@ extra-y += constants.py
$(obj)/constants.py: $(src)/constants.py.in FORCE
$(call if_changed_dep,gen_constants_py)
-clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
+clean-files := *.pyc *.pyo
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
` (3 preceding siblings ...)
2019-02-19 9:33 ` [PATCH 5/5] scripts/gdb: refactor rules for symlink creation Masahiro Yamada
@ 2019-02-27 11:31 ` Kieran Bingham
2019-02-27 12:42 ` Masahiro Yamada
5 siblings, 0 replies; 14+ messages in thread
From: Kieran Bingham @ 2019-02-27 11:31 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild; +Cc: Michal Marek, Heiko Carstens, linux-kernel
Hi Yamada-san
Thank you for the patch,
On 19/02/2019 09:33, Masahiro Yamada wrote:
> scripts/gdb/linux/constants.py is never used in the kernel build
> process. There is no good reason to create it so early.
>
I agree, I originally put it here to ensure it was only built after
bounds.h (following the asm-offsets.h example I expect). It has no
requirement to be built in this stage itself.
Depending on the prepare stage instead sounds a lot more reasonable.
Thanks
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Get it out of the 'prepare' stage.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Kbuild | 10 ----------
> Makefile | 11 +++++++++++
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/Kbuild b/Kbuild
> index 65db5be..4cebcc7 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -6,7 +6,6 @@
> # 2) Generate timeconst.h
> # 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
> # 4) Check for missing system calls
> -# 5) Generate constants.py (may need bounds.h)
>
> #####
> # 1) Generate bounds.h
> @@ -58,14 +57,5 @@ quiet_cmd_syscalls = CALL $<
> missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
> $(call cmd,syscalls)
>
> -#####
> -# 5) Generate constants for Python GDB integration
> -#
> -
> -extra-$(CONFIG_GDB_SCRIPTS) += build_constants_py
> -
> -build_constants_py: $(timeconst-file) $(bounds-file)
> - @$(MAKE) $(build)=scripts/gdb/linux $@
> -
> # Keep these three files during make clean
> no-clean-files := $(bounds-file) $(offsets-file) $(timeconst-file)
> diff --git a/Makefile b/Makefile
> index 88db36b..26dbcb7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1513,6 +1513,17 @@ PHONY += $(DOC_TARGETS)
> $(DOC_TARGETS): scripts_basic FORCE
> $(Q)$(MAKE) $(build)=Documentation $@
>
> +# Misc
> +# ---------------------------------------------------------------------------
> +
> +PHONY += scripts_gdb
> +scripts_gdb: prepare
> + $(Q)$(MAKE) $(build)=scripts/gdb/linux build_constants_py
> +
> +ifdef CONFIG_GDB_SCRIPTS
> +all: scripts_gdb
> +endif
> +
> else # KBUILD_EXTMOD
>
> ###
>
--
Regards
--
Kieran
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild
2019-02-19 9:33 ` [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild Masahiro Yamada
@ 2019-02-27 11:33 ` Kieran Bingham
0 siblings, 0 replies; 14+ messages in thread
From: Kieran Bingham @ 2019-02-27 11:33 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild; +Cc: Heiko Carstens, linux-kernel
Hi Yamada-san,
On 19/02/2019 09:33, Masahiro Yamada wrote:
> Every time we add/remove a target, we need to touch the header part,
> including renumbering. This is not so important information.
>
> Numbering targets is rather misleading because they are not necessarily
> generated in this order. For example, 1) and 2) can be executed
> simultaneously when the -j option is given.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Sounds reasonable to me.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>
> Kbuild | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/Kbuild b/Kbuild
> index 4cebcc7..a07bbd6 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -1,14 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> #
> # Kbuild for top-level directory of the kernel
> -# This file takes care of the following:
> -# 1) Generate bounds.h
> -# 2) Generate timeconst.h
> -# 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
> -# 4) Check for missing system calls
>
> #####
> -# 1) Generate bounds.h
> +# Generate bounds.h
>
> bounds-file := include/generated/bounds.h
>
> @@ -19,7 +14,7 @@ $(bounds-file): kernel/bounds.s FORCE
> $(call filechk,offsets,__LINUX_BOUNDS_H__)
>
> #####
> -# 2) Generate timeconst.h
> +# Generate timeconst.h
>
> timeconst-file := include/generated/timeconst.h
>
> @@ -31,8 +26,7 @@ $(timeconst-file): kernel/time/timeconst.bc FORCE
> $(call filechk,gentimeconst)
>
> #####
> -# 3) Generate asm-offsets.h
> -#
> +# Generate asm-offsets.h
>
> offsets-file := include/generated/asm-offsets.h
>
> @@ -45,8 +39,7 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
> $(call filechk,offsets,__ASM_OFFSETS_H__)
>
> #####
> -# 4) Check for missing system calls
> -#
> +# Check for missing system calls
>
> always += missing-syscalls
> targets += missing-syscalls
>
--
--
Kieran
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts
2019-02-19 9:33 ` [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts Masahiro Yamada
@ 2019-02-27 11:44 ` Kieran Bingham
2019-02-27 12:38 ` Masahiro Yamada
0 siblings, 1 reply; 14+ messages in thread
From: Kieran Bingham @ 2019-02-27 11:44 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild; +Cc: linux-kernel, Michal Marek, Jan Kiszka
Hi Yamada-san,
On 19/02/2019 09:33, Masahiro Yamada wrote:
> Currently, Kbuild descends from scripts/Makefile to scripts/gdb/Makefile
> just for creating symbolic links, but it does not need to do it so early.
>
> Merge the two descending paths to simplify the code.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Makefile | 2 +-
> scripts/Makefile | 3 +--
> scripts/gdb/linux/Makefile | 9 +++------
> 3 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 26dbcb7..a5762c6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1518,7 +1518,7 @@ $(DOC_TARGETS): scripts_basic FORCE
>
> PHONY += scripts_gdb
> scripts_gdb: prepare
> - $(Q)$(MAKE) $(build)=scripts/gdb/linux build_constants_py
> + $(Q)$(MAKE) $(build)=scripts/gdb
>
> ifdef CONFIG_GDB_SCRIPTS
> all: scripts_gdb
> diff --git a/scripts/Makefile b/scripts/Makefile
> index feb1f71..9d442ee 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -39,7 +39,6 @@ build_unifdef: $(obj)/unifdef
> subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
> subdir-$(CONFIG_MODVERSIONS) += genksyms
> subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> -subdir-$(CONFIG_GDB_SCRIPTS) += gdb
>
> # Let clean descend into subdirs
> -subdir- += basic dtc kconfig mod package
> +subdir- += basic dtc gdb kconfig mod package
> diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
> index aba23be..7545806 100644
> --- a/scripts/gdb/linux/Makefile
> +++ b/scripts/gdb/linux/Makefile
> @@ -14,11 +14,8 @@ quiet_cmd_gen_constants_py = GEN $@
> $(CPP) -E -x c -P $(c_flags) $< > $@ ;\
> sed -i '1,/<!-- end-c-headers -->/d;' $@
>
> -targets += constants.py
> -$(obj)/constants.py: $(SRCTREE)/$(obj)/constants.py.in FORCE
> +extra-y += constants.py
> +$(obj)/constants.py: $(src)/constants.py.in FORCE
> $(call if_changed_dep,gen_constants_py)
>
> -build_constants_py: $(obj)/constants.py
> - @:
> -
> -clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py) $(obj)/constants.py
> +clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
This looks like the generated file will no longer be cleaned on
distclean, if the build is 'in-tree'... Is that right? Or OK if so?
Perhaps I'm mis-understanding the "$(if $(KBUILD_SRC),*.py)" statement
intent?
As long as you're content with the result, and it's understood:
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
--
Regards
--
Kieran
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] kbuild: create symlink to vmlinux-gdb.py in scripts_gdb target
2019-02-19 9:33 ` [PATCH 4/5] kbuild: create symlink to vmlinux-gdb.py in scripts_gdb target Masahiro Yamada
@ 2019-02-27 11:48 ` Kieran Bingham
0 siblings, 0 replies; 14+ messages in thread
From: Kieran Bingham @ 2019-02-27 11:48 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild; +Cc: Michal Marek, linux-kernel
Hi Yamada-san,
On 19/02/2019 09:33, Masahiro Yamada wrote:
> It is weird to create gdb stuff as a side-effect of vmlinux.
>
> Move it to a more relevant place.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Makefile | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a5762c6..0459260 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1015,9 +1015,6 @@ cmd_link-vmlinux = \
> $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>
> vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
> -ifdef CONFIG_GDB_SCRIPTS
> - $(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
> -endif
My initial thought was, doesn't this have a dependency on the vmlinux
... but of course it doesn't. It's not symlinking to the output binary -
it's creating a helper link for GDB to know how to tie in the
scripts-gdb package. It doesn't require vmlinux to exist at all.
So yes, I agree this is fine.
> +$(call if_changed,link-vmlinux)
>
> targets := vmlinux
> @@ -1519,6 +1516,7 @@ $(DOC_TARGETS): scripts_basic FORCE
> PHONY += scripts_gdb
> scripts_gdb: prepare
> $(Q)$(MAKE) $(build)=scripts/gdb
> + $(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
and especially as we now have this convenient location for it.
This looks much better.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> ifdef CONFIG_GDB_SCRIPTS
> all: scripts_gdb
>
--
Regards
--
Kieran
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] scripts/gdb: refactor rules for symlink creation
2019-02-19 9:33 ` [PATCH 5/5] scripts/gdb: refactor rules for symlink creation Masahiro Yamada
@ 2019-02-27 11:55 ` Kieran Bingham
2019-02-27 12:32 ` Masahiro Yamada
0 siblings, 1 reply; 14+ messages in thread
From: Kieran Bingham @ 2019-02-27 11:55 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild; +Cc: Jan Kiszka, linux-kernel
Hi Yamada-san,
On 19/02/2019 09:33, Masahiro Yamada wrote:
> gdb-scripts is not a real object, but (ab)used like a phony target.
>
> Rewrite the code in a more Kbuild-ish way. Add symlinks to extra-y
> and use if_changed.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> scripts/gdb/linux/Makefile | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
> index 7545806..3df395a 100644
> --- a/scripts/gdb/linux/Makefile
> +++ b/scripts/gdb/linux/Makefile
> @@ -1,13 +1,17 @@
> # SPDX-License-Identifier: GPL-2.0
> -always := gdb-scripts
>
> -SRCTREE := $(abspath $(srctree))
> -
> -$(obj)/gdb-scripts:
> ifneq ($(KBUILD_SRC),)
> - $(Q)ln -fsn $(SRCTREE)/$(obj)/*.py $(objtree)/$(obj)
> +
> +symlinks := $(patsubst $(srctree)/$(src)/%,%,$(wildcard $(srctree)/$(src)/*.py))
> +
> +quiet_cmd_symlink = SYMLINK $@
> + cmd_symlink = ln -fsn $(patsubst $(obj)/%,$(abspath $(srctree))/$(src)/%,$@) $@
> +
> +extra-y += $(symlinks)
> +$(addprefix $(obj)/, $(symlinks)): FORCE
> + $(call if_changed,symlink)
> +
> endif
> - @:
>
> quiet_cmd_gen_constants_py = GEN $@
> cmd_gen_constants_py = \
> @@ -18,4 +22,4 @@ extra-y += constants.py
> $(obj)/constants.py: $(src)/constants.py.in FORCE
> $(call if_changed_dep,gen_constants_py)
>
> -clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
> +clean-files := *.pyc *.pyo
Perhaps this answers my earlier question.
I guess the extra-y hook is somehow handling the clean up of these files?
Aha - yes, I've just found it in
Documentation/kbuild/makefiles.txt:
> === 5 Kbuild clean infrastructure
> ...
> Kbuild knows targets listed in $(hostprogs-y), $(hostprogs-m), $(always), $(extra-y) and $(targets). They are all deleted during "make clean".
Perfect, so this is much better.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
--
Regards
--
Kieran
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] scripts/gdb: refactor rules for symlink creation
2019-02-27 11:55 ` Kieran Bingham
@ 2019-02-27 12:32 ` Masahiro Yamada
2019-02-27 13:02 ` Kieran Bingham
0 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-27 12:32 UTC (permalink / raw)
To: kieran.bingham
Cc: Linux Kbuild mailing list, Jan Kiszka, Linux Kernel Mailing List
Hi Kieran,
On Wed, Feb 27, 2019 at 8:56 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Yamada-san,
>
> On 19/02/2019 09:33, Masahiro Yamada wrote:
> > gdb-scripts is not a real object, but (ab)used like a phony target.
> >
> > Rewrite the code in a more Kbuild-ish way. Add symlinks to extra-y
> > and use if_changed.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > scripts/gdb/linux/Makefile | 18 +++++++++++-------
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
> > index 7545806..3df395a 100644
> > --- a/scripts/gdb/linux/Makefile
> > +++ b/scripts/gdb/linux/Makefile
> > @@ -1,13 +1,17 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -always := gdb-scripts
> >
> > -SRCTREE := $(abspath $(srctree))
> > -
> > -$(obj)/gdb-scripts:
> > ifneq ($(KBUILD_SRC),)
> > - $(Q)ln -fsn $(SRCTREE)/$(obj)/*.py $(objtree)/$(obj)
> > +
> > +symlinks := $(patsubst $(srctree)/$(src)/%,%,$(wildcard $(srctree)/$(src)/*.py))
> > +
> > +quiet_cmd_symlink = SYMLINK $@
> > + cmd_symlink = ln -fsn $(patsubst $(obj)/%,$(abspath $(srctree))/$(src)/%,$@) $@
> > +
> > +extra-y += $(symlinks)
> > +$(addprefix $(obj)/, $(symlinks)): FORCE
> > + $(call if_changed,symlink)
> > +
> > endif
> > - @:
> >
> > quiet_cmd_gen_constants_py = GEN $@
> > cmd_gen_constants_py = \
> > @@ -18,4 +22,4 @@ extra-y += constants.py
> > $(obj)/constants.py: $(src)/constants.py.in FORCE
> > $(call if_changed_dep,gen_constants_py)
> >
> > -clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
> > +clean-files := *.pyc *.pyo
>
> Perhaps this answers my earlier question.
> I guess the extra-y hook is somehow handling the clean up of these files?
>
> Aha - yes, I've just found it in
>
> Documentation/kbuild/makefiles.txt:
> > === 5 Kbuild clean infrastructure
> > ...
> > Kbuild knows targets listed in $(hostprogs-y), $(hostprogs-m), $(always), $(extra-y) and $(targets). They are all deleted during "make clean".
>
> Perfect, so this is much better.
Exactly.
If you are interested in the real code,
see scripts/Makefile.clean
__clean-files := $(extra-y) $(extra-m) $(extra-) \
$(always) $(targets) $(clean-files) \
$(hostprogs-y) $(hostprogs-m) $(hostprogs-) \
$(hostlibs-y) $(hostlibs-m) $(hostlibs-) \
$(hostcxxlibs-y) $(hostcxxlibs-m)
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> --
> Regards
> --
> Kieran
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts
2019-02-27 11:44 ` Kieran Bingham
@ 2019-02-27 12:38 ` Masahiro Yamada
0 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-27 12:38 UTC (permalink / raw)
To: kieran.bingham
Cc: Linux Kbuild mailing list, Linux Kernel Mailing List,
Michal Marek, Jan Kiszka
Hi Kieran,
On Wed, Feb 27, 2019 at 8:46 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Yamada-san,
>
> On 19/02/2019 09:33, Masahiro Yamada wrote:
> > Currently, Kbuild descends from scripts/Makefile to scripts/gdb/Makefile
> > just for creating symbolic links, but it does not need to do it so early.
> >
> > Merge the two descending paths to simplify the code.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > Makefile | 2 +-
> > scripts/Makefile | 3 +--
> > scripts/gdb/linux/Makefile | 9 +++------
> > 3 files changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 26dbcb7..a5762c6 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1518,7 +1518,7 @@ $(DOC_TARGETS): scripts_basic FORCE
> >
> > PHONY += scripts_gdb
> > scripts_gdb: prepare
> > - $(Q)$(MAKE) $(build)=scripts/gdb/linux build_constants_py
> > + $(Q)$(MAKE) $(build)=scripts/gdb
> >
> > ifdef CONFIG_GDB_SCRIPTS
> > all: scripts_gdb
> > diff --git a/scripts/Makefile b/scripts/Makefile
> > index feb1f71..9d442ee 100644
> > --- a/scripts/Makefile
> > +++ b/scripts/Makefile
> > @@ -39,7 +39,6 @@ build_unifdef: $(obj)/unifdef
> > subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
> > subdir-$(CONFIG_MODVERSIONS) += genksyms
> > subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > -subdir-$(CONFIG_GDB_SCRIPTS) += gdb
> >
> > # Let clean descend into subdirs
> > -subdir- += basic dtc kconfig mod package
> > +subdir- += basic dtc gdb kconfig mod package
> > diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
> > index aba23be..7545806 100644
> > --- a/scripts/gdb/linux/Makefile
> > +++ b/scripts/gdb/linux/Makefile
> > @@ -14,11 +14,8 @@ quiet_cmd_gen_constants_py = GEN $@
> > $(CPP) -E -x c -P $(c_flags) $< > $@ ;\
> > sed -i '1,/<!-- end-c-headers -->/d;' $@
> >
> > -targets += constants.py
> > -$(obj)/constants.py: $(SRCTREE)/$(obj)/constants.py.in FORCE
> > +extra-y += constants.py
> > +$(obj)/constants.py: $(src)/constants.py.in FORCE
> > $(call if_changed_dep,gen_constants_py)
> >
> > -build_constants_py: $(obj)/constants.py
> > - @:
> > -
> > -clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py) $(obj)/constants.py
> > +clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
>
>
> This looks like the generated file will no longer be cleaned on
> distclean, if the build is 'in-tree'... Is that right? Or OK if so?
As you noticed in 5/5, files listed in $(extra-y) are cleaned up.
So, I removed the now redundant $(obj)/constants.py from 'clean-files'.
> Perhaps I'm mis-understanding the "$(if $(KBUILD_SRC),*.py)" statement
> intent?
$(if $(KBUILD_SRC),*.py) will clean *.py only for out-of-tree build.
'*.py' files in the build directory are symbolic links.
'*.py' files in the source directory are check-in source files.
> As long as you're content with the result, and it's understood:
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
` (4 preceding siblings ...)
2019-02-27 11:31 ` [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Kieran Bingham
@ 2019-02-27 12:42 ` Masahiro Yamada
5 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2019-02-27 12:42 UTC (permalink / raw)
To: Linux Kbuild mailing list
Cc: Kieran Bingham, Michal Marek, Heiko Carstens, Linux Kernel Mailing List
On Tue, Feb 19, 2019 at 6:34 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> scripts/gdb/linux/constants.py is never used in the kernel build
> process. There is no good reason to create it so early.
>
> Get it out of the 'prepare' stage.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
Added Kieran's Reviewed-by, and applied to linux-kbuild.
>
> Kbuild | 10 ----------
> Makefile | 11 +++++++++++
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/Kbuild b/Kbuild
> index 65db5be..4cebcc7 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -6,7 +6,6 @@
> # 2) Generate timeconst.h
> # 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
> # 4) Check for missing system calls
> -# 5) Generate constants.py (may need bounds.h)
>
> #####
> # 1) Generate bounds.h
> @@ -58,14 +57,5 @@ quiet_cmd_syscalls = CALL $<
> missing-syscalls: scripts/checksyscalls.sh $(offsets-file) FORCE
> $(call cmd,syscalls)
>
> -#####
> -# 5) Generate constants for Python GDB integration
> -#
> -
> -extra-$(CONFIG_GDB_SCRIPTS) += build_constants_py
> -
> -build_constants_py: $(timeconst-file) $(bounds-file)
> - @$(MAKE) $(build)=scripts/gdb/linux $@
> -
> # Keep these three files during make clean
> no-clean-files := $(bounds-file) $(offsets-file) $(timeconst-file)
> diff --git a/Makefile b/Makefile
> index 88db36b..26dbcb7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1513,6 +1513,17 @@ PHONY += $(DOC_TARGETS)
> $(DOC_TARGETS): scripts_basic FORCE
> $(Q)$(MAKE) $(build)=Documentation $@
>
> +# Misc
> +# ---------------------------------------------------------------------------
> +
> +PHONY += scripts_gdb
> +scripts_gdb: prepare
> + $(Q)$(MAKE) $(build)=scripts/gdb/linux build_constants_py
> +
> +ifdef CONFIG_GDB_SCRIPTS
> +all: scripts_gdb
> +endif
> +
> else # KBUILD_EXTMOD
>
> ###
> --
> 2.7.4
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] scripts/gdb: refactor rules for symlink creation
2019-02-27 12:32 ` Masahiro Yamada
@ 2019-02-27 13:02 ` Kieran Bingham
0 siblings, 0 replies; 14+ messages in thread
From: Kieran Bingham @ 2019-02-27 13:02 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Jan Kiszka, Linux Kernel Mailing List
Hi Yamada-san,
On 27/02/2019 12:32, Masahiro Yamada wrote:
> Hi Kieran,
>
>
> On Wed, Feb 27, 2019 at 8:56 PM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
>>
>> Hi Yamada-san,
>>
>> On 19/02/2019 09:33, Masahiro Yamada wrote:
>>> gdb-scripts is not a real object, but (ab)used like a phony target.
>>>
>>> Rewrite the code in a more Kbuild-ish way. Add symlinks to extra-y
>>> and use if_changed.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>> scripts/gdb/linux/Makefile | 18 +++++++++++-------
>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
>>> index 7545806..3df395a 100644
>>> --- a/scripts/gdb/linux/Makefile
>>> +++ b/scripts/gdb/linux/Makefile
>>> @@ -1,13 +1,17 @@
>>> # SPDX-License-Identifier: GPL-2.0
>>> -always := gdb-scripts
>>>
>>> -SRCTREE := $(abspath $(srctree))
>>> -
>>> -$(obj)/gdb-scripts:
>>> ifneq ($(KBUILD_SRC),)
>>> - $(Q)ln -fsn $(SRCTREE)/$(obj)/*.py $(objtree)/$(obj)
>>> +
>>> +symlinks := $(patsubst $(srctree)/$(src)/%,%,$(wildcard $(srctree)/$(src)/*.py))
>>> +
>>> +quiet_cmd_symlink = SYMLINK $@
>>> + cmd_symlink = ln -fsn $(patsubst $(obj)/%,$(abspath $(srctree))/$(src)/%,$@) $@
>>> +
>>> +extra-y += $(symlinks)
>>> +$(addprefix $(obj)/, $(symlinks)): FORCE
>>> + $(call if_changed,symlink)
>>> +
>>> endif
>>> - @:
>>>
>>> quiet_cmd_gen_constants_py = GEN $@
>>> cmd_gen_constants_py = \
>>> @@ -18,4 +22,4 @@ extra-y += constants.py
>>> $(obj)/constants.py: $(src)/constants.py.in FORCE
>>> $(call if_changed_dep,gen_constants_py)
>>>
>>> -clean-files := *.pyc *.pyo $(if $(KBUILD_SRC),*.py)
>>> +clean-files := *.pyc *.pyo
>>
>> Perhaps this answers my earlier question.
>> I guess the extra-y hook is somehow handling the clean up of these files?
>>
>> Aha - yes, I've just found it in
>>
>> Documentation/kbuild/makefiles.txt:
>>> === 5 Kbuild clean infrastructure
>>> ...
>>> Kbuild knows targets listed in $(hostprogs-y), $(hostprogs-m), $(always), $(extra-y) and $(targets). They are all deleted during "make clean".
>>
>> Perfect, so this is much better.
>
> Exactly.
>
> If you are interested in the real code,
> see scripts/Makefile.clean
>
>
> __clean-files := $(extra-y) $(extra-m) $(extra-) \
> $(always) $(targets) $(clean-files) \
> $(hostprogs-y) $(hostprogs-m) $(hostprogs-) \
> $(hostlibs-y) $(hostlibs-m) $(hostlibs-) \
> $(hostcxxlibs-y) $(hostcxxlibs-m)
>
Thank you - that makes it very clear.
Thanks again for you work :)
--
Kieran
>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>> --
>> Regards
>> --
>> Kieran
>
>
>
--
Regards
--
Kieran
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-02-27 13:02 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-19 9:33 [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Masahiro Yamada
2019-02-19 9:33 ` [PATCH 2/5] kbuild: remove unimportant comments from ./Kbuild Masahiro Yamada
2019-02-27 11:33 ` Kieran Bingham
2019-02-19 9:33 ` [PATCH 3/5] scripts/gdb: do not descend into scripts/gdb from scripts Masahiro Yamada
2019-02-27 11:44 ` Kieran Bingham
2019-02-27 12:38 ` Masahiro Yamada
2019-02-19 9:33 ` [PATCH 4/5] kbuild: create symlink to vmlinux-gdb.py in scripts_gdb target Masahiro Yamada
2019-02-27 11:48 ` Kieran Bingham
2019-02-19 9:33 ` [PATCH 5/5] scripts/gdb: refactor rules for symlink creation Masahiro Yamada
2019-02-27 11:55 ` Kieran Bingham
2019-02-27 12:32 ` Masahiro Yamada
2019-02-27 13:02 ` Kieran Bingham
2019-02-27 11:31 ` [PATCH 1/5] scripts/gdb: delay generation of gdb constants.py Kieran Bingham
2019-02-27 12:42 ` Masahiro Yamada
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).