linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM
@ 2020-02-12 20:21 Quentin Perret
  2020-02-12 20:21 ` [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Quentin Perret @ 2020-02-12 20:21 UTC (permalink / raw)
  To: masahiroy, nico
  Cc: linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch, qperret

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.

v4:
 - removed [[]] bash-specific pattern from the scripts (Nicolas)
 - use $CONFIG_SHELL consistently in all patches (Masahiro)
 - added shortlog for initial generation of autoksyms.h (Masahiro)
 - added comment on how 'eval' expands the whitelist path (Masahiro)

v3:
 - added a cover letter to explain why this is in fact an attempt to
   help upstream 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                    |  7 ++++--
 init/Kconfig                | 13 +++++++++++
 scripts/adjust_autoksyms.sh | 24 ++++----------------
 scripts/gen_autoksyms.sh    | 45 +++++++++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 22 deletions(-)
 create mode 100755 scripts/gen_autoksyms.sh

-- 
2.25.0.225.g125e21ebc7-goog


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

* [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-12 20:21 [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Quentin Perret
@ 2020-02-12 20:21 ` Quentin Perret
  2020-02-17 14:23   ` Quentin Perret
  2020-02-17 15:22   ` Matthias Maennich
  2020-02-12 20:21 ` [PATCH v4 2/3] kbuild: split adjust_autoksyms.sh in two parts Quentin Perret
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Quentin Perret @ 2020-02-12 20:21 UTC (permalink / raw)
  To: masahiroy, nico
  Cc: linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch, qperret

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 <jeyu@kernel.org>
Signed-off-by: Quentin Perret <qperret@google.com>
---
 init/Kconfig                | 13 +++++++++++++
 scripts/adjust_autoksyms.sh |  5 +++++
 2 files changed, 18 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index cfee56c151f1..58b672afceb2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2210,6 +2210,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..93f4d10e66e6 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
 
+# Use 'eval' to expand the whitelist path and check if it is relative
+eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
+[ "${ksym_wl:0:1}" = "/" ] || 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.225.g125e21ebc7-goog


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

* [PATCH v4 2/3] kbuild: split adjust_autoksyms.sh in two parts
  2020-02-12 20:21 [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Quentin Perret
  2020-02-12 20:21 ` [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
@ 2020-02-12 20:21 ` Quentin Perret
  2020-02-17 15:37   ` Matthias Maennich
  2020-02-12 20:21 ` [PATCH v4 3/3] kbuild: generate autoksyms.h early Quentin Perret
  2020-02-12 20:48 ` [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Nicolas Pitre
  3 siblings, 1 reply; 13+ messages in thread
From: Quentin Perret @ 2020-02-12 20:21 UTC (permalink / raw)
  To: masahiroy, nico
  Cc: linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch, qperret

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 <qperret@google.com>
---
 scripts/adjust_autoksyms.sh | 29 ++++--------------------
 scripts/gen_autoksyms.sh    | 44 +++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 25 deletions(-)
 create mode 100755 scripts/gen_autoksyms.sh

diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 93f4d10e66e6..2b366d945ccb 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
@@ -38,28 +37,8 @@ esac
 # We need access to CONFIG_ symbols
 . include/config/auto.conf
 
-# Use 'eval' to expand the whitelist path and check if it is relative
-eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
-[ "${ksym_wl:0:1}" = "/" ] || 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
+$CONFIG_SHELL $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..2cea433616a8
--- /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
+
+# Use 'eval' to expand the whitelist path and check if it is relative
+eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
+[ "${ksym_wl:0:1}" = "/" ] || 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.225.g125e21ebc7-goog


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

* [PATCH v4 3/3] kbuild: generate autoksyms.h early
  2020-02-12 20:21 [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Quentin Perret
  2020-02-12 20:21 ` [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
  2020-02-12 20:21 ` [PATCH v4 2/3] kbuild: split adjust_autoksyms.sh in two parts Quentin Perret
@ 2020-02-12 20:21 ` Quentin Perret
  2020-02-17 16:34   ` Matthias Maennich
  2020-02-12 20:48 ` [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Nicolas Pitre
  3 siblings, 1 reply; 13+ messages in thread
From: Quentin Perret @ 2020-02-12 20:21 UTC (permalink / raw)
  To: masahiroy, nico
  Cc: linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch, qperret

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 <qperret@google.com>
---
 Makefile                 | 7 +++++--
 scripts/gen_autoksyms.sh | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 84b71845c43f..17b7e7f441bd 100644
--- a/Makefile
+++ b/Makefile
@@ -1062,9 +1062,12 @@ endif
 
 autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
 
+quiet_cmd_autoksyms_h = GEN     $@
+      cmd_autoksyms_h = mkdir -p $(dir $@); $(CONFIG_SHELL) \
+			$(srctree)/scripts/gen_autoksyms.sh $@
+
 $(autoksyms_h):
-	$(Q)mkdir -p $(dir $@)
-	$(Q)touch $@
+	$(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 2cea433616a8..f52b93ad122c 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.225.g125e21ebc7-goog


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

* Re: [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM
  2020-02-12 20:21 [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Quentin Perret
                   ` (2 preceding siblings ...)
  2020-02-12 20:21 ` [PATCH v4 3/3] kbuild: generate autoksyms.h early Quentin Perret
@ 2020-02-12 20:48 ` Nicolas Pitre
  2020-02-12 20:53   ` Quentin Perret
  3 siblings, 1 reply; 13+ messages in thread
From: Nicolas Pitre @ 2020-02-12 20:48 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch

On Wed, 12 Feb 2020, Quentin Perret wrote:

> 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.

For the whole series:

Acked-by: Nicolas Pitre <nico@fluxnic.net>


Nicolas

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

* Re: [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM
  2020-02-12 20:48 ` [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Nicolas Pitre
@ 2020-02-12 20:53   ` Quentin Perret
  0 siblings, 0 replies; 13+ messages in thread
From: Quentin Perret @ 2020-02-12 20:53 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: masahiroy, linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch

On Wednesday 12 Feb 2020 at 15:48:50 (-0500), Nicolas Pitre wrote:
> For the whole series:
> 
> Acked-by: Nicolas Pitre <nico@fluxnic.net>

Thanks!
Quentin

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

* Re: [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-12 20:21 ` [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
@ 2020-02-17 14:23   ` Quentin Perret
  2020-02-17 15:22   ` Matthias Maennich
  1 sibling, 0 replies; 13+ messages in thread
From: Quentin Perret @ 2020-02-17 14:23 UTC (permalink / raw)
  To: masahiroy, nico
  Cc: linux-kernel, linux-kbuild, maennich, kernel-team, jeyu, hch

On Wednesday 12 Feb 2020 at 20:21:38 (+0000), Quentin Perret wrote:
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index a904bf1f5e67..93f4d10e66e6 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
>  
> +# Use 'eval' to expand the whitelist path and check if it is relative
> +eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
> +[ "${ksym_wl:0:1}" = "/" ] || ksym_wl="$abs_srctree/$ksym_wl"

Our internal CI just informed me that this is *still* not quite POSIX
compliant ... I believe that the following should (finally) solve this
issue:

  [ "${ksym_wl}" != "${ksym_wl#/}" ] || ksym_wl="$abs_srctree/$ksym_wl"

The above seems to work with bash, zsh, dash, posh and, IIUC, complies
with https://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html.
I'll try other shells and stare at the doc some more, but if there is a
preferred pattern in the kernel I'm happy to change it.

Thanks,
Quentin

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

* Re: [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-12 20:21 ` [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
  2020-02-17 14:23   ` Quentin Perret
@ 2020-02-17 15:22   ` Matthias Maennich
  2020-02-17 15:30     ` Quentin Perret
  1 sibling, 1 reply; 13+ messages in thread
From: Matthias Maennich @ 2020-02-17 15:22 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu, hch

On Wed, Feb 12, 2020 at 08:21:38PM +0000, Quentin Perret 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 <jeyu@kernel.org>
>Signed-off-by: Quentin Perret <qperret@google.com>
>---
> init/Kconfig                | 13 +++++++++++++
> scripts/adjust_autoksyms.sh |  5 +++++
> 2 files changed, 18 insertions(+)
>
>diff --git a/init/Kconfig b/init/Kconfig
>index cfee56c151f1..58b672afceb2 100644
>--- a/init/Kconfig
>+++ b/init/Kconfig
>@@ -2210,6 +2210,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..93f4d10e66e6 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
>
>+# Use 'eval' to expand the whitelist path and check if it is relative
>+eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
>+[ "${ksym_wl:0:1}" = "/" ] || 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" |

In case the whitelist file can't be found, the error message is

  cat: path/to/file: file not found

I wonder whether we can make this error message a bit more specific by
telling the user that the KSYMS_WHITELIST is missing.

With the above addressed (and your amend for the absolute path test),
please feel free to add

Tested-by: Matthias Maennich <maennich@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

> sort -u |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
>
>-- 
>2.25.0.225.g125e21ebc7-goog
>

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

* Re: [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-17 15:22   ` Matthias Maennich
@ 2020-02-17 15:30     ` Quentin Perret
  2020-02-17 16:00       ` Nicolas Pitre
  0 siblings, 1 reply; 13+ messages in thread
From: Quentin Perret @ 2020-02-17 15:30 UTC (permalink / raw)
  To: Matthias Maennich
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu, hch

On Monday 17 Feb 2020 at 15:22:01 (+0000), Matthias Maennich wrote:
> In case the whitelist file can't be found, the error message is
> 
>  cat: path/to/file: file not found
> 
> I wonder whether we can make this error message a bit more specific by
> telling the user that the KSYMS_WHITELIST is missing.

+1, that'd be really useful. I'll check the file existence in v5 (in a
POSIX-compliant way, I promise).

> With the above addressed (and your amend for the absolute path test),
> please feel free to add
> 
> Tested-by: Matthias Maennich <maennich@google.com>
> Reviewed-by: Matthias Maennich <maennich@google.com>

Thanks!
Quentin

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

* Re: [PATCH v4 2/3] kbuild: split adjust_autoksyms.sh in two parts
  2020-02-12 20:21 ` [PATCH v4 2/3] kbuild: split adjust_autoksyms.sh in two parts Quentin Perret
@ 2020-02-17 15:37   ` Matthias Maennich
  0 siblings, 0 replies; 13+ messages in thread
From: Matthias Maennich @ 2020-02-17 15:37 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu, hch

On Wed, Feb 12, 2020 at 08:21:39PM +0000, Quentin Perret 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 <qperret@google.com>

Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> scripts/adjust_autoksyms.sh | 29 ++++--------------------
> scripts/gen_autoksyms.sh    | 44 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 25 deletions(-)
> create mode 100755 scripts/gen_autoksyms.sh
>
>diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>index 93f4d10e66e6..2b366d945ccb 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
>@@ -38,28 +37,8 @@ esac
> # We need access to CONFIG_ symbols
> . include/config/auto.conf
>
>-# Use 'eval' to expand the whitelist path and check if it is relative
>-eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
>-[ "${ksym_wl:0:1}" = "/" ] || 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
>+$CONFIG_SHELL $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..2cea433616a8
>--- /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
>+
>+# Use 'eval' to expand the whitelist path and check if it is relative
>+eval ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
>+[ "${ksym_wl:0:1}" = "/" ] || 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.225.g125e21ebc7-goog
>

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

* Re: [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-17 15:30     ` Quentin Perret
@ 2020-02-17 16:00       ` Nicolas Pitre
  2020-02-17 16:35         ` Matthias Maennich
  0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Pitre @ 2020-02-17 16:00 UTC (permalink / raw)
  To: Quentin Perret
  Cc: Matthias Maennich, masahiroy, linux-kernel, linux-kbuild,
	kernel-team, jeyu, hch

On Mon, 17 Feb 2020, Quentin Perret wrote:

> On Monday 17 Feb 2020 at 15:22:01 (+0000), Matthias Maennich wrote:
> > In case the whitelist file can't be found, the error message is
> > 
> >  cat: path/to/file: file not found
> > 
> > I wonder whether we can make this error message a bit more specific by
> > telling the user that the KSYMS_WHITELIST is missing.
> 
> +1, that'd be really useful. I'll check the file existence in v5 (in a
> POSIX-compliant way, I promise).

In fact, if you explicitly provide a file that is not there, then this 
is arguably a good reason to even fail the build.


Nicolas

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

* Re: [PATCH v4 3/3] kbuild: generate autoksyms.h early
  2020-02-12 20:21 ` [PATCH v4 3/3] kbuild: generate autoksyms.h early Quentin Perret
@ 2020-02-17 16:34   ` Matthias Maennich
  0 siblings, 0 replies; 13+ messages in thread
From: Matthias Maennich @ 2020-02-17 16:34 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu, hch

On Wed, Feb 12, 2020 at 08:21:40PM +0000, Quentin Perret 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 <qperret@google.com>

Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> Makefile                 | 7 +++++--
> scripts/gen_autoksyms.sh | 3 ++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/Makefile b/Makefile
>index 84b71845c43f..17b7e7f441bd 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -1062,9 +1062,12 @@ endif
>
> autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
>
>+quiet_cmd_autoksyms_h = GEN     $@
>+      cmd_autoksyms_h = mkdir -p $(dir $@); $(CONFIG_SHELL) \
>+			$(srctree)/scripts/gen_autoksyms.sh $@
>+
> $(autoksyms_h):
>-	$(Q)mkdir -p $(dir $@)
>-	$(Q)touch $@
>+	$(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 2cea433616a8..f52b93ad122c 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.225.g125e21ebc7-goog
>

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

* Re: [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-17 16:00       ` Nicolas Pitre
@ 2020-02-17 16:35         ` Matthias Maennich
  0 siblings, 0 replies; 13+ messages in thread
From: Matthias Maennich @ 2020-02-17 16:35 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Quentin Perret, masahiroy, linux-kernel, linux-kbuild,
	kernel-team, jeyu, hch

On Mon, Feb 17, 2020 at 11:00:39AM -0500, Nicolas Pitre wrote:
>On Mon, 17 Feb 2020, Quentin Perret wrote:
>
>> On Monday 17 Feb 2020 at 15:22:01 (+0000), Matthias Maennich wrote:
>> > In case the whitelist file can't be found, the error message is
>> >
>> >  cat: path/to/file: file not found
>> >
>> > I wonder whether we can make this error message a bit more specific by
>> > telling the user that the KSYMS_WHITELIST is missing.
>>
>> +1, that'd be really useful. I'll check the file existence in v5 (in a
>> POSIX-compliant way, I promise).
>
>In fact, if you explicitly provide a file that is not there, then this
>is arguably a good reason to even fail the build.

I agree, I would expect the build to fail in that case.

Cheers,
Matthias

>
>
>Nicolas

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

end of thread, other threads:[~2020-02-17 16:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12 20:21 [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Quentin Perret
2020-02-12 20:21 ` [PATCH v4 1/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
2020-02-17 14:23   ` Quentin Perret
2020-02-17 15:22   ` Matthias Maennich
2020-02-17 15:30     ` Quentin Perret
2020-02-17 16:00       ` Nicolas Pitre
2020-02-17 16:35         ` Matthias Maennich
2020-02-12 20:21 ` [PATCH v4 2/3] kbuild: split adjust_autoksyms.sh in two parts Quentin Perret
2020-02-17 15:37   ` Matthias Maennich
2020-02-12 20:21 ` [PATCH v4 3/3] kbuild: generate autoksyms.h early Quentin Perret
2020-02-17 16:34   ` Matthias Maennich
2020-02-12 20:48 ` [PATCH v4 0/3] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYM Nicolas Pitre
2020-02-12 20:53   ` Quentin Perret

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).