Linux kbuild/kconfig development
 help / color / mirror / Atom feed
* [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps
@ 2024-05-22 11:47 Masahiro Yamada
  2024-05-22 11:47 ` [PATCH 1/3] kbuild: refactor variables in scripts/link-vmlinux.sh Masahiro Yamada
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Masahiro Yamada @ 2024-05-22 11:47 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, bpf, Ard Biesheuvel, Masahiro Yamada, Arnd Bergmann,
	Nathan Chancellor, Nicolas Schier, linux-arch


 - Remove PROVIDE() in the linker script
 - Merge temporary vmlinux link steps for BTF and kallsyms



Masahiro Yamada (3):
  kbuild: refactor variables in scripts/link-vmlinux.sh
  kbuild: remove PROVIDE() for kallsyms symbols
  kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and
    CONFIG_KALLSYMS

 include/asm-generic/vmlinux.lds.h | 19 -------
 kernel/kallsyms_internal.h        |  5 --
 scripts/kallsyms.c                |  6 ---
 scripts/link-vmlinux.sh           | 87 ++++++++++++++++---------------
 4 files changed, 45 insertions(+), 72 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] kbuild: refactor variables in scripts/link-vmlinux.sh
  2024-05-22 11:47 [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Masahiro Yamada
@ 2024-05-22 11:47 ` Masahiro Yamada
  2024-05-22 11:47 ` [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols Masahiro Yamada
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2024-05-22 11:47 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, bpf, Ard Biesheuvel, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier

Clean up the variables in scripts/link-vmlinux.sh

 - Specify the extra objects directly in vmlinux_link()
 - Move the AS rule to kallsyms()
 - Set kallsymso and btf_vmlinux_bin_o where they are generated
 - Remove unneeded variable, kallsymso_prev

No functional change intended.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/link-vmlinux.sh | 48 +++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 46ce5d04dbeb..b16967d33f1c 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -45,7 +45,6 @@ info()
 
 # Link of vmlinux
 # ${1} - output file
-# ${2}, ${3}, ... - optional extra .o files
 vmlinux_link()
 {
 	local output=${1}
@@ -101,7 +100,7 @@ vmlinux_link()
 	${ld} ${ldflags} -o ${output}					\
 		${wl}--whole-archive ${objs} ${wl}--no-whole-archive	\
 		${wl}--start-group ${libs} ${wl}--end-group		\
-		$@ ${ldlibs}
+		${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}
 }
 
 # generate .BTF typeinfo from DWARF debuginfo
@@ -142,9 +141,11 @@ gen_btf()
 		et_rel='\1\0'
 	fi
 	printf "${et_rel}" | dd of=${2} conv=notrunc bs=1 seek=16 status=none
+
+	btf_vmlinux_bin_o=${2}
 }
 
-# Create ${2} .S file with all symbols from the ${1} object file
+# Create ${2}.o file with all symbols from the ${1} object file
 kallsyms()
 {
 	local kallsymopt;
@@ -165,27 +166,25 @@ kallsyms()
 		kallsymopt="${kallsymopt} --lto-clang"
 	fi
 
-	info KSYMS ${2}
-	scripts/kallsyms ${kallsymopt} ${1} > ${2}
+	info KSYMS "${2}.S"
+	scripts/kallsyms ${kallsymopt} "${1}" > "${2}.S"
+
+	info AS "${2}.o"
+	${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \
+	      ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o "${2}.o" "${2}.S"
+
+	kallsymso=${2}.o
 }
 
 # Perform one step in kallsyms generation, including temporary linking of
 # vmlinux.
 kallsyms_step()
 {
-	kallsymso_prev=${kallsymso}
 	kallsyms_vmlinux=.tmp_vmlinux.kallsyms${1}
-	kallsymso=${kallsyms_vmlinux}.o
-	kallsyms_S=${kallsyms_vmlinux}.S
 
-	vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o}
-	mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms
-	kallsyms ${kallsyms_vmlinux}.syms ${kallsyms_S}
-
-	info AS ${kallsymso}
-	${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \
-	      ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} \
-	      -c -o ${kallsymso} ${kallsyms_S}
+	vmlinux_link "${kallsyms_vmlinux}"
+	mksysmap "${kallsyms_vmlinux}" "${kallsyms_vmlinux}.syms"
+	kallsyms "${kallsyms_vmlinux}.syms" "${kallsyms_vmlinux}"
 }
 
 # Create map file with all symbols from ${1}
@@ -223,19 +222,17 @@ fi
 
 ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o
 
-btf_vmlinux_bin_o=""
+kallsymso=
+btf_vmlinux_bin_o=
+
 if is_enabled CONFIG_DEBUG_INFO_BTF; then
-	btf_vmlinux_bin_o=.btf.vmlinux.bin.o
-	if ! gen_btf .tmp_vmlinux.btf $btf_vmlinux_bin_o ; then
+	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
 	fi
 fi
 
-kallsymso=""
-kallsymso_prev=""
-kallsyms_vmlinux=""
 if is_enabled CONFIG_KALLSYMS; then
 
 	# kallsyms support
@@ -262,10 +259,9 @@ if is_enabled CONFIG_KALLSYMS; then
 	#     ${kallsymso}.
 
 	kallsyms_step 1
-	kallsyms_step 2
+	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
 
-	# step 3
-	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso_prev})
+	kallsyms_step 2
 	size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
 
 	if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
@@ -273,7 +269,7 @@ if is_enabled CONFIG_KALLSYMS; then
 	fi
 fi
 
-vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o}
+vmlinux_link vmlinux
 
 # fill in BTF IDs
 if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols
  2024-05-22 11:47 [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Masahiro Yamada
  2024-05-22 11:47 ` [PATCH 1/3] kbuild: refactor variables in scripts/link-vmlinux.sh Masahiro Yamada
@ 2024-05-22 11:47 ` Masahiro Yamada
  2024-05-23  9:31   ` Ard Biesheuvel
  2024-05-22 11:47 ` [PATCH 3/3] kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS Masahiro Yamada
  2024-05-28  9:42 ` [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Jiri Olsa
  3 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2024-05-22 11:47 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, bpf, Ard Biesheuvel, Masahiro Yamada, Arnd Bergmann,
	Nathan Chancellor, Nicolas Schier, linux-arch

This reimplements commit 951bcae6c5a0 ("kallsyms: Avoid weak references
for kallsyms symbols").

I am not a big fan of PROVIDE() because it always satisfies the linker
even in situations that should result in a link error. In other words,
it can potentially shift a compile-time error into a run-time error.

Duplicating kallsyms_* in vmlinux.lds.h also reduces maintainability.

As an alternative solution, this commit prepends one more kallsyms step.

    KSYMS   .tmp_vmlinux.kallsyms0.S          # added
    AS      .tmp_vmlinux.kallsyms0.o          # added
    LD      .tmp_vmlinux.btf
    BTF     .btf.vmlinux.bin.o
    LD      .tmp_vmlinux.kallsyms1
    NM      .tmp_vmlinux.kallsyms1.syms
    KSYMS   .tmp_vmlinux.kallsyms1.S
    AS      .tmp_vmlinux.kallsyms1.o
    LD      .tmp_vmlinux.kallsyms2
    NM      .tmp_vmlinux.kallsyms2.syms
    KSYMS   .tmp_vmlinux.kallsyms2.S
    AS      .tmp_vmlinux.kallsyms2.o
    LD      vmlinux

Step 0 takes /dev/null as input, and generates .tmp_vmlinux.kallsyms0.o,
which has a valid kallsyms format with the empty symbol list, and can be
linked to vmlinux. Since it is really small, the added compile-time cost
is negligible.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 include/asm-generic/vmlinux.lds.h | 19 -------------------
 kernel/kallsyms_internal.h        |  5 -----
 scripts/kallsyms.c                |  6 ------
 scripts/link-vmlinux.sh           | 10 ++++++++--
 4 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 5703526d6ebf..62b4cb0462e6 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -451,30 +451,11 @@
 #endif
 #endif
 
-/*
- * Some symbol definitions will not exist yet during the first pass of the
- * link, but are guaranteed to exist in the final link. Provide preliminary
- * definitions that will be superseded in the final link to avoid having to
- * rely on weak external linkage, which requires a GOT when used in position
- * independent code.
- */
-#define PRELIMINARY_SYMBOL_DEFINITIONS					\
-	PROVIDE(kallsyms_addresses = .);				\
-	PROVIDE(kallsyms_offsets = .);					\
-	PROVIDE(kallsyms_names = .);					\
-	PROVIDE(kallsyms_num_syms = .);					\
-	PROVIDE(kallsyms_relative_base = .);				\
-	PROVIDE(kallsyms_token_table = .);				\
-	PROVIDE(kallsyms_token_index = .);				\
-	PROVIDE(kallsyms_markers = .);					\
-	PROVIDE(kallsyms_seqs_of_names = .);
-
 /*
  * Read only Data
  */
 #define RO_DATA(align)							\
 	. = ALIGN((align));						\
-	PRELIMINARY_SYMBOL_DEFINITIONS					\
 	.rodata           : AT(ADDR(.rodata) - LOAD_OFFSET) {		\
 		__start_rodata = .;					\
 		*(.rodata) *(.rodata.*)					\
diff --git a/kernel/kallsyms_internal.h b/kernel/kallsyms_internal.h
index 85480274fc8f..925f2ab22639 100644
--- a/kernel/kallsyms_internal.h
+++ b/kernel/kallsyms_internal.h
@@ -4,11 +4,6 @@
 
 #include <linux/types.h>
 
-/*
- * These will be re-linked against their real values during the second link
- * stage. Preliminary values must be provided in the linker script using the
- * PROVIDE() directive so that the first link stage can complete successfully.
- */
 extern const unsigned long kallsyms_addresses[];
 extern const int kallsyms_offsets[];
 extern const u8 kallsyms_names[];
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 47978efe4797..fa53b5eef553 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -259,12 +259,6 @@ static void shrink_table(void)
 		}
 	}
 	table_cnt = pos;
-
-	/* When valid symbol is not registered, exit to error */
-	if (!table_cnt) {
-		fprintf(stderr, "No valid symbol.\n");
-		exit(1);
-	}
 }
 
 static void read_map(const char *in)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index b16967d33f1c..fe7db9a265ca 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -225,6 +225,11 @@ ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o
 kallsymso=
 btf_vmlinux_bin_o=
 
+if is_enabled CONFIG_KALLSYMS; then
+	# kallsyms step 0
+	kallsyms /dev/null .tmp_vmlinux.kallsyms0
+fi
+
 if is_enabled CONFIG_DEBUG_INFO_BTF; then
 	if ! gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then
 		echo >&2 "Failed to generate BTF for vmlinux"
@@ -237,9 +242,10 @@ if is_enabled CONFIG_KALLSYMS; then
 
 	# kallsyms support
 	# Generate section listing all symbols and add it into vmlinux
-	# It's a three step process:
+	# It's a four step process:
+	# 0)  Generate a dummy __kallsyms with empty symbol list.
 	# 1)  Link .tmp_vmlinux.kallsyms1 so it has all symbols and sections,
-	#     but __kallsyms is empty.
+	#     with a dummy __kallsyms.
 	#     Running kallsyms on that gives us .tmp_kallsyms1.o with
 	#     the right size
 	# 2)  Link .tmp_vmlinux.kallsyms2 so it now has a __kallsyms section of
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS
  2024-05-22 11:47 [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Masahiro Yamada
  2024-05-22 11:47 ` [PATCH 1/3] kbuild: refactor variables in scripts/link-vmlinux.sh Masahiro Yamada
  2024-05-22 11:47 ` [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols Masahiro Yamada
@ 2024-05-22 11:47 ` Masahiro Yamada
  2024-05-30 21:17   ` Andrii Nakryiko
  2024-05-28  9:42 ` [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Jiri Olsa
  3 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2024-05-22 11:47 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, bpf, Ard Biesheuvel, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier

CONFIG_DEBUG_INFO_BTF=y requires one additional link step.
(.tmp_vmlinux.btf)

CONFIG_KALLSYMS=y requires two additional link steps.
(.tmp_vmlinux.kallsyms1 and .tmp_vmlinux.kallsyms2)

Enabling both requires three additional link steps.

When CONFIG_DEBUG_INFO_BTF=y and CONFIG_KALLSYMS=y, the build step looks
as follows:

    KSYMS   .tmp_vmlinux.kallsyms0.S
    AS      .tmp_vmlinux.kallsyms0.o
    LD      .tmp_vmlinux.btf             # temp vmlinux for BTF
    BTF     .btf.vmlinux.bin.o
    LD      .tmp_vmlinux.kallsyms1       # temp vmlinux for kallsyms step 1
    NM      .tmp_vmlinux.kallsyms1.syms
    KSYMS   .tmp_vmlinux.kallsyms1.S
    AS      .tmp_vmlinux.kallsyms1.o
    LD      .tmp_vmlinux.kallsyms2       # temp vmlinux for kallsyms step 2
    NM      .tmp_vmlinux.kallsyms2.syms
    KSYMS   .tmp_vmlinux.kallsyms2.S
    AS      .tmp_vmlinux.kallsyms2.o
    LD      vmlinux                      # final vmlinux

This is redundant because the BTF generation and the kallsyms step 1 can
be performed against the same temporary vmlinux.

When both CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS are enabled, we can
reduce the number of link steps.

The build step will look as follows:

    KSYMS   .tmp_vmlinux0.kallsyms.S
    AS      .tmp_vmlinux0.kallsyms.o
    LD      .tmp_vmlinux1                # temp vmlinux for BTF and kallsyms step 1
    BTF     .tmp_vmlinux1.btf.o
    NM      .tmp_vmlinux1.syms
    KSYMS   .tmp_vmlinux1.kallsyms.S
    AS      .tmp_vmlinux1.kallsyms.o
    LD      .tmp_vmlinux2                # temp vmlinux for kallsyms step 2
    NM      .tmp_vmlinux2.syms
    KSYMS   .tmp_vmlinux2.kallsyms.S
    AS      .tmp_vmlinux2.kallsyms.o
    LD      vmlinux                      # final link

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/link-vmlinux.sh | 45 +++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index fe7db9a265ca..ea004aae6ce5 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -105,10 +105,10 @@ vmlinux_link()
 
 # generate .BTF typeinfo from DWARF debuginfo
 # ${1} - vmlinux image
-# ${2} - file to dump raw BTF data into
 gen_btf()
 {
 	local pahole_ver
+	local btf_data=${1}.btf.o
 
 	if ! [ -x "$(command -v ${PAHOLE})" ]; then
 		echo >&2 "BTF: ${1}: pahole (${PAHOLE}) is not available"
@@ -121,18 +121,16 @@ gen_btf()
 		return 1
 	fi
 
-	vmlinux_link ${1}
-
-	info "BTF" ${2}
+	info BTF "${btf_data}"
 	LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1}
 
-	# Create ${2} which contains just .BTF section but no symbols. Add
+	# Create ${btf_data} which contains just .BTF section but no symbols. Add
 	# SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
 	# deletes all symbols including __start_BTF and __stop_BTF, which will
 	# be redefined in the linker script. Add 2>/dev/null to suppress GNU
 	# objcopy warnings: "empty loadable segment detected at ..."
 	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
-		--strip-all ${1} ${2} 2>/dev/null
+		--strip-all ${1} "${btf_data}" 2>/dev/null
 	# Change e_type to ET_REL so that it can be used to link final vmlinux.
 	# GNU ld 2.35+ and lld do not allow an ET_EXEC input.
 	if is_enabled CONFIG_CPU_BIG_ENDIAN; then
@@ -140,9 +138,9 @@ gen_btf()
 	else
 		et_rel='\1\0'
 	fi
-	printf "${et_rel}" | dd of=${2} conv=notrunc bs=1 seek=16 status=none
+	printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none
 
-	btf_vmlinux_bin_o=${2}
+	btf_vmlinux_bin_o=${btf_data}
 }
 
 # Create ${2}.o file with all symbols from the ${1} object file
@@ -176,15 +174,13 @@ kallsyms()
 	kallsymso=${2}.o
 }
 
-# Perform one step in kallsyms generation, including temporary linking of
-# vmlinux.
-kallsyms_step()
+# Perform kallsyms for the given temporary vmlinux.
+sysmap_and_kallsyms()
 {
-	kallsyms_vmlinux=.tmp_vmlinux.kallsyms${1}
+	mksysmap "${1}" "${1}.syms"
+	kallsyms "${1}.syms" "${1}.kallsyms"
 
-	vmlinux_link "${kallsyms_vmlinux}"
-	mksysmap "${kallsyms_vmlinux}" "${kallsyms_vmlinux}.syms"
-	kallsyms "${kallsyms_vmlinux}.syms" "${kallsyms_vmlinux}"
+	kallsyms_sysmap=${1}.syms
 }
 
 # Create map file with all symbols from ${1}
@@ -226,12 +222,15 @@ kallsymso=
 btf_vmlinux_bin_o=
 
 if is_enabled CONFIG_KALLSYMS; then
-	# kallsyms step 0
-	kallsyms /dev/null .tmp_vmlinux.kallsyms0
+	kallsyms /dev/null .tmp_vmlinux0.kallsyms
+fi
+
+if is_enabled CONFIG_KALLSYMS || is_enabled CONFIG_DEBUG_INFO_BTF; then
+	vmlinux_link .tmp_vmlinux1
 fi
 
 if is_enabled CONFIG_DEBUG_INFO_BTF; then
-	if ! gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then
+	if ! gen_btf .tmp_vmlinux1; then
 		echo >&2 "Failed to generate BTF for vmlinux"
 		echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF"
 		exit 1
@@ -264,14 +263,16 @@ if is_enabled CONFIG_KALLSYMS; then
 	# a)  Verify that the System.map from vmlinux matches the map from
 	#     ${kallsymso}.
 
-	kallsyms_step 1
+	sysmap_and_kallsyms .tmp_vmlinux1
 	size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
 
-	kallsyms_step 2
+	vmlinux_link .tmp_vmlinux2
+	sysmap_and_kallsyms .tmp_vmlinux2
 	size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso})
 
 	if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then
-		kallsyms_step 3
+		vmlinux_link .tmp_vmlinux3
+		sysmap_and_kallsyms .tmp_vmlinux3
 	fi
 fi
 
@@ -295,7 +296,7 @@ fi
 
 # step a (see comment above)
 if is_enabled CONFIG_KALLSYMS; then
-	if ! cmp -s System.map ${kallsyms_vmlinux}.syms; then
+	if ! cmp -s System.map ${kallsyms_sysmap}; then
 		echo >&2 Inconsistent kallsyms data
 		echo >&2 'Try "make KALLSYMS_EXTRA_PASS=1" as a workaround'
 		exit 1
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols
  2024-05-22 11:47 ` [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols Masahiro Yamada
@ 2024-05-23  9:31   ` Ard Biesheuvel
  2024-05-25 16:43     ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2024-05-23  9:31 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, bpf, Arnd Bergmann, Nathan Chancellor,
	Nicolas Schier, linux-arch

On Wed, 22 May 2024 at 13:48, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This reimplements commit 951bcae6c5a0 ("kallsyms: Avoid weak references
> for kallsyms symbols").
>
> I am not a big fan of PROVIDE() because it always satisfies the linker
> even in situations that should result in a link error. In other words,
> it can potentially shift a compile-time error into a run-time error.
>

I don't disagree. However, I did realize that, in this particular
case, we could at least make the preliminary symbol definitions
conditional on CONFIG_KALLSYMS rather than always providing them.

This approach is also fine with me, though.


> Duplicating kallsyms_* in vmlinux.lds.h also reduces maintainability.
>
> As an alternative solution, this commit prepends one more kallsyms step.
>
>     KSYMS   .tmp_vmlinux.kallsyms0.S          # added
>     AS      .tmp_vmlinux.kallsyms0.o          # added
>     LD      .tmp_vmlinux.btf
>     BTF     .btf.vmlinux.bin.o
>     LD      .tmp_vmlinux.kallsyms1
>     NM      .tmp_vmlinux.kallsyms1.syms
>     KSYMS   .tmp_vmlinux.kallsyms1.S
>     AS      .tmp_vmlinux.kallsyms1.o
>     LD      .tmp_vmlinux.kallsyms2
>     NM      .tmp_vmlinux.kallsyms2.syms
>     KSYMS   .tmp_vmlinux.kallsyms2.S
>     AS      .tmp_vmlinux.kallsyms2.o
>     LD      vmlinux
>
> Step 0 takes /dev/null as input, and generates .tmp_vmlinux.kallsyms0.o,
> which has a valid kallsyms format with the empty symbol list, and can be
> linked to vmlinux. Since it is really small, the added compile-time cost
> is negligible.
>

OK, so the number of linker invocations is the same, right? The
difference is that the kallsyms symbol references are satisfied by a
dummy object?

That seems reasonable to me.

For the series,

Acked-by: Ard Biesheuvel <ardb@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols
  2024-05-23  9:31   ` Ard Biesheuvel
@ 2024-05-25 16:43     ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2024-05-25 16:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-kbuild, linux-kernel, bpf, Arnd Bergmann, Nathan Chancellor,
	Nicolas Schier, linux-arch

On Thu, May 23, 2024 at 6:32 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 22 May 2024 at 13:48, Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > This reimplements commit 951bcae6c5a0 ("kallsyms: Avoid weak references
> > for kallsyms symbols").
> >
> > I am not a big fan of PROVIDE() because it always satisfies the linker
> > even in situations that should result in a link error. In other words,
> > it can potentially shift a compile-time error into a run-time error.
> >
>
> I don't disagree. However, I did realize that, in this particular
> case, we could at least make the preliminary symbol definitions
> conditional on CONFIG_KALLSYMS rather than always providing them.


Fair enough. I am fine with dropping this statement.




>
> This approach is also fine with me, though.
>
>
> > Duplicating kallsyms_* in vmlinux.lds.h also reduces maintainability.
> >
> > As an alternative solution, this commit prepends one more kallsyms step.
> >
> >     KSYMS   .tmp_vmlinux.kallsyms0.S          # added
> >     AS      .tmp_vmlinux.kallsyms0.o          # added
> >     LD      .tmp_vmlinux.btf
> >     BTF     .btf.vmlinux.bin.o
> >     LD      .tmp_vmlinux.kallsyms1
> >     NM      .tmp_vmlinux.kallsyms1.syms
> >     KSYMS   .tmp_vmlinux.kallsyms1.S
> >     AS      .tmp_vmlinux.kallsyms1.o
> >     LD      .tmp_vmlinux.kallsyms2
> >     NM      .tmp_vmlinux.kallsyms2.syms
> >     KSYMS   .tmp_vmlinux.kallsyms2.S
> >     AS      .tmp_vmlinux.kallsyms2.o
> >     LD      vmlinux
> >
> > Step 0 takes /dev/null as input, and generates .tmp_vmlinux.kallsyms0.o,
> > which has a valid kallsyms format with the empty symbol list, and can be
> > linked to vmlinux. Since it is really small, the added compile-time cost
> > is negligible.
> >
>
> OK, so the number of linker invocations is the same, right? The
> difference is that the kallsyms symbol references are satisfied by a
> dummy object?


Correct.

In 3/3, I even reduce the number of link steps
when both CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS are enabled.





>
> That seems reasonable to me.
>
> For the series,
>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps
  2024-05-22 11:47 [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Masahiro Yamada
                   ` (2 preceding siblings ...)
  2024-05-22 11:47 ` [PATCH 3/3] kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS Masahiro Yamada
@ 2024-05-28  9:42 ` Jiri Olsa
  3 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2024-05-28  9:42 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, bpf, Ard Biesheuvel, Arnd Bergmann,
	Nathan Chancellor, Nicolas Schier, linux-arch

On Wed, May 22, 2024 at 08:47:52PM +0900, Masahiro Yamada wrote:
> 
>  - Remove PROVIDE() in the linker script
>  - Merge temporary vmlinux link steps for BTF and kallsyms
> 
> 
> 
> Masahiro Yamada (3):
>   kbuild: refactor variables in scripts/link-vmlinux.sh
>   kbuild: remove PROVIDE() for kallsyms symbols
>   kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and
>     CONFIG_KALLSYMS

lgtm, fyi I ran bpf CI on top of this change and passed

https://github.com/kernel-patches/bpf/pull/7104

jirka

> 
>  include/asm-generic/vmlinux.lds.h | 19 -------
>  kernel/kallsyms_internal.h        |  5 --
>  scripts/kallsyms.c                |  6 ---
>  scripts/link-vmlinux.sh           | 87 ++++++++++++++++---------------
>  4 files changed, 45 insertions(+), 72 deletions(-)
> 
> -- 
> 2.40.1
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS
  2024-05-22 11:47 ` [PATCH 3/3] kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS Masahiro Yamada
@ 2024-05-30 21:17   ` Andrii Nakryiko
  0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2024-05-30 21:17 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, bpf, Ard Biesheuvel,
	Nathan Chancellor, Nicolas Schier

On Wed, May 22, 2024 at 4:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> CONFIG_DEBUG_INFO_BTF=y requires one additional link step.
> (.tmp_vmlinux.btf)
>
> CONFIG_KALLSYMS=y requires two additional link steps.
> (.tmp_vmlinux.kallsyms1 and .tmp_vmlinux.kallsyms2)
>
> Enabling both requires three additional link steps.
>
> When CONFIG_DEBUG_INFO_BTF=y and CONFIG_KALLSYMS=y, the build step looks
> as follows:
>
>     KSYMS   .tmp_vmlinux.kallsyms0.S
>     AS      .tmp_vmlinux.kallsyms0.o
>     LD      .tmp_vmlinux.btf             # temp vmlinux for BTF
>     BTF     .btf.vmlinux.bin.o
>     LD      .tmp_vmlinux.kallsyms1       # temp vmlinux for kallsyms step 1
>     NM      .tmp_vmlinux.kallsyms1.syms
>     KSYMS   .tmp_vmlinux.kallsyms1.S
>     AS      .tmp_vmlinux.kallsyms1.o
>     LD      .tmp_vmlinux.kallsyms2       # temp vmlinux for kallsyms step 2
>     NM      .tmp_vmlinux.kallsyms2.syms
>     KSYMS   .tmp_vmlinux.kallsyms2.S
>     AS      .tmp_vmlinux.kallsyms2.o
>     LD      vmlinux                      # final vmlinux
>
> This is redundant because the BTF generation and the kallsyms step 1 can
> be performed against the same temporary vmlinux.
>
> When both CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS are enabled, we can
> reduce the number of link steps.
>
> The build step will look as follows:
>
>     KSYMS   .tmp_vmlinux0.kallsyms.S
>     AS      .tmp_vmlinux0.kallsyms.o
>     LD      .tmp_vmlinux1                # temp vmlinux for BTF and kallsyms step 1
>     BTF     .tmp_vmlinux1.btf.o
>     NM      .tmp_vmlinux1.syms
>     KSYMS   .tmp_vmlinux1.kallsyms.S
>     AS      .tmp_vmlinux1.kallsyms.o
>     LD      .tmp_vmlinux2                # temp vmlinux for kallsyms step 2
>     NM      .tmp_vmlinux2.syms
>     KSYMS   .tmp_vmlinux2.kallsyms.S
>     AS      .tmp_vmlinux2.kallsyms.o
>     LD      vmlinux                      # final link
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>

LGTM, thanks!

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  scripts/link-vmlinux.sh | 45 +++++++++++++++++++++--------------------
>  1 file changed, 23 insertions(+), 22 deletions(-)
>

[...]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-05-30 21:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-22 11:47 [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Masahiro Yamada
2024-05-22 11:47 ` [PATCH 1/3] kbuild: refactor variables in scripts/link-vmlinux.sh Masahiro Yamada
2024-05-22 11:47 ` [PATCH 2/3] kbuild: remove PROVIDE() for kallsyms symbols Masahiro Yamada
2024-05-23  9:31   ` Ard Biesheuvel
2024-05-25 16:43     ` Masahiro Yamada
2024-05-22 11:47 ` [PATCH 3/3] kbuild: merge temp vmlinux for CONFIG_DEBUG_INFO_BTF and CONFIG_KALLSYMS Masahiro Yamada
2024-05-30 21:17   ` Andrii Nakryiko
2024-05-28  9:42 ` [PATCH 0/3] kbuild: remove PROVIDE() and refactor vmlinux_link steps Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox