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

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

[Quentin Perret]

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

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

[Quentin Perret]

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

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

[Nicolas Pitre]

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

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

[Quentin Perret]

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

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

[Masahiro Yamada]

Hi.



On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <qperret@google.com> 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>
> ---
> @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

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

[Quentin Perret]

On Saturday 08 Feb 2020 at 06:05:02 (+0100), Masahiro Yamada wrote:
> On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <qperret@google.com> 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

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

[Quentin Perret]

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

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

[Masahiro Yamada]

On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <qperret@google.com> 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>
> ---
>  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

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

[Quentin Perret]

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

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

[Quentin Perret]

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

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

[Masahiro Yamada]

On Fri, Feb 7, 2020 at 7:08 PM Quentin Perret <qperret@google.com> 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>
> ---
>  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

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

[Quentin Perret]

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

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39984 bytes --]

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

[Quentin Perret]

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 <lkp@intel.com>
> 
> 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

Re: [kbuild-all] Re: [PATCH v3 3/3] kbuild: generate autoksyms.h early

[Quentin Perret]

On Thursday 13 Feb 2020 at 16:42:51 (+0800), kernel test robot wrote:
> I attached the build log for your reference.

Thanks!

<snip>
> scripts/kconfig/conf  --syncconfig Kconfig
>   SYSTBL  arch/x86/include/generated/asm/syscalls_32.h
>   SYSHDR  arch/x86/include/generated/uapi/asm/unistd_32.h
>   UPD     include/config/kernel.release
>   SYSHDR  arch/x86/include/generated/uapi/asm/unistd_64.h
>   SYSHDR  arch/x86/include/generated/uapi/asm/unistd_x32.h
>   UPD     include/generated/utsrelease.h
>   HOSTCC  scripts/dtc/dtc.o
>   HOSTCC  scripts/dtc/flattree.o
> ./scripts/gen_autoksyms.sh: 24: ./scripts/gen_autoksyms.sh: [[: not found
> ./scripts/gen_autoksyms.sh: 35: ./scripts/gen_autoksyms.sh: [[: not found
> cat: /home/nfs/linux//dev/null: No such file or directory

OK, so that's the issue, which Nicolas pointed out earlier. That should
be fixed in v4.

Thanks,
Quentin