On Wed, Aug 14, 2019 at 12:00:05AM +0900, Masahiro Yamada wrote: > On Tue, Aug 13, 2019 at 6:49 AM Kees Cook <keescook@xxxxxxxxxxxx> 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 <keescook@xxxxxxxxxxxx> > > --- > > 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