[PATCH 0/4] localmodconfig: Improve the number of modules removed

[PATCH 0/4] localmodconfig: Improve the number of modules removed

[Steven Rostedt]

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

make localmodconfig was used to make a minimal config but still make
sure that everything you enabled stays enabled. Thus, it took a
conservative approach when it came to disabling configs. If there was a
chance that disabling a config would cause a required module not to be
built, it would keep that config set.

There was a time it was discovered that a config without a prompt
may be required to build a module, but it was only set if another
config option was set, because that config option would select it.
If the second config option was a tristate, but did not directly
enable a module, localmodconfig would deselect it and cause the required
module to be built.

The simple and conservative way to handle this was to also enable all
configs that select a required module config. But this caused those
configs to include the configs that they depend on, as well as all
the configs that select it. This increased the number of modules being
kept much higher than what the minimal set that was needed.

One issue was that it would even include configs that were core and not
even a module.

Another issue is that there was no reason to enable all configs when just
one select would do.

The second and third patches fix this (the other two are clean ups
and added debugging facility). The first of the fix reads the original
.config file and only processes configs that are modules, and ignores
configs that are set as core (localmodconfig will not touch them anyway).

The second of the fixes was a bit of code change that would process
the direct modules first (one to one relationship with configs to their
modules in the Makefiles). Then it will include all the configs that
those modules depend on. After that set is complete, it kept track
of the configs without prompts (needing to enable the selects), it
would look at those configs requiring selects, check to see if one
of their selects is already selected by a set config (do nothing in
that case), and if not, then pick only one config that selects it and
set that.

With these changes, the number of modules my test case had went from
leaving 356 CONFIG_*=m set, to just 67 of them set.

A much smaller and quicker build.

Thanks to John David Yost (AlleyTrotter) for testing his config and
build as well.

If you want this code, I've already sent it to my for-next branch.

-- Steve

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-kconfig.git
for-next

Head SHA1: 4503379cb811809470ebefb58c943fe605bc3e29


Steven Rostedt (4):
      localmodconfig: Comments and cleanup for streamline_config.pl
      localmodconfig: Read in orig config file to avoid extra processing
      localmodconfig: Check if configs are already set for selects
      localmodconfig: Add debug environment variable LOCALMODCONFIG_DEBUG

----
 scripts/kconfig/streamline_config.pl |  175 ++++++++++++++++++++++++++++++----
 1 file changed, 158 insertions(+), 17 deletions(-)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/4] localmodconfig: Comments and cleanup for streamline_config.pl

[Steven Rostedt]

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

From: Steven Rostedt <srostedt@redhat.com>

Added some more comments and cleaned up part of the the code to use
a named variable instead of one of the special $1 perl variables.

No functional changes.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/kconfig/streamline_config.pl |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index bccf07dd..5c1ce87 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -186,6 +186,7 @@ sub read_kconfig {
 	    $state = "NEW";
 	    $config = $2;
 
+	    # Add depends for 'if' nesting
 	    for (my $i = 0; $i < $iflevel; $i++) {
 		if ($i) {
 		    $depends{$config} .= " " . $ifdeps[$i];
@@ -204,10 +205,11 @@ sub read_kconfig {
 
 	# Get the configs that select this config
 	} elsif ($state ne "NONE" && /^\s*select\s+(\S+)/) {
-	    if (defined($selects{$1})) {
-		$selects{$1} .= " " . $config;
+	    my $conf = $1;
+	    if (defined($selects{$conf})) {
+		$selects{$conf} .= " " . $config;
 	    } else {
-		$selects{$1} = $config;
+		$selects{$conf} = $config;
 	    }
 
 	# configs without prompts must be selected
@@ -250,6 +252,7 @@ if ($kconfig) {
     read_kconfig($kconfig);
 }
 
+# Makefiles can use variables to define their dependencies
 sub convert_vars {
     my ($line, %vars) = @_;
 
@@ -293,6 +296,7 @@ foreach my $makefile (@makefiles) {
 
 	my $objs;
 
+	# Convert variables in a line (could define configs)
 	$_ = convert_vars($_, %make_vars);
 
 	# collect objects after obj-$(CONFIG_FOO_BAR)
@@ -373,7 +377,8 @@ while (<LIN>) {
 close (LIN);
 
 # add to the configs hash all configs that are needed to enable
-# a loaded module.
+# a loaded module. This is a direct obj-${CONFIG_FOO} += bar.o
+# where we know we need bar.o so we add FOO to the list.
 my %configs;
 foreach my $module (keys(%modules)) {
     if (defined($objects{$module})) {
-- 
1.7.10



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/4] localmodconfig: Read in orig config file to avoid extra processing

[Steven Rostedt]

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

From: Steven Rostedt <srostedt@redhat.com>

Read in the entire config file. If there's a config that we depend on
that happens to be in the core set (not a module) then we do not need
to process it as a module.

Currently, we follow the entire depend and selects even if they
are enabled as core and not modules. By checking to make sure that we
only look at modules we can drop the count a little.

From one of my tests, localmodconfig went from taking 3095 set modules
down to 356 before this patch, and down to 290 modules after the change.

Tested-by: John David Yost <johnyost@ptd.net> # AlleyTrotter
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/kconfig/streamline_config.pl |   26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 5c1ce87..ab4985f 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -113,6 +113,10 @@ sub find_config {
 
 find_config;
 
+# Read in the entire config file into config_file
+my @config_file = <CIN>;
+close CIN;
+
 # Parse options
 my $localmodconfig = 0;
 my $localyesconfig = 0;
@@ -392,7 +396,20 @@ foreach my $module (keys(%modules)) {
     }
 }
 
+# Read the current config, and see what is enabled. We want to
+# ignore configs that we would not enable anyway.
+
+my %orig_configs;
 my $valid = "A-Za-z_0-9";
+
+foreach my $line (@config_file) {
+    $_ = $line;
+
+    if (/(CONFIG_[$valid]*)=(m|y)/) {
+	$orig_configs{$1} = $2;
+    }
+}
+
 my $repeat = 1;
 
 #
@@ -414,6 +431,11 @@ sub parse_config_dep_select
 
 	    $p =~ s/^[^$valid]*[$valid]+//;
 
+	    # We only need to process if the depend config is a module
+	    if (!defined($orig_configs{$conf}) || !$orig_configs{conf} eq "m") {
+		next;
+	    }
+
 	    if (!defined($configs{$conf})) {
 		# We must make sure that this config has its
 		# dependencies met.
@@ -450,7 +472,8 @@ my %setconfigs;
 
 # Finally, read the .config file and turn off any module enabled that
 # we could not find a reason to keep enabled.
-while(<CIN>) {
+foreach my $line (@config_file) {
+    $_ = $line;
 
     if (/CONFIG_IKCONFIG/) {
 	if (/# CONFIG_IKCONFIG is not set/) {
@@ -478,7 +501,6 @@ while(<CIN>) {
     }
     print;
 }
-close(CIN);
 
 # Integrity check, make sure all modules that we want enabled do
 # indeed have their configs set.
-- 
1.7.10



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/4] localmodconfig: Check if configs are already set for selects

[Steven Rostedt]

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

From: Steven Rostedt <srostedt@redhat.com>

There are some cases that a required module does not have a prompt
and needs to have another module enabled that selects it to be set.
As localmodconfig is conservative and tries to make the minimum config
without breaking the user's kernel, or keeping the user from using
devices that were loaded when the lsmod was done, all modules that
select this module will also be enabled.

If you needed module A, but module A did not have a prompt but needed
module B to be selected, localmodconfig would make sure B was still
enabled. If not only B selected A, but C, D, E, F, and G also
selected A, then all of those would also be included, as well as the
modules they depend on. This ballooned the number of configs that
localmodconfig would keep.

The fix here is to process the depends first, and then record those
configs that did not have a prompt and needed to be selected.
After the depends are done, check what configs are needed to select
the configs in the list, and if a config that selects it is already
set, then we don't need to do anything else.

If no config that selects the config is set, then just pick one and
try again.

This change brought down the number of selected modules from 290
to 67! Both before and after were run against a config that had 3095
modules enabled.

Tested-by: John David Yost <johnyost@ptd.net> # AlleyTrotter
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/kconfig/streamline_config.pl |  115 ++++++++++++++++++++++++++++++----
 1 file changed, 104 insertions(+), 11 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index ab4985f..fcfcb30 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -420,7 +420,7 @@ my $repeat = 1;
 # to keep on. If A was on in the original config, B would not have been
 # and B would not be turned on by this script.
 #
-sub parse_config_dep_select
+sub parse_config_depends
 {
     my ($p) = @_;
 
@@ -448,26 +448,119 @@ sub parse_config_dep_select
     }
 }
 
-while ($repeat) {
-    $repeat = 0;
+# Select is treated a bit differently than depends. We call this
+# when a config has no prompt and requires another config to be
+# selected. We use to just select all configs that selected this
+# config, but found that that can balloon into enabling hundreds
+# of configs that we do not care about.
+#
+# The idea is we look at all the configs that select it. If one
+# is already in our list of configs to enable, then there's nothing
+# else to do. If there isn't, we pick the first config that was
+# enabled in the orignal config and use that.
+sub parse_config_selects
+{
+    my ($config, $p) = @_;
 
-    foreach my $config (keys %configs) {
-	$config =~ s/^CONFIG_//;
+    my $next_config;
+
+    while ($p =~ /[$valid]/) {
+
+	if ($p =~ /^[^$valid]*([$valid]+)/) {
+	    my $conf = "CONFIG_" . $1;
+
+	    $p =~ s/^[^$valid]*[$valid]+//;
 
-	if (defined($depends{$config})) {
-	    # This config has dependencies. Make sure they are also included
-	    parse_config_dep_select $depends{$config};
+	    # Make sure that this config exists in the current .config file
+	    if (!defined($orig_configs{$conf})) {
+		next;
+	    }
+
+	    # Check if something other than a module selects this config
+	    if (defined($orig_configs{$conf}) && $orig_configs{$conf} ne "m") {
+		# we are good with this
+		return;
+	    }
+	    if (defined($configs{$conf})) {
+		# A set config selects this config, we are good
+		return;
+	    }
+	    # Set this config to be selected
+	    if (!defined($next_config)) {
+		$next_config = $conf;
+	    }
+	} else {
+	    die "this should never happen";
 	}
+    }
 
-	if (defined($prompts{$config}) || !defined($selects{$config})) {
-	    next;
+    # If no possible config selected this, then something happened.
+    if (!defined($next_config)) {
+	print STDERR "WARNING: $config is required, but nothing in the\n";
+	print STDERR "  current config selects it.\n";
+	return;
+    }
+
+    # If we are here, then we found no config that is set and
+    # selects this config. Repeat.
+    $repeat = 1;
+    # Make this config need to be selected
+    $configs{$next_config} = 1;
+}
+
+my %process_selects;
+
+# loop through all configs, select their dependencies.
+sub loop_depend {
+    $repeat = 1;
+
+    while ($repeat) {
+	$repeat = 0;
+
+      forloop:
+	foreach my $config (keys %configs) {
+
+	    # If this config is not a module, we do not need to process it
+	    if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m") {
+		next forloop;
+	    }
+
+	    $config =~ s/^CONFIG_//;
+
+	    if (defined($depends{$config})) {
+		# This config has dependencies. Make sure they are also included
+		parse_config_depends $depends{$config};
+	    }
+
+	    # If the config has no prompt, then we need to check if a config
+	    # that is enabled selected it. Or if we need to enable one.
+	    if (!defined($prompts{$config}) && defined($selects{$config})) {
+		$process_selects{$config} = 1;
+	    }
 	}
+    }
+}
+
+sub loop_select {
+
+    foreach my $config (keys %process_selects) {
+	$config =~ s/^CONFIG_//;
 
 	# config has no prompt and must be selected.
-	parse_config_dep_select $selects{$config};
+	parse_config_selects $config, $selects{$config};
     }
 }
 
+while ($repeat) {
+    # Get the first set of configs and their dependencies.
+    loop_depend;
+
+    $repeat = 0;
+
+    # Now we need to see if we have to check selects;
+    loop_select;
+}	    
+
 my %setconfigs;
 
 # Finally, read the .config file and turn off any module enabled that
-- 
1.7.10



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 4/4] localmodconfig: Add debug environment variable LOCALMODCONFIG_DEBUG

[Steven Rostedt]

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

From: Steven Rostedt <srostedt@redhat.com>

If the environment variable LOCALMODCONFIG_DEBUG is set, then debug output
will appear in the make localmodconfig. This will simplify debugging what
people get with their output, as I can just tell people to do:

  LOCALMODCONFIG_DEBUG=1 make localmodconfig 2>out.txt

and have them send me the out.txt. I'll be able to see why things are not
working as they think it should be.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/kconfig/streamline_config.pl |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index fcfcb30..2fbbbc1 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -45,6 +45,16 @@
 use strict;
 use Getopt::Long;
 
+# set the environment variable LOCALMODCONFIG_DEBUG to get
+# debug output.
+my $debugprint = 0;
+$debugprint = 1 if (defined($ENV{LOCALMODCONFIG_DEBUG}));
+
+sub dprint {
+    return if (!$debugprint);
+    print STDERR @_;
+}
+
 my $config = ".config";
 
 my $uname = `uname -r`;
@@ -389,6 +399,7 @@ foreach my $module (keys(%modules)) {
 	my @arr = @{$objects{$module}};
 	foreach my $conf (@arr) {
 	    $configs{$conf} = $module;
+	    dprint "$conf added by direct ($module)\n";
 	}
     } else {
 	# Most likely, someone has a custom (binary?) module loaded.
@@ -412,6 +423,8 @@ foreach my $line (@config_file) {
 
 my $repeat = 1;
 
+my $depconfig;
+
 #
 # Note, we do not care about operands (like: &&, ||, !) we want to add any
 # config that is in the depend list of another config. This script does
@@ -440,6 +453,7 @@ sub parse_config_depends
 		# We must make sure that this config has its
 		# dependencies met.
 		$repeat = 1; # do again
+		dprint "$conf selected by depend $depconfig\n";
 		$configs{$conf} = 1;
 	    }
 	} else {
@@ -473,15 +487,18 @@ sub parse_config_selects
 
 	    # Make sure that this config exists in the current .config file
 	    if (!defined($orig_configs{$conf})) {
+		dprint "$conf not set for $config select\n";
 		next;
 	    }
 
 	    # Check if something other than a module selects this config
 	    if (defined($orig_configs{$conf}) && $orig_configs{$conf} ne "m") {
+		dprint "$conf (non module) selects config, we are good\n";
 		# we are good with this
 		return;
 	    }
 	    if (defined($configs{$conf})) {
+		dprint "$conf selects $config so we are good\n";
 		# A set config selects this config, we are good
 		return;
 	    }
@@ -506,6 +523,7 @@ sub parse_config_selects
     $repeat = 1;
     # Make this config need to be selected
     $configs{$next_config} = 1;
+    dprint "$next_config selected by select $config\n";
 }
 
 my %process_selects;
@@ -526,6 +544,7 @@ sub loop_depend {
 	    }
 
 	    $config =~ s/^CONFIG_//;
+	    $depconfig = $config;
 
 	    if (defined($depends{$config})) {
 		# This config has dependencies. Make sure they are also included
@@ -546,6 +565,8 @@ sub loop_select {
     foreach my $config (keys %process_selects) {
 	$config =~ s/^CONFIG_//;
 
+	dprint "Process select $config\n";
+
 	# config has no prompt and must be selected.
 	parse_config_selects $config, $selects{$config};
     }
-- 
1.7.10



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/4] localmodconfig: Comments and cleanup for streamline_config.pl

[Steven Rostedt]

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

From: Steven Rostedt <srostedt@redhat.com>

Added some more comments and cleaned up part of the the code to use
a named variable instead of one of the special $1 perl variables.

No functional changes.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/kconfig/streamline_config.pl |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index bccf07dd..5c1ce87 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -186,6 +186,7 @@ sub read_kconfig {
 	    $state = "NEW";
 	    $config = $2;
 
+	    # Add depends for 'if' nesting
 	    for (my $i = 0; $i < $iflevel; $i++) {
 		if ($i) {
 		    $depends{$config} .= " " . $ifdeps[$i];
@@ -204,10 +205,11 @@ sub read_kconfig {
 
 	# Get the configs that select this config
 	} elsif ($state ne "NONE" && /^\s*select\s+(\S+)/) {
-	    if (defined($selects{$1})) {
-		$selects{$1} .= " " . $config;
+	    my $conf = $1;
+	    if (defined($selects{$conf})) {
+		$selects{$conf} .= " " . $config;
 	    } else {
-		$selects{$1} = $config;
+		$selects{$conf} = $config;
 	    }
 
 	# configs without prompts must be selected
@@ -250,6 +252,7 @@ if ($kconfig) {
     read_kconfig($kconfig);
 }
 
+# Makefiles can use variables to define their dependencies
 sub convert_vars {
     my ($line, %vars) = @_;
 
@@ -293,6 +296,7 @@ foreach my $makefile (@makefiles) {
 
 	my $objs;
 
+	# Convert variables in a line (could define configs)
 	$_ = convert_vars($_, %make_vars);
 
 	# collect objects after obj-$(CONFIG_FOO_BAR)
@@ -373,7 +377,8 @@ while (<LIN>) {
 close (LIN);
 
 # add to the configs hash all configs that are needed to enable
-# a loaded module.
+# a loaded module. This is a direct obj-${CONFIG_FOO} += bar.o
+# where we know we need bar.o so we add FOO to the list.
 my %configs;
 foreach my $module (keys(%modules)) {
     if (defined($objects{$module})) {
-- 
1.7.10.4



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]