kbuild: Parameterize kallsyms generation and correct reporting
PATCH 1/1 kbuild: Parameterize kallsyms generation and correct reporting
When kallsyms generation happens, temporary vmlinux outputs are linked but the quiet make output doesn't report it, giving the impression that the prior command is taking longer than expected.
Instead, report the KSYM step before the temporary linking. While at it, this consolidates the repeated "kallsyms generation step" into a single function and removes the existing copy/pasting.
--- scripts/link-vmlinux.sh | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-)
scripts/link-vmlinux.sh +20 -19
@@ -170,7 +170,6 @@ gen_btf() # Create ${2} .o file with all symbols from the ${1} object file kallsyms() { - info KSYM ${2} local kallsymopt; if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then
@@ -277,7 +276,22 @@ info MODINFO modules.builtin.modinfo ${OBJCOPY} -j .modinfo -O binary vmlinux.o modules.builtin.modinfo kallsymso="" +kallsymso_previous="" kallsyms_vmlinux="" + +# Perform one step in kallsyms generation, including temporary linking of +# vmlinux. +kallsyms_step() +{ + kallsymso_previous=${kallsymso} + kallsymso=.tmp_kallsyms${1}.o + kallsyms_vmlinux=.tmp_vmlinux${1} + + info KSYM ${kallsymso} + vmlinux_link "${kallsymso_previous}" ${kallsyms_vmlinux} + kallsyms ${kallsyms_vmlinux} ${kallsymso} +} + if [ -n "${CONFIG_KALLSYMS}" ]; then # kallsyms support
@@ -303,28 +317,15 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then # a) Verify that the System.map from vmlinux matches the map from # ${kallsymso}. - kallsymso=.tmp_kallsyms2.o - kallsyms_vmlinux=.tmp_vmlinux2 - - # step 1 - vmlinux_link "" .tmp_vmlinux1 - kallsyms .tmp_vmlinux1 .tmp_kallsyms1.o - - # step 2 - vmlinux_link .tmp_kallsyms1.o .tmp_vmlinux2 - kallsyms .tmp_vmlinux2 .tmp_kallsyms2.o + kallsyms_step 1 + kallsyms_step 2 # step 3 - size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" .tmp_kallsyms1.o) - size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" .tmp_kallsyms2.o) + size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso_previous}) + size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then - kallsymso=.tmp_kallsyms3.o - kallsyms_vmlinux=.tmp_vmlinux3 - - vmlinux_link .tmp_kallsyms2.o .tmp_vmlinux3 - - kallsyms .tmp_vmlinux3 .tmp_kallsyms3.o + kallsyms_step 3 fi fi
Signature · 5 lines
2.17.1 -- Kees Cook
Quoted 11 lines · Kees Cook
When kallsyms generation happens, temporary vmlinux outputs are linked but the quiet make output doesn't report it, giving the impression that the prior command is taking longer than expected.
Instead, report the KSYM step before the temporary linking. While at it, this consolidates the repeated "kallsyms generation step" into a single function and removes the existing copy/pasting.
Signed-off-by Kees Cook <[email protected]>---
Hmm, I did not notice this.
How about showing the link stage explicitly? (Is it too verbose?)
MODINFO modules.builtin.modinfo
LD .tmp_vmlinux1 KSYMS .tmp_kallsyms1.o LD .tmp_vmlinux2 KSYMS .tmp_kallsyms2.o LD vmlinux SORTEX vmlinux
If this verbosity is OK,
you can move 'info LD ${2}' into vmlinux_link()
Anyway, I like the clean-ups in this patch.
This is just my personal preference, but may I ask two cosmetic changes?
[1] Could you move kallsyms_step() between kallsyms() and mksysmap() ? I want to collect function definitions to the top of the script.
[2] Could you shorten 'kallsymso_previous' to 'kallsymso_prev' ?
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 6 lines · Masahiro Yamada
Quoted 5 lines · Kees Cook
When kallsyms generation happens, temporary vmlinux outputs are linked but the quiet make output doesn't report it, giving the impression that the prior command is taking longer than expected.
Quoted 8 lines
Quoted 6 lines
Instead, report the KSYM step before the temporary linking. While at it, this consolidates the repeated "kallsyms generation step" into a single function and removes the existing copy/pasting.
Signed-off-by Kees Cook <[email protected]>---
Hmm, I did not notice this.
He either until I was getting link errors from MODINFO :)
Quoted 10 lines
How about showing the link stage explicitly? (Is it too verbose?)
MODINFO modules.builtin.modinfo
LD .tmp_vmlinux1 KSYMS .tmp_kallsyms1.o LD .tmp_vmlinux2 KSYMS .tmp_kallsyms2.o LD vmlinux SORTEX vmlinux
I'm fine with this -- it's probably the right thing to do since getting vmlinux link errors fro KSYMS probably doesn't make sense either. ;)
Quoted 2 lines
If this verbosity is OK,
you can move 'info LD ${2}' intovmlinux_link()
Done.
Quoted 5 lines
Anyway, I like the clean-ups in this patch.
This is just my personal preference, but may I ask two cosmetic changes?
Quoted 7 lines
[1] Could you move
kallsyms_step()betweenkallsyms()andmksysmap()? I want to collect function definitions to the top of the script.[2] Could you shorten '
kallsymso_previous' to 'kallsymso_prev' ?
Sounds good. v2 sent.
Signature · 1 line
Kees Cook