| Message ID | 20241014141345.5680-7-david.hunter.linux@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | streamline_config.pl: fix: process configs set to "y" | expand |
On Mon, 14 Oct 2024 10:13:36 -0400 David Hunter <david.hunter.linux@gmail.com> wrote: > An assumption made in this script is that the config options do not need > to be processed because they will simply be in the new config file. This > assumption is incorrect. > > Process the config entries set to "y" because those config entries might > have dependencies set to "m". If a config entry is set to "m" and is not > loaded directly into the machine, the script will currently turn off > that config entry; however, if that turned off config entry is a > dependency for a "y" option. that means the config entry set to "y" > will also be turned off later when the conf executive file is called. > > Here is a model of the problem (arrows show dependency): > > Original config file > Config_1 (m) <-- Config_2 (y) > > Config_1 is not loaded in this example, so it is turned off. > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > Config_1 (n) <-- Config_2 (y) > > After scripts/kconfig/conf > Config_1 (n) <-- Config_2 (n) Hmm, can you show the example of this. My worry here is that there are a lot of cases that have: Config_1 (m) <-- Config_2 (y) Where Config_2 is just some binary option to modify the module set by Config_1. If we keep Config_1 because there's some 'y' that selects it, we are going to keep a lot of unused modules around. > > It should also be noted that any module in the dependency chain will > also be turned off, even if that module is loaded directly onto the > computer. Here is an example: > > Original config file > Config_1 (m) <-- Config_2 (y) <-- Config_3 (m) If Config_3 is used, then it should add Config_2 as a dependency. I guess that's what this patch does. > > Config_3 will be loaded in this example. > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > Config_1 (n) <-- Config_2 (y) <-- Config_3 (m) > > After scripts/kconfig/conf > Config_1 (n) <-- Config_2 (n) <-- Config_3 (n) > > Signed-off-by: David Hunter <david.hunter.linux@gmail.com> > --- > V1 https://lore.kernel.org/all/20240913171205.22126-8-david.hunter.linux@gmail.com/ > > V2 > - change subject > - changed part of the code that only processed config options > set to "m" so that config options set to "y" are processed. I > forgot to put this in v1 when making the series patch. > - removed an unneccessary condition that would have skipped a > dependency loop. > --- > scripts/kconfig/streamline_config.pl | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl > index 4149c4b344db..c3b434220c9f 100755 > --- a/scripts/kconfig/streamline_config.pl > +++ b/scripts/kconfig/streamline_config.pl > @@ -473,6 +473,11 @@ foreach my $line (@config_file) { > > if (/(CONFIG_[$valid]*)=(m|y)/) { > $orig_configs{$1} = $2; > + # all configs options set to 'y' need to be processed > + if ($2 eq "y") { > + $configs{$1}= $2; > + } > + > } > } > > @@ -499,8 +504,8 @@ sub parse_config_depends > > $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 "y") { > + # Make sure that this config exists in the current .config file > + if (!defined($orig_configs{$conf})) { > next; > } > > @@ -600,12 +605,6 @@ sub loop_depend { > > 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; > - } > - OK, and I guess you tests show this doesn't have any other issues. I'll check it out. Thanks, -- Steve > $config =~ s/^CONFIG_//; > $depconfig = $config; >
On Wed, Oct 16, 2024 at 8:19 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 14 Oct 2024 10:13:36 -0400 > David Hunter <david.hunter.linux@gmail.com> wrote: > > > An assumption made in this script is that the config options do not need > > to be processed because they will simply be in the new config file. This > > assumption is incorrect. > > > > Process the config entries set to "y" because those config entries might > > have dependencies set to "m". If a config entry is set to "m" and is not > > loaded directly into the machine, the script will currently turn off > > that config entry; however, if that turned off config entry is a > > dependency for a "y" option. that means the config entry set to "y" > > will also be turned off later when the conf executive file is called. > > > > Here is a model of the problem (arrows show dependency): > > > > Original config file > > Config_1 (m) <-- Config_2 (y) > > > > Config_1 is not loaded in this example, so it is turned off. > > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > > Config_1 (n) <-- Config_2 (y) > > > > After scripts/kconfig/conf > > Config_1 (n) <-- Config_2 (n) > > Hmm, can you show the example of this. My worry here is that there are a > lot of cases that have: > > Config_1 (m) <-- Config_2 (y) > > Where Config_2 is just some binary option to modify the module set by > Config_1. If we keep Config_1 because there's some 'y' that selects it, we > are going to keep a lot of unused modules around. Basically, there is no good way to distinguish in-module option (just CONFIG_BCACHEFS_SIX_OPTIMISTIC_SPIN=y, which is a property for CONFIG_BCACHEFS=m) from real dependency. > > > > It should also be noted that any module in the dependency chain will > > also be turned off, even if that module is loaded directly onto the > > computer. Here is an example: > > > > Original config file > > Config_1 (m) <-- Config_2 (y) <-- Config_3 (m) > > If Config_3 is used, then it should add Config_2 as a dependency. I guess > that's what this patch does. Agree, but I am not sure how to pick up only this case.
diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl index 4149c4b344db..c3b434220c9f 100755 --- a/scripts/kconfig/streamline_config.pl +++ b/scripts/kconfig/streamline_config.pl @@ -473,6 +473,11 @@ foreach my $line (@config_file) { if (/(CONFIG_[$valid]*)=(m|y)/) { $orig_configs{$1} = $2; + # all configs options set to 'y' need to be processed + if ($2 eq "y") { + $configs{$1}= $2; + } + } } @@ -499,8 +504,8 @@ sub parse_config_depends $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 "y") { + # Make sure that this config exists in the current .config file + if (!defined($orig_configs{$conf})) { next; } @@ -600,12 +605,6 @@ sub loop_depend { 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_//; $depconfig = $config;
An assumption made in this script is that the config options do not need to be processed because they will simply be in the new config file. This assumption is incorrect. Process the config entries set to "y" because those config entries might have dependencies set to "m". If a config entry is set to "m" and is not loaded directly into the machine, the script will currently turn off that config entry; however, if that turned off config entry is a dependency for a "y" option. that means the config entry set to "y" will also be turned off later when the conf executive file is called. Here is a model of the problem (arrows show dependency): Original config file Config_1 (m) <-- Config_2 (y) Config_1 is not loaded in this example, so it is turned off. After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf Config_1 (n) <-- Config_2 (y) After scripts/kconfig/conf Config_1 (n) <-- Config_2 (n) It should also be noted that any module in the dependency chain will also be turned off, even if that module is loaded directly onto the computer. Here is an example: Original config file Config_1 (m) <-- Config_2 (y) <-- Config_3 (m) Config_3 will be loaded in this example. After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf Config_1 (n) <-- Config_2 (y) <-- Config_3 (m) After scripts/kconfig/conf Config_1 (n) <-- Config_2 (n) <-- Config_3 (n) Signed-off-by: David Hunter <david.hunter.linux@gmail.com> --- V1 https://lore.kernel.org/all/20240913171205.22126-8-david.hunter.linux@gmail.com/ V2 - change subject - changed part of the code that only processed config options set to "m" so that config options set to "y" are processed. I forgot to put this in v1 when making the series patch. - removed an unneccessary condition that would have skipped a dependency loop. --- scripts/kconfig/streamline_config.pl | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)