linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
@ 2020-01-29 18:15 Quentin Perret
  2020-01-29 18:29 ` Nicolas Pitre
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Quentin Perret @ 2020-01-29 18:15 UTC (permalink / raw)
  To: masahiroy, nico
  Cc: linux-kernel, linux-kbuild, maennich, kernel-team, qperret, jeyu

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.

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.

Signed-off-by: Quentin Perret <qperret@google.com>

---
v2: make sure to quote the whitelist path properly (Nicolas)
---
 init/Kconfig                | 12 ++++++++++++
 scripts/adjust_autoksyms.sh |  1 +
 2 files changed, 13 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index a34064a031a5..d9c977ef7de5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2180,6 +2180,18 @@ 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 trimmed 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.
+
 endif # MODULES
 
 config MODULES_TREE_LOOKUP
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index a904bf1f5e67..8e1b7f70e800 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
 EOT
 sed 's/ko$/mod/' modules.order |
 xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
+cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
 sort -u |
 sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
 
-- 
2.25.0.341.g760bfbb309-goog


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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-01-29 18:15 [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
@ 2020-01-29 18:29 ` Nicolas Pitre
  2020-01-31 13:15 ` Matthias Maennich
  2020-02-06 15:56 ` Jessica Yu
  2 siblings, 0 replies; 9+ messages in thread
From: Nicolas Pitre @ 2020-01-29 18:29 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, linux-kernel, linux-kbuild, maennich, kernel-team, jeyu

On Wed, 29 Jan 2020, 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.
> 
> 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.
> 
> Signed-off-by: Quentin Perret <qperret@google.com>

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


> ---
> v2: make sure to quote the whitelist path properly (Nicolas)
> ---
>  init/Kconfig                | 12 ++++++++++++
>  scripts/adjust_autoksyms.sh |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index a34064a031a5..d9c977ef7de5 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2180,6 +2180,18 @@ 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 trimmed 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.
> +
>  endif # MODULES
>  
>  config MODULES_TREE_LOOKUP
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index a904bf1f5e67..8e1b7f70e800 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
>  EOT
>  sed 's/ko$/mod/' modules.order |
>  xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> +cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
>  sort -u |
>  sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
>  
> -- 
> 2.25.0.341.g760bfbb309-goog
> 
> 

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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-01-29 18:15 [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
  2020-01-29 18:29 ` Nicolas Pitre
@ 2020-01-31 13:15 ` Matthias Maennich
  2020-01-31 17:40   ` Quentin Perret
  2020-02-06 15:56 ` Jessica Yu
  2 siblings, 1 reply; 9+ messages in thread
From: Matthias Maennich @ 2020-01-31 13:15 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu

On Wed, Jan 29, 2020 at 06:15:41PM +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.
>
>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.

Thank you for working on this! I like the idea!

>
>Signed-off-by: Quentin Perret <qperret@google.com>
>
>---
>v2: make sure to quote the whitelist path properly (Nicolas)
>---
> init/Kconfig                | 12 ++++++++++++
> scripts/adjust_autoksyms.sh |  1 +
> 2 files changed, 13 insertions(+)
>
>diff --git a/init/Kconfig b/init/Kconfig
>index a34064a031a5..d9c977ef7de5 100644
>--- a/init/Kconfig
>+++ b/init/Kconfig
>@@ -2180,6 +2180,18 @@ 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 trimmed 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.
>+
> endif # MODULES
>
> config MODULES_TREE_LOOKUP
>diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>index a904bf1f5e67..8e1b7f70e800 100755
>--- a/scripts/adjust_autoksyms.sh
>+++ b/scripts/adjust_autoksyms.sh
>@@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
> EOT
> sed 's/ko$/mod/' modules.order |
> xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
>+cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |

This handles absolute paths very well. I wonder whether we can make this
more useful for folks that want to maintain such a whitelist in their
copy of the tree. Lets say, I have in my sources
arch/x86/configs/x86_64_symbol_whitelist and in my config I have
CONFIG_UNUSED_KSYMS_WHITELIST="arch/x86/configs/x86_64_symbol_whitelist".

If I see it correctly, UNUSED_KSYMS_WHITELIST is currently either an
absolute path or a relative path to the current build directory. I would
prefer if relative paths would be relative to the source directory to
support the above use case. (Note, that scenario above works if I build
directly in the sources, but fails if I build O=/somewhere/else.)

Cheers,
Matthias

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

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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-01-31 13:15 ` Matthias Maennich
@ 2020-01-31 17:40   ` Quentin Perret
  2020-02-04 14:44     ` Matthias Maennich
  0 siblings, 1 reply; 9+ messages in thread
From: Quentin Perret @ 2020-01-31 17:40 UTC (permalink / raw)
  To: Matthias Maennich
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu

On Friday 31 Jan 2020 at 13:15:08 (+0000), 'Matthias Maennich' via kernel-team wrote:
> On Wed, Jan 29, 2020 at 06:15:41PM +0000, Quentin Perret wrote:
> > diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> > index a904bf1f5e67..8e1b7f70e800 100755
> > --- a/scripts/adjust_autoksyms.sh
> > +++ b/scripts/adjust_autoksyms.sh
> > @@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
> > EOT
> > sed 's/ko$/mod/' modules.order |
> > xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> > +cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
> 
> This handles absolute paths very well. I wonder whether we can make this
> more useful for folks that want to maintain such a whitelist in their
> copy of the tree. Lets say, I have in my sources
> arch/x86/configs/x86_64_symbol_whitelist and in my config I have
> CONFIG_UNUSED_KSYMS_WHITELIST="arch/x86/configs/x86_64_symbol_whitelist".
> 
> If I see it correctly, UNUSED_KSYMS_WHITELIST is currently either an
> absolute path or a relative path to the current build directory. I would
> prefer if relative paths would be relative to the source directory to
> support the above use case. (Note, that scenario above works if I build
> directly in the sources, but fails if I build O=/somewhere/else.)

Right, that is an interesting use case. I suppose something like the
below should work (with appropriate documentation of the config option).

---8<---
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 8e1b7f70e800..d37803fd75ce 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -38,6 +38,12 @@ esac
 # We need access to CONFIG_ symbols
 . include/config/auto.conf
 
+ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
+# If the path is relative, it must be relative to the source tree
+if [ "$ksym_wl" == "${ksym_wl#/}" ]; then
+       ksym_wl="$abs_srctree/$ksym_wl"
+fi
+
 # Generate a new ksym list file with symbols needed by the current
 # set of modules.
 cat > "$new_ksyms_file" << EOT
@@ -48,7 +54,7 @@ cat > "$new_ksyms_file" << EOT
 EOT
 sed 's/ko$/mod/' modules.order |
 xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
-cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
+cat - "$ksym_wl" |
 sort -u |
 sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
--->8---

Thoughts ?

Thanks,
Quentin

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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-01-31 17:40   ` Quentin Perret
@ 2020-02-04 14:44     ` Matthias Maennich
  2020-02-05 11:53       ` Quentin Perret
  0 siblings, 1 reply; 9+ messages in thread
From: Matthias Maennich @ 2020-02-04 14:44 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu

On Fri, Jan 31, 2020 at 05:40:55PM +0000, Quentin Perret wrote:
>On Friday 31 Jan 2020 at 13:15:08 (+0000), 'Matthias Maennich' via kernel-team wrote:
>> On Wed, Jan 29, 2020 at 06:15:41PM +0000, Quentin Perret wrote:
>> > diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>> > index a904bf1f5e67..8e1b7f70e800 100755
>> > --- a/scripts/adjust_autoksyms.sh
>> > +++ b/scripts/adjust_autoksyms.sh
>> > @@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
>> > EOT
>> > sed 's/ko$/mod/' modules.order |
>> > xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
>> > +cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
>>
>> This handles absolute paths very well. I wonder whether we can make this
>> more useful for folks that want to maintain such a whitelist in their
>> copy of the tree. Lets say, I have in my sources
>> arch/x86/configs/x86_64_symbol_whitelist and in my config I have
>> CONFIG_UNUSED_KSYMS_WHITELIST="arch/x86/configs/x86_64_symbol_whitelist".
>>
>> If I see it correctly, UNUSED_KSYMS_WHITELIST is currently either an
>> absolute path or a relative path to the current build directory. I would
>> prefer if relative paths would be relative to the source directory to
>> support the above use case. (Note, that scenario above works if I build
>> directly in the sources, but fails if I build O=/somewhere/else.)
>
>Right, that is an interesting use case. I suppose something like the
>below should work (with appropriate documentation of the config option).
>
>---8<---
>diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>index 8e1b7f70e800..d37803fd75ce 100755
>--- a/scripts/adjust_autoksyms.sh
>+++ b/scripts/adjust_autoksyms.sh
>@@ -38,6 +38,12 @@ esac
> # We need access to CONFIG_ symbols
> . include/config/auto.conf
>
>+ksym_wl="${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}"
>+# If the path is relative, it must be relative to the source tree
>+if [ "$ksym_wl" == "${ksym_wl#/}" ]; then
>+       ksym_wl="$abs_srctree/$ksym_wl"
>+fi
>+
> # Generate a new ksym list file with symbols needed by the current
> # set of modules.
> cat > "$new_ksyms_file" << EOT
>@@ -48,7 +54,7 @@ cat > "$new_ksyms_file" << EOT
> EOT
> sed 's/ko$/mod/' modules.order |
> xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
>-cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
>+cat - "$ksym_wl" |
> sort -u |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
>--->8---
>
>Thoughts ?

That definitely looks like I would expect that config option to work.
Thanks for looking into that!

Cheers,
Matthias

>
>Thanks,
>Quentin

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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-04 14:44     ` Matthias Maennich
@ 2020-02-05 11:53       ` Quentin Perret
  0 siblings, 0 replies; 9+ messages in thread
From: Quentin Perret @ 2020-02-05 11:53 UTC (permalink / raw)
  To: Matthias Maennich
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, kernel-team, jeyu

On Tuesday 04 Feb 2020 at 14:44:15 (+0000), 'Matthias Maennich' via kernel-team wrote:
> That definitely looks like I would expect that config option to work.

Good, thank you, I'll send an update.

And while at it, I realized we could actually pre-populate autoksyms.h
very early on with the content of the whitelist instead of having it
empty. That way, there's a whole lot of things that will be compiled
'correctly' from the start (that is, adjust_autoksyms.sh won't need to
re-compile them later on). And in fact, if all symbols used by in-tree
modules happen to already be on the whitelist, you'll build a trimmed
kernel in a single pass, which should improve build time a bit I hope.

I'll add something to v3 in addition to your suggestion.

Thanks,
Quentin

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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-01-29 18:15 [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
  2020-01-29 18:29 ` Nicolas Pitre
  2020-01-31 13:15 ` Matthias Maennich
@ 2020-02-06 15:56 ` Jessica Yu
  2020-02-06 16:12   ` Nicolas Pitre
  2020-02-06 16:17   ` Quentin Perret
  2 siblings, 2 replies; 9+ messages in thread
From: Jessica Yu @ 2020-02-06 15:56 UTC (permalink / raw)
  To: Quentin Perret
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, maennich, kernel-team

+++ Quentin Perret [29/01/20 18:15 +0000]:
>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.
>
>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.
>
>Signed-off-by: Quentin Perret <qperret@google.com>
>
>---
>v2: make sure to quote the whitelist path properly (Nicolas)
>---
> init/Kconfig                | 12 ++++++++++++
> scripts/adjust_autoksyms.sh |  1 +
> 2 files changed, 13 insertions(+)
>
>diff --git a/init/Kconfig b/init/Kconfig
>index a34064a031a5..d9c977ef7de5 100644
>--- a/init/Kconfig
>+++ b/init/Kconfig
>@@ -2180,6 +2180,18 @@ 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 trimmed from the
>+	  build when TRIM_UNUSED_KSYMS is selected.

Hm, I thought TRIM_UNUSED_KSYMS just *unexports* unused symbols, no?
"Trimmed from the build" sounds like the symbols are not compiled in
or dropped completely. Please correct me if I misunderstood.

>+	  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.
>+
> endif # MODULES
>
> config MODULES_TREE_LOOKUP
>diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>index a904bf1f5e67..8e1b7f70e800 100755
>--- a/scripts/adjust_autoksyms.sh
>+++ b/scripts/adjust_autoksyms.sh
>@@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
> EOT
> sed 's/ko$/mod/' modules.order |
> xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
>+cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
> sort -u |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"

In general, I agree with the motivation behind this patch, even though
we try not to provide too much support for out-of-tree modules.
However, in this particular case, I think it's fair to provide some
mechanism to keep some exported symbols around that we know will have
users, despite having no in-tree users for a particular
configuration/build. For example, livepatch exports symbols that have
no in-tree users (except for the sample livepatch module, but you'd
have to enable SAMPLES), and all livepatch users will always be out of
tree. 

I also agree with Matthias' feedback, so assuming that gets
incorporated into v3:

Acked-by: Jessica Yu <jeyu@kernel.org>

Thanks!


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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-06 15:56 ` Jessica Yu
@ 2020-02-06 16:12   ` Nicolas Pitre
  2020-02-06 16:17   ` Quentin Perret
  1 sibling, 0 replies; 9+ messages in thread
From: Nicolas Pitre @ 2020-02-06 16:12 UTC (permalink / raw)
  To: Jessica Yu
  Cc: Quentin Perret, masahiroy, linux-kernel, linux-kbuild, maennich,
	kernel-team

On Thu, 6 Feb 2020, Jessica Yu wrote:

> Hm, I thought TRIM_UNUSED_KSYMS just *unexports* unused symbols, no?
> "Trimmed from the build" sounds like the symbols are not compiled in
> or dropped completely. Please correct me if I misunderstood.

If they are unexposed, then it doesn't make much sense to keep them 
around wasting space. So yes, the compiler is free to optimize away the 
unused code at that point.

Please see the first part of the following article where effective 
kernel size reduction is is achieved with this feature:

https://lwn.net/Articles/746780/


Nicolas

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

* Re: [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS
  2020-02-06 15:56 ` Jessica Yu
  2020-02-06 16:12   ` Nicolas Pitre
@ 2020-02-06 16:17   ` Quentin Perret
  1 sibling, 0 replies; 9+ messages in thread
From: Quentin Perret @ 2020-02-06 16:17 UTC (permalink / raw)
  To: Jessica Yu
  Cc: masahiroy, nico, linux-kernel, linux-kbuild, maennich, kernel-team

Hi Jessica,

On Thursday 06 Feb 2020 at 16:56:51 (+0100), Jessica Yu wrote:
> +++ Quentin Perret [29/01/20 18:15 +0000]:
> > +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 trimmed from the
> > +	  build when TRIM_UNUSED_KSYMS is selected.
> 
> Hm, I thought TRIM_UNUSED_KSYMS just *unexports* unused symbols, no?

Right.

> "Trimmed from the build" sounds like the symbols are not compiled in
> or dropped completely. Please correct me if I misunderstood.

Good point. I tried to re-use the wording from the help text of
TRIM_UNUSED_KSYMS ("This option allows for unused exported symbols
to be dropped from the build"), but I agree this is a bit confusing.

I'll re-write the help text in v3.

> 
> > +	  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.
> > +
> > endif # MODULES
> > 
> > config MODULES_TREE_LOOKUP
> > diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> > index a904bf1f5e67..8e1b7f70e800 100755
> > --- a/scripts/adjust_autoksyms.sh
> > +++ b/scripts/adjust_autoksyms.sh
> > @@ -48,6 +48,7 @@ cat > "$new_ksyms_file" << EOT
> > EOT
> > sed 's/ko$/mod/' modules.order |
> > xargs -n1 sed -n -e '2{s/ /\n/g;/^$/!p;}' -- |
> > +cat - "${CONFIG_UNUSED_KSYMS_WHITELIST:-/dev/null}" |
> > sort -u |
> > sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
> 
> In general, I agree with the motivation behind this patch, even though
> we try not to provide too much support for out-of-tree modules.
> However, in this particular case, I think it's fair to provide some
> mechanism to keep some exported symbols around that we know will have
> users, despite having no in-tree users for a particular
> configuration/build. For example, livepatch exports symbols that have
> no in-tree users (except for the sample livepatch module, but you'd
> have to enable SAMPLES), and all livepatch users will always be out of
> tree.
> 
> I also agree with Matthias' feedback, so assuming that gets
> incorporated into v3:
> 
> Acked-by: Jessica Yu <jeyu@kernel.org>

Thanks for the review!
Quentin

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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 18:15 [PATCH v2] kbuild: allow symbol whitelisting with TRIM_UNUSED_KSYMS Quentin Perret
2020-01-29 18:29 ` Nicolas Pitre
2020-01-31 13:15 ` Matthias Maennich
2020-01-31 17:40   ` Quentin Perret
2020-02-04 14:44     ` Matthias Maennich
2020-02-05 11:53       ` Quentin Perret
2020-02-06 15:56 ` Jessica Yu
2020-02-06 16:12   ` Nicolas Pitre
2020-02-06 16:17   ` 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).