Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 2 Mar 2020 20:48:56 -0800
From:   Kees Cook <keescook@chromium.org>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     Michal Marek <michal.lkml@markovi.net>,
        linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Alexei Starovoitov <ast@kernel.org>, bpf@vger.kernel.org
Subject: Re: [PATCH] kbuild: Remove debug info from kallsyms linking
Message-ID: <202003022046.4185359A@keescook>
References: <202002242114.CBED7F1@keescook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202002242114.CBED7F1@keescook>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Feb 24, 2020 at 09:16:17PM -0800, Kees Cook wrote:
> When CONFIG_DEBUG_INFO is enabled, the two kallsyms linking steps spend
> time collecting and writing the dwarf sections to the temporary output
> files. kallsyms does not need this information, and leaving it off
> halves their linking time. This is especially noticeable without
> CONFIG_DEBUG_INFO_REDUCED. The BTF linking stage, however, does still
> need those details.
> 
> Refactor the BTF and kallsyms generation stages slightly for more
> regularized temporary names. Skip debug during kallsyms links.
> 
> For a full debug info build with BTF, my link time goes from 1m06s to
> 0m54s, saving about 12 seconds, or 18%.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Ping. Masahiro what do you think of this? It saves me a fair bit of time
on the link stage... I bet the BPF folks would be interested too. :)

-Kees

> ---
>  scripts/link-vmlinux.sh | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index dd484e92752e..ac569e197bfa 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -63,12 +63,18 @@ vmlinux_link()
>  	local lds="${objtree}/${KBUILD_LDS}"
>  	local output=${1}
>  	local objects
> +	local strip_debug
>  
>  	info LD ${output}
>  
>  	# skip output file argument
>  	shift
>  
> +	# The kallsyms linking does not need debug symbols included.
> +	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> +		strip_debug=-Wl,--strip-debug
> +	fi
> +
>  	if [ "${SRCARCH}" != "um" ]; then
>  		objects="--whole-archive			\
>  			${KBUILD_VMLINUX_OBJS}			\
> @@ -79,6 +85,7 @@ vmlinux_link()
>  			${@}"
>  
>  		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
> +			${strip_debug#-Wl,}			\
>  			-o ${output}				\
>  			-T ${lds} ${objects}
>  	else
> @@ -91,6 +98,7 @@ vmlinux_link()
>  			${@}"
>  
>  		${CC} ${CFLAGS_vmlinux}				\
> +			${strip_debug}				\
>  			-o ${output}				\
>  			-Wl,-T,${lds}				\
>  			${objects}				\
> @@ -106,6 +114,8 @@ gen_btf()
>  {
>  	local pahole_ver
>  	local bin_arch
> +	local bin_format
> +	local bin_file
>  
>  	if ! [ -x "$(command -v ${PAHOLE})" ]; then
>  		echo >&2 "BTF: ${1}: pahole (${PAHOLE}) is not available"
> @@ -118,8 +128,9 @@ gen_btf()
>  		return 1
>  	fi
>  
> -	info "BTF" ${2}
>  	vmlinux_link ${1}
> +
> +	info "BTF" ${2}
>  	LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
>  
>  	# dump .BTF section into raw binary file to link with final vmlinux
> @@ -127,11 +138,12 @@ gen_btf()
>  		cut -d, -f1 | cut -d' ' -f2)
>  	bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \
>  		awk '{print $4}')
> +	bin_file=.btf.vmlinux.bin
>  	${OBJCOPY} --change-section-address .BTF=0 \
>  		--set-section-flags .BTF=alloc -O binary \
> -		--only-section=.BTF ${1} .btf.vmlinux.bin
> +		--only-section=.BTF ${1} $bin_file
>  	${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \
> -		--rename-section .data=.BTF .btf.vmlinux.bin ${2}
> +		--rename-section .data=.BTF $bin_file ${2}
>  }
>  
>  # Create ${2} .o file with all symbols from the ${1} object file
> @@ -166,8 +178,8 @@ kallsyms()
>  kallsyms_step()
>  {
>  	kallsymso_prev=${kallsymso}
> -	kallsymso=.tmp_kallsyms${1}.o
> -	kallsyms_vmlinux=.tmp_vmlinux${1}
> +	kallsyms_vmlinux=.tmp_vmlinux.kallsyms${1}
> +	kallsymso=${kallsyms_vmlinux}.o
>  
>  	vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o}
>  	kallsyms ${kallsyms_vmlinux} ${kallsymso}
> @@ -190,7 +202,6 @@ cleanup()
>  {
>  	rm -f .btf.*
>  	rm -f .tmp_System.map
> -	rm -f .tmp_kallsyms*
>  	rm -f .tmp_vmlinux*
>  	rm -f System.map
>  	rm -f vmlinux
> @@ -257,9 +268,8 @@ tr '\0' '\n' < modules.builtin.modinfo | sed -n 's/^[[:alnum:]:_]*\.file=//p' |
>  
>  btf_vmlinux_bin_o=""
>  if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then
> -	if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then
> -		btf_vmlinux_bin_o=.btf.vmlinux.bin.o
> -	else
> +	btf_vmlinux_bin_o=.btf.vmlinux.bin.o
> +	if ! gen_btf .tmp_vmlinux.btf $btf_vmlinux_bin_o ; then
>  		echo >&2 "Failed to generate BTF for vmlinux"
>  		echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF"
>  		exit 1
> -- 
> 2.20.1
> 
> 
> -- 
> Kees Cook

-- 
Kees Cook