| Message ID | 20210305100212.818562-1-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: apply fixdep logic to link-vmlinux.sh | expand |
On Fri, Mar 5, 2021 at 7:02 PM Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > > The patch adding CONFIG_VMLINUX_MAP revealed a small defect in the > build system: link-vmlinux.sh takes decisions based on CONFIG_* > options, but changing one of those does not always lead to vmlinux > being linked again. > > For most of the CONFIG_* knobs referenced previously, this has > probably been hidden by those knobs also affecting some object file, > hence indirectly also vmlinux. > > But CONFIG_VMLINUX_MAP is only handled inside link-vmlinux.sh, and > changing CONFIG_VMLINUX_MAP=n to CONFIG_VMLINUX_MAP=y does not cause > the build system to re-link (and hence have vmlinux.map > emitted). Since that map file is mostly a debugging aid, this is > merely a nuisance which is easily worked around by just deleting > vmlinux and building again. > > But one could imagine other (possibly future) CONFIG options that > actually do affect the vmlinux binary but which are not captured > through some object file dependency. > > To fix this, make link-vmlinux.sh emit a .vmlinux.d file in the same > format as the dependency files generated by gcc, and apply the fixdep > logic to that. I've tested that this correctly works with both in-tree > and out-of-tree builds. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- Another person finally noticed this long-standing bug! Actually, I had known about this bug some years before, and was wondering how to fix this, but your patch is much more elegant than my idea. My original idea was to grep (or sed) CONFIG_ in scripts/link-vmlinux.sh, and surround the output by /* ... */ (or prefix //) into init/dummy.c so that fixdep handles this as a usual C code case. (But, I did not send a patch) But, you are right. There is no reason to restrict fixdep only to C source files. I did not come up with the idea to make fixdep scan this shell script directly. Applied to linux-kbuild. Thanks! > Makefile | 2 +- > scripts/link-vmlinux.sh | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index b18dbc634690..19d2f7fd088a 100644 > --- a/Makefile > +++ b/Makefile > @@ -1192,7 +1192,7 @@ cmd_link-vmlinux = \ > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE > - +$(call if_changed,link-vmlinux) > + +$(call if_changed_dep,link-vmlinux) > > targets := vmlinux > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index 855fd4e6f03e..7d4b7c6f01e8 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -312,6 +312,7 @@ cleanup() > rm -f vmlinux > rm -f vmlinux.map > rm -f vmlinux.o > + rm -f .vmlinux.d > } > > on_exit() > @@ -421,6 +422,7 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then > fi > > vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} > +echo "vmlinux: $0" > .vmlinux.d > > # fill in BTF IDs > if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then > -- > 2.29.2 > -- Best Regards Masahiro Yamada
On Fri, Mar 5, 2021 at 10:50 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Mar 5, 2021 at 7:02 PM Rasmus Villemoes > <linux@rasmusvillemoes.dk> wrote: > > > > The patch adding CONFIG_VMLINUX_MAP revealed a small defect in the > > build system: link-vmlinux.sh takes decisions based on CONFIG_* > > options, but changing one of those does not always lead to vmlinux > > being linked again. > > > > For most of the CONFIG_* knobs referenced previously, this has > > probably been hidden by those knobs also affecting some object file, > > hence indirectly also vmlinux. > > > > But CONFIG_VMLINUX_MAP is only handled inside link-vmlinux.sh, and > > changing CONFIG_VMLINUX_MAP=n to CONFIG_VMLINUX_MAP=y does not cause > > the build system to re-link (and hence have vmlinux.map > > emitted). Since that map file is mostly a debugging aid, this is > > merely a nuisance which is easily worked around by just deleting > > vmlinux and building again. > > > > But one could imagine other (possibly future) CONFIG options that > > actually do affect the vmlinux binary but which are not captured > > through some object file dependency. > > > > To fix this, make link-vmlinux.sh emit a .vmlinux.d file in the same > > format as the dependency files generated by gcc, and apply the fixdep > > logic to that. I've tested that this correctly works with both in-tree > > and out-of-tree builds. > > > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > > --- > I moved the code to the last line of scripts/link-vmlinux.sh, and added a comment, otherwise, the intent is obscure. diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 7d4b7c6f01e8..e9516bdfcc6f 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -422,7 +422,6 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then fi vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} -echo "vmlinux: $0" > .vmlinux.d # fill in BTF IDs if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then @@ -451,3 +450,6 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then exit 1 fi fi + +# For fixdep +echo "vmlinux: $0" > .vmlinux.d
diff --git a/Makefile b/Makefile index b18dbc634690..19d2f7fd088a 100644 --- a/Makefile +++ b/Makefile @@ -1192,7 +1192,7 @@ cmd_link-vmlinux = \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE - +$(call if_changed,link-vmlinux) + +$(call if_changed_dep,link-vmlinux) targets := vmlinux diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 855fd4e6f03e..7d4b7c6f01e8 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -312,6 +312,7 @@ cleanup() rm -f vmlinux rm -f vmlinux.map rm -f vmlinux.o + rm -f .vmlinux.d } on_exit() @@ -421,6 +422,7 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then fi vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o} +echo "vmlinux: $0" > .vmlinux.d # fill in BTF IDs if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
The patch adding CONFIG_VMLINUX_MAP revealed a small defect in the build system: link-vmlinux.sh takes decisions based on CONFIG_* options, but changing one of those does not always lead to vmlinux being linked again. For most of the CONFIG_* knobs referenced previously, this has probably been hidden by those knobs also affecting some object file, hence indirectly also vmlinux. But CONFIG_VMLINUX_MAP is only handled inside link-vmlinux.sh, and changing CONFIG_VMLINUX_MAP=n to CONFIG_VMLINUX_MAP=y does not cause the build system to re-link (and hence have vmlinux.map emitted). Since that map file is mostly a debugging aid, this is merely a nuisance which is easily worked around by just deleting vmlinux and building again. But one could imagine other (possibly future) CONFIG options that actually do affect the vmlinux binary but which are not captured through some object file dependency. To fix this, make link-vmlinux.sh emit a .vmlinux.d file in the same format as the dependency files generated by gcc, and apply the fixdep logic to that. I've tested that this correctly works with both in-tree and out-of-tree builds. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- Makefile | 2 +- scripts/link-vmlinux.sh | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)