2020-02-07 18:09:15

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v3 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM

The current norm on Android and many other systems is for vendors to
introduce significant changes to their downstream kernels, and to
contribute very little (if any) code back upstream. The Generic Kernel
Image (GKI) project in Android attempts to improve the status-quo by
having a unique kernel for all android devices of the same architecture,
regardless of the SoC vendor. The key idea is to make all interested
parties agree on a common solution, and contribute their code upstream
to make it available to use by the wider community.

The kernel-to-drivers ABI on Android devices varies significantly from
one vendor kernel to another today because of changes to exported
symbols, dependencies on vendor symbols, and surely other things. The
first step for GKI is to try and put some order into this by agreeing on
one version of the ABI that works for everybody.

For practical reasons, we need to reduce the ABI surface to a subset of
the exported symbols, simply to make the problem realistically solvable,
but there is currently no upstream support for this use-case.

As such, this series attempts to improve the situation by enabling users
to specify a symbol 'whitelist' at compile time. Any symbol specified in
this whitelist will be kept exported when CONFIG_TRIM_UNUSED_KSYMS is
set, even if it has no in-tree user. The whitelist is defined as a
simple text file, listing symbols, one per line.

v2 -> v3:
- added a cover letter to explain why this is in fact an attempt to
help usptream in the long term (Christoph)
- made path relative to the kernel source tree (Matthias)
- made the Kconfig help text less confusing (Jessica)
- added patch 02 and 03 to optimize build time when a whitelist is
provided

v2:
- make sure to quote the whitelist path properly (Nicolas)

Quentin Perret (3):
kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
kbuild: split adjust_autoksyms.sh in two parts
kbuild: generate autoksyms.h early

Makefile | 2 +-
init/Kconfig | 13 +++++++++++
scripts/adjust_autoksyms.sh | 27 ++++------------------
scripts/gen_autoksyms.sh | 45 +++++++++++++++++++++++++++++++++++++
4 files changed, 63 insertions(+), 24 deletions(-)
create mode 100755 scripts/gen_autoksyms.sh

--
2.25.0.341.g760bfbb309-goog


2020-02-07 18:09:17

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v3 2/3] kbuild: split adjust_autoksyms.sh in two parts

In order to prepare the ground for a build-time optimization, split
adjust_autoksyms.sh into two scripts: one that generates autoksyms.h
based on all currently available information (whitelist, and .mod
files), and the other to inspect the diff between two versions of
autoksyms.h and trigger appropriate rebuilds.

Signed-off-by: Quentin Perret <[email protected]>
---
scripts/adjust_autoksyms.sh | 32 ++++-----------------------
scripts/gen_autoksyms.sh | 44 +++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 28 deletions(-)
create mode 100755 scripts/gen_autoksyms.sh

diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 58335eee4b38..ae1e65e9009c 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -1,14 +1,13 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0-only

-# Script to create/update include/generated/autoksyms.h and dependency files
+# Script to update include/generated/autoksyms.h and dependency files
#
# Copyright: (C) 2016 Linaro Limited
# Created by: Nicolas Pitre, January 2016
#

-# Create/update the include/generated/autoksyms.h file from the list
-# of all module's needed symbols as recorded on the second line of *.mod files.
+# Update the include/generated/autoksyms.h file.
#
# For each symbol being added or removed, the corresponding dependency
# file's timestamp is updated to force a rebuild of the affected source
@@ -35,31 +34,8 @@ case "$KBUILD_VERBOSE" in
;;
esac

-# We need access to CONFIG_ symbols
-. include/config/auto.conf
-
-# The symbol whitelist, relative to the source tree
-eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
-[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"
-
-# Generate a new ksym list file with symbols needed by the current
-# set of modules.
-cat > "$new_ksyms_file" << EOT
-/*
- * Automatically generated file; DO NOT EDIT.
- */
-
-EOT
-sed 's/ko$/mod/' modules.order |
-xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
-cat - "$ksym_wl" |
-sort -u |
-sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
-
-# Special case for modversions (see modpost.c)
-if [ -n "$CONFIG_MODVERSIONS" ]; then
- echo "#define __KSYM_module_layout 1" >> "$new_ksyms_file"
-fi
+# Generate a new symbol list file
+$srctree/scripts/gen_autoksyms.sh "$new_ksyms_file"

# Extract changes between old and new list and touch corresponding
# dependency files.
diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
new file mode 100755
index 000000000000..ce0919c3791a
--- /dev/null
+++ b/scripts/gen_autoksyms.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-only
+
+# Create an autoksyms.h header file from the list of all module's needed symbols
+# as recorded on the second line of *.mod files and the user-provided symbol
+# whitelist.
+
+set -e
+
+output_file="$1"
+
+# Use "make V=1" to debug this script.
+case "$KBUILD_VERBOSE" in
+*1*)
+ set -x
+ ;;
+esac
+
+# We need access to CONFIG_ symbols
+. include/config/auto.conf
+
+# The symbol whitelist, relative to the source tree
+eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
+[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"
+
+# Generate a new ksym list file with symbols needed by the current
+# set of modules.
+cat > "$output_file" << EOT
+/*
+ * Automatically generated file; DO NOT EDIT.
+ */
+
+EOT
+
+sed 's/ko$/mod/' modules.order |
+xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
+cat - "$ksym_wl" |
+sort -u |
+sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
+
+# Special case for modversions (see modpost.c)
+if [ -n "$CONFIG_MODVERSIONS" ]; then
+ echo "#define __KSYM_module_layout 1" >> "$output_file"
+fi
--
2.25.0.341.g760bfbb309-goog

2020-02-07 18:09:30

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v3 3/3] kbuild: generate autoksyms.h early

When doing a cold build, autoksyms.h starts empty, and is updated late
in the build process to have visibility over the symbols used by in-tree
drivers. But since the symbol whitelist is known upfront, it can be used
to pre-populate autoksyms.h and maximize the amount of code that can be
compiled to its final state in a single pass, hence reducing build time.

Do this by using gen_autoksyms.sh to initialize autoksyms.h instead of
creating an empty file.

Signed-off-by: Quentin Perret <[email protected]>
---
Makefile | 2 +-
scripts/gen_autoksyms.sh | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 6a01b073915e..e5c389d189f7 100644
--- a/Makefile
+++ b/Makefile
@@ -1065,7 +1065,7 @@ autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)

$(autoksyms_h):
$(Q)mkdir -p $(dir $@)
- $(Q)touch $@
+ $(Q)$(CONFIG_SHELL) $(srctree)/scripts/gen_autoksyms.sh $@

ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)

diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
index ce0919c3791a..ae033ab03a4a 100755
--- a/scripts/gen_autoksyms.sh
+++ b/scripts/gen_autoksyms.sh
@@ -32,7 +32,8 @@ cat > "$output_file" << EOT

EOT

-sed 's/ko$/mod/' modules.order |
+[[ -f modules.order ]] && modlist=modules.order || modlist=/dev/null
+sed 's/ko$/mod/' $modlist |
xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
cat - "$ksym_wl" |
sort -u |
--
2.25.0.341.g760bfbb309-goog

2020-02-07 18:11:12

by Quentin Perret

[permalink] [raw]
Subject: [PATCH v3 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS

CONFIG_TRIM_UNUSED_KSYMS currently removes all unused exported symbols
from ksymtab. This works really well when using in-tree drivers, but
cannot be used in its current form if some of them are out-of-tree.

Indeed, even if the list of symbols required by out-of-tree drivers is
known at compile time, the only solution today to guarantee these don't
get trimmed is to set CONFIG_TRIM_UNUSED_KSYMS=n. This not only wastes
space, but also makes it difficult to control the ABI usable by vendor
modules in distribution kernels such as Android. Being able to control
the kernel ABI surface is particularly useful to ship a unique Generic
Kernel Image (GKI) for all vendors, which is a first step in the
direction of getting all vendors to contribute their code upstream.

As such, attempt to improve the situation by enabling users to specify a
symbol 'whitelist' at compile time. Any symbol specified in this
whitelist will be kept exported when CONFIG_TRIM_UNUSED_KSYMS is set,
even if it has no in-tree user. The whitelist is defined as a simple
text file, listing symbols, one per line.

Acked-by: Jessica Yu <[email protected]>
Signed-off-by: Quentin Perret <[email protected]>
---
@Nicolas: I left your Reviewed-by behind as the code has changed a bit
but let me know what you think
---
init/Kconfig | 13 +++++++++++++
scripts/adjust_autoksyms.sh | 5 +++++
2 files changed, 18 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index a34064a031a5..79fd976ce031 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2180,6 +2180,19 @@ config TRIM_UNUSED_KSYMS

If unsure, or if you need to build out-of-tree modules, say N.

+config UNUSED_KSYMS_WHITELIST
+ string "Whitelist of symbols to keep in ksymtab"
+ depends on TRIM_UNUSED_KSYMS
+ help
+ By default, all unused exported symbols will be un-exported from the
+ build when TRIM_UNUSED_KSYMS is selected.
+
+ UNUSED_KSYMS_WHITELIST allows to whitelist symbols that must be kept
+ exported at all times, even in absence of in-tree users. The value to
+ set here is the path to a text file containing the list of symbols,
+ one per line. The path can be absolute, or relative to the kernel
+ source tree.
+
endif # MODULES

config MODULES_TREE_LOOKUP
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index a904bf1f5e67..58335eee4b38 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -38,6 +38,10 @@ esac
# We need access to CONFIG_ symbols
. include/config/auto.conf

+# The symbol whitelist, relative to the source tree
+eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
+[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"
+
# Generate a new ksym list file with symbols needed by the current
# set of modules.
cat > "$new_ksyms_file" << EOT
@@ -48,6 +52,7 @@ cat > "$new_ksyms_file" << EOT
EOT
sed 's/ko$/mod/' modules.order |
xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
+cat - "$ksym_wl" |
sort -u |
sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"

--
2.25.0.341.g760bfbb309-goog

2020-02-07 18:23:23

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS

On Fri, 7 Feb 2020, Quentin Perret wrote:

> @Nicolas: I left your Reviewed-by behind as the code has changed a bit
> but let me know what you think
> ---
> init/Kconfig | 13 +++++++++++++
> scripts/adjust_autoksyms.sh | 5 +++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index a34064a031a5..79fd976ce031 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2180,6 +2180,19 @@ config TRIM_UNUSED_KSYMS
>
> If unsure, or if you need to build out-of-tree modules, say N.
>
> +config UNUSED_KSYMS_WHITELIST
> + string "Whitelist of symbols to keep in ksymtab"
> + depends on TRIM_UNUSED_KSYMS
> + help
> + By default, all unused exported symbols will be un-exported from the
> + build when TRIM_UNUSED_KSYMS is selected.
> +
> + UNUSED_KSYMS_WHITELIST allows to whitelist symbols that must be kept
> + exported at all times, even in absence of in-tree users. The value to
> + set here is the path to a text file containing the list of symbols,
> + one per line. The path can be absolute, or relative to the kernel
> + source tree.
> +
> endif # MODULES
>
> config MODULES_TREE_LOOKUP
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index a904bf1f5e67..58335eee4b38 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -38,6 +38,10 @@ esac
> # We need access to CONFIG_ symbols
> . include/config/auto.conf
>
> +# The symbol whitelist, relative to the source tree
> +eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
> +[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"

This "[[ ]]" is a bashism. I think there was an effort not to depend on
bash for the build system. So either this needs to be changed to basic
bourne shell, or the interpretor has to be /bin/bash not /bin/sh.


Nicolas

2020-02-08 05:08:08

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS

Hi.



On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <[email protected]> wrote:
>
> CONFIG_TRIM_UNUSED_KSYMS currently removes all unused exported symbols
> from ksymtab. This works really well when using in-tree drivers, but
> cannot be used in its current form if some of them are out-of-tree.
>
> Indeed, even if the list of symbols required by out-of-tree drivers is
> known at compile time, the only solution today to guarantee these don't
> get trimmed is to set CONFIG_TRIM_UNUSED_KSYMS=n. This not only wastes
> space, but also makes it difficult to control the ABI usable by vendor
> modules in distribution kernels such as Android. Being able to control
> the kernel ABI surface is particularly useful to ship a unique Generic
> Kernel Image (GKI) for all vendors, which is a first step in the
> direction of getting all vendors to contribute their code upstream.
>
> As such, attempt to improve the situation by enabling users to specify a
> symbol 'whitelist' at compile time. Any symbol specified in this
> whitelist will be kept exported when CONFIG_TRIM_UNUSED_KSYMS is set,
> even if it has no in-tree user. The whitelist is defined as a simple
> text file, listing symbols, one per line.
>
> Acked-by: Jessica Yu <[email protected]>
> Signed-off-by: Quentin Perret <[email protected]>
> ---
> @Nicolas: I left your Reviewed-by behind as the code has changed a bit
> but let me know what you think
> ---
> init/Kconfig | 13 +++++++++++++
> scripts/adjust_autoksyms.sh | 5 +++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index a34064a031a5..79fd976ce031 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2180,6 +2180,19 @@ config TRIM_UNUSED_KSYMS
>
> If unsure, or if you need to build out-of-tree modules, say N.
>
> +config UNUSED_KSYMS_WHITELIST
> + string "Whitelist of symbols to keep in ksymtab"
> + depends on TRIM_UNUSED_KSYMS
> + help
> + By default, all unused exported symbols will be un-exported from the
> + build when TRIM_UNUSED_KSYMS is selected.
> +
> + UNUSED_KSYMS_WHITELIST allows to whitelist symbols that must be kept
> + exported at all times, even in absence of in-tree users. The value to
> + set here is the path to a text file containing the list of symbols,
> + one per line. The path can be absolute, or relative to the kernel
> + source tree.
> +
> endif # MODULES
>
> config MODULES_TREE_LOOKUP
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index a904bf1f5e67..58335eee4b38 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -38,6 +38,10 @@ esac
> # We need access to CONFIG_ symbols
> . include/config/auto.conf
>
> +# The symbol whitelist, relative to the source tree
> +eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"

What is this 'eval' needed for?

This worked for me without it.





> +[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"
> +
> # Generate a new ksym list file with symbols needed by the current
> # set of modules.
> cat > "$new_ksyms_file" << EOT
> @@ -48,6 +52,7 @@ cat > "$new_ksyms_file" << EOT
> EOT
> sed 's/ko$/mod/' modules.order |
> xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> +cat - "$ksym_wl" |
> sort -u |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
>
> --
> 2.25.0.341.g760bfbb309-goog
>


--
Best Regards
Masahiro Yamada

2020-02-08 05:10:05

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] kbuild: split adjust_autoksyms.sh in two parts

On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <[email protected]> wrote:
>
> In order to prepare the ground for a build-time optimization, split
> adjust_autoksyms.sh into two scripts: one that generates autoksyms.h
> based on all currently available information (whitelist, and .mod
> files), and the other to inspect the diff between two versions of
> autoksyms.h and trigger appropriate rebuilds.
>
> Signed-off-by: Quentin Perret <[email protected]>
> ---
> scripts/adjust_autoksyms.sh | 32 ++++-----------------------
> scripts/gen_autoksyms.sh | 44 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 28 deletions(-)
> create mode 100755 scripts/gen_autoksyms.sh
>
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index 58335eee4b38..ae1e65e9009c 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -1,14 +1,13 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0-only
>
> -# Script to create/update include/generated/autoksyms.h and dependency files
> +# Script to update include/generated/autoksyms.h and dependency files
> #
> # Copyright: (C) 2016 Linaro Limited
> # Created by: Nicolas Pitre, January 2016
> #
>
> -# Create/update the include/generated/autoksyms.h file from the list
> -# of all module's needed symbols as recorded on the second line of *.mod files.
> +# Update the include/generated/autoksyms.h file.
> #
> # For each symbol being added or removed, the corresponding dependency
> # file's timestamp is updated to force a rebuild of the affected source
> @@ -35,31 +34,8 @@ case "$KBUILD_VERBOSE" in
> ;;
> esac
>
> -# We need access to CONFIG_ symbols
> -. include/config/auto.conf
> -
> -# The symbol whitelist, relative to the source tree
> -eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
> -[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"
> -
> -# Generate a new ksym list file with symbols needed by the current
> -# set of modules.
> -cat > "$new_ksyms_file" << EOT
> -/*
> - * Automatically generated file; DO NOT EDIT.
> - */
> -
> -EOT
> -sed 's/ko$/mod/' modules.order |
> -xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> -cat - "$ksym_wl" |
> -sort -u |
> -sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
> -
> -# Special case for modversions (see modpost.c)
> -if [ -n "$CONFIG_MODVERSIONS" ]; then
> - echo "#define __KSYM_module_layout 1" >> "$new_ksyms_file"
> -fi
> +# Generate a new symbol list file
> +$srctree/scripts/gen_autoksyms.sh "$new_ksyms_file"

In 3/3, you will call this script with $(CONFIG_SHELL) from Makefile.

For consistency,

$CONFIG_SHELL $srctree/scripts/gen_autoksyms.sh "$new_ksyms_file"

is better.


Thanks.




>
> # Extract changes between old and new list and touch corresponding
> # dependency files.
> diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> new file mode 100755
> index 000000000000..ce0919c3791a
> --- /dev/null
> +++ b/scripts/gen_autoksyms.sh
> @@ -0,0 +1,44 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +# Create an autoksyms.h header file from the list of all module's needed symbols
> +# as recorded on the second line of *.mod files and the user-provided symbol
> +# whitelist.
> +
> +set -e
> +
> +output_file="$1"
> +
> +# Use "make V=1" to debug this script.
> +case "$KBUILD_VERBOSE" in
> +*1*)
> + set -x
> + ;;
> +esac
> +
> +# We need access to CONFIG_ symbols
> +. include/config/auto.conf
> +
> +# The symbol whitelist, relative to the source tree
> +eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
> +[[ "$ksym_wl" =~ ^/ ]] || ksym_wl="$abs_srctree/$ksym_wl"
> +
> +# Generate a new ksym list file with symbols needed by the current
> +# set of modules.
> +cat > "$output_file" << EOT
> +/*
> + * Automatically generated file; DO NOT EDIT.
> + */
> +
> +EOT
> +
> +sed 's/ko$/mod/' modules.order |
> +xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> +cat - "$ksym_wl" |
> +sort -u |
> +sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
> +
> +# Special case for modversions (see modpost.c)
> +if [ -n "$CONFIG_MODVERSIONS" ]; then
> + echo "#define __KSYM_module_layout 1" >> "$output_file"
> +fi
> --
> 2.25.0.341.g760bfbb309-goog
>


--
Best Regards
Masahiro Yamada

2020-02-08 05:11:02

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] kbuild: generate autoksyms.h early

On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <[email protected]> wrote:
>
> When doing a cold build, autoksyms.h starts empty, and is updated late
> in the build process to have visibility over the symbols used by in-tree
> drivers. But since the symbol whitelist is known upfront, it can be used
> to pre-populate autoksyms.h and maximize the amount of code that can be
> compiled to its final state in a single pass, hence reducing build time.
>
> Do this by using gen_autoksyms.sh to initialize autoksyms.h instead of
> creating an empty file.
>
> Signed-off-by: Quentin Perret <[email protected]>
> ---
> Makefile | 2 +-
> scripts/gen_autoksyms.sh | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 6a01b073915e..e5c389d189f7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1065,7 +1065,7 @@ autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
>
> $(autoksyms_h):
> $(Q)mkdir -p $(dir $@)
> - $(Q)touch $@
> + $(Q)$(CONFIG_SHELL) $(srctree)/scripts/gen_autoksyms.sh $@


Now that this line is doing a non-trivial task,
it might be a good idea to show a short log, like this:

GEN include/generated/autoksyms.h



You can do it like this:


quiet_cmd_autoksyms_h = GEN $@
cmd_autoksyms_h = mkdir -p $(dir $@); $(BASH)
$(srctree)/scripts/gen_autoksyms.sh $@

$(autoksyms_h):
$(call cmd,autoksyms_h)





>
> ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>
> diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> index ce0919c3791a..ae033ab03a4a 100755
> --- a/scripts/gen_autoksyms.sh
> +++ b/scripts/gen_autoksyms.sh
> @@ -32,7 +32,8 @@ cat > "$output_file" << EOT
>
> EOT
>
> -sed 's/ko$/mod/' modules.order |
> +[[ -f modules.order ]] && modlist=modules.order || modlist=/dev/null
> +sed 's/ko$/mod/' $modlist |
> xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> cat - "$ksym_wl" |
> sort -u |
> --
> 2.25.0.341.g760bfbb309-goog
>


--
Best Regards

Masahiro Yamada

2020-02-11 02:27:16

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] kbuild: generate autoksyms.h early

Hi Quentin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kbuild/for-next]
[also build test ERROR on linux/master linus/master v5.6-rc1 next-20200210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Quentin-Perret/kbuild-allow-symbol-whitelisting-with-TRIM_UNUSED_KSYM/20200211-020659
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
config: i386-randconfig-c002-20200211 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-4) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

>> ERROR: "trace_event_raw_init" [lib/objagg.ko] undefined!
>> ERROR: "trace_event_reg" [lib/objagg.ko] undefined!
>> ERROR: "ida_destroy" [lib/objagg.ko] undefined!
>> ERROR: "rhashtable_destroy" [lib/objagg.ko] undefined!
>> ERROR: "kmalloc_order_trace" [lib/objagg.ko] undefined!
>> ERROR: "sort" [lib/objagg.ko] undefined!
>> ERROR: "__raw_spin_lock_init" [lib/objagg.ko] undefined!
>> ERROR: "rhashtable_init" [lib/objagg.ko] undefined!
>> ERROR: "rht_bucket_nested" [lib/objagg.ko] undefined!
>> ERROR: "__list_del_entry_valid" [lib/objagg.ko] undefined!
>> ERROR: "__rht_bucket_nested" [lib/objagg.ko] undefined!
>> ERROR: "kmem_cache_alloc_trace" [lib/objagg.ko] undefined!
>> ERROR: "kmalloc_caches" [lib/objagg.ko] undefined!
>> ERROR: "queue_work_on" [lib/objagg.ko] undefined!
>> ERROR: "system_wq" [lib/objagg.ko] undefined!
>> ERROR: "kfree" [lib/objagg.ko] undefined!
>> ERROR: "__list_add_valid" [lib/objagg.ko] undefined!
>> ERROR: "lockdep_rht_bucket_is_held" [lib/objagg.ko] undefined!
>> ERROR: "rhashtable_insert_slow" [lib/objagg.ko] undefined!
>> ERROR: "__local_bh_enable_ip" [lib/objagg.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.21 kB)
.config.gz (39.05 kB)
Download all attachments

2020-02-11 06:05:41

by Quentin Perret

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS

On Friday 07 Feb 2020 at 13:22:12 (-0500), Nicolas Pitre wrote:
> This "[[ ]]" is a bashism. I think there was an effort not to depend on
> bash for the build system.

OK, I see.

> So either this needs to be changed to basic
> bourne shell, or the interpretor has to be /bin/bash not /bin/sh.

So, as per the above, the basic bourne shell option sounds preferable,
I'll go fix this for v4.

Thanks,
Quentin

2020-02-11 06:05:53

by Quentin Perret

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS

On Saturday 08 Feb 2020 at 06:05:02 (+0100), Masahiro Yamada wrote:
> On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <[email protected]> wrote:
> > diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> > index a904bf1f5e67..58335eee4b38 100755
> > --- a/scripts/adjust_autoksyms.sh
> > +++ b/scripts/adjust_autoksyms.sh
> > @@ -38,6 +38,10 @@ esac
> > # We need access to CONFIG_ symbols
> > . include/config/auto.conf
> >
> > +# The symbol whitelist, relative to the source tree
> > +eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
>
> What is this 'eval' needed for?
>
> This worked for me without it.

Right, it is there to expand the path in cases where the user sets the
option to "~/my_whitelist" for instance. That could most certainly use a
comment, though.

Thanks,
Quentin

2020-02-11 06:07:46

by Quentin Perret

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] kbuild: split adjust_autoksyms.sh in two parts

On Saturday 08 Feb 2020 at 06:08:08 (+0100), Masahiro Yamada wrote:
> In 3/3, you will call this script with $(CONFIG_SHELL) from Makefile.
>
> For consistency,
>
> $CONFIG_SHELL $srctree/scripts/gen_autoksyms.sh "$new_ksyms_file"
>
> is better.

Agreed, I'll fix this in v4.

Thanks,
Quentin

2020-02-11 06:21:56

by Quentin Perret

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] kbuild: generate autoksyms.h early

On Saturday 08 Feb 2020 at 06:09:06 (+0100), Masahiro Yamada wrote:
> Now that this line is doing a non-trivial task,
> it might be a good idea to show a short log, like this:
>
> GEN include/generated/autoksyms.h

Sounds good.

> You can do it like this:
>
>
> quiet_cmd_autoksyms_h = GEN $@
> cmd_autoksyms_h = mkdir -p $(dir $@); $(BASH)
> $(srctree)/scripts/gen_autoksyms.sh $@
>
> $(autoksyms_h):
> $(call cmd,autoksyms_h)

Nice, thanks for sharing!
Quentin

2020-02-12 19:57:19

by Quentin Perret

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] kbuild: generate autoksyms.h early

On Tuesday 11 Feb 2020 at 10:14:14 (+0800), kbuild test robot wrote:
> Hi Quentin,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on kbuild/for-next]
> [also build test ERROR on linux/master linus/master v5.6-rc1 next-20200210]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Quentin-Perret/kbuild-allow-symbol-whitelisting-with-TRIM_UNUSED_KSYM/20200211-020659
> base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> config: i386-randconfig-c002-20200211 (attached as .config)
> compiler: gcc-7 (Debian 7.5.0-4) 7.5.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> >> ERROR: "trace_event_raw_init" [lib/objagg.ko] undefined!
> >> ERROR: "trace_event_reg" [lib/objagg.ko] undefined!
> >> ERROR: "ida_destroy" [lib/objagg.ko] undefined!
> >> ERROR: "rhashtable_destroy" [lib/objagg.ko] undefined!
> >> ERROR: "kmalloc_order_trace" [lib/objagg.ko] undefined!
> >> ERROR: "sort" [lib/objagg.ko] undefined!
> >> ERROR: "__raw_spin_lock_init" [lib/objagg.ko] undefined!
> >> ERROR: "rhashtable_init" [lib/objagg.ko] undefined!
> >> ERROR: "rht_bucket_nested" [lib/objagg.ko] undefined!
> >> ERROR: "__list_del_entry_valid" [lib/objagg.ko] undefined!
> >> ERROR: "__rht_bucket_nested" [lib/objagg.ko] undefined!
> >> ERROR: "kmem_cache_alloc_trace" [lib/objagg.ko] undefined!
> >> ERROR: "kmalloc_caches" [lib/objagg.ko] undefined!
> >> ERROR: "queue_work_on" [lib/objagg.ko] undefined!
> >> ERROR: "system_wq" [lib/objagg.ko] undefined!
> >> ERROR: "kfree" [lib/objagg.ko] undefined!
> >> ERROR: "__list_add_valid" [lib/objagg.ko] undefined!
> >> ERROR: "lockdep_rht_bucket_is_held" [lib/objagg.ko] undefined!
> >> ERROR: "rhashtable_insert_slow" [lib/objagg.ko] undefined!
> >> ERROR: "__local_bh_enable_ip" [lib/objagg.ko] undefined!

I find myself unable to reproduce this error on my box. Could you please
provide the full build log ?

In the meantime I'll proceed to send a v4.

Thanks,
Quentin