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]>
---
scripts/link-vmlinux.sh | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 96b6c0233a61..ed52be015523 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -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
--
2.17.1
--
Kees Cook
On Tue, Aug 13, 2019 at 6:49 AM Kees Cook <[email protected]> wrote:
>
> 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' ?
--
Best Regards
Masahiro Yamada
On Wed, Aug 14, 2019 at 12:00:05AM +0900, Masahiro Yamada wrote:
> On Tue, Aug 13, 2019 at 6:49 AM Kees Cook <[email protected]> wrote:
> >
> > 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.
He either until I was getting link errors from MODINFO :)
> 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. ;)
> If this verbosity is OK,
> you can move 'info LD ${2}' into vmlinux_link()
Done.
> 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' ?
Sounds good. v2 sent.
--
Kees Cook