* [PATCH v2 0/2] update kselftest framework to check for required configs @ 2024-12-20 19:35 Siddharth Menon 2024-12-20 19:35 ` [PATCH v2 1/2] selftests: Introduce script to validate " Siddharth Menon 2024-12-20 19:35 ` [PATCH v2 2/2] selftests/lib.mk: Introduce check " Siddharth Menon 0 siblings, 2 replies; 5+ messages in thread From: Siddharth Menon @ 2024-12-20 19:35 UTC (permalink / raw) To: simeddon, shuah; +Cc: skhan, pmladek, mbenes, linux-kselftest Currently, kselftests does not have a generalised mechanism to skip compilation and run tests when required kernel configuration options are disabled. This patch series adresses this issue by checking whether all required configs from selftest/<test>/config are enabled in the current kernel Siddharth Menon (2): selftests: Introduce script to validate required configs selftests/lib.mk: Introduce check to validate required configs tools/testing/selftests/lib.mk | 11 ++- tools/testing/selftests/mktest.pl | 138 ++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 2 deletions(-) mode change 100644 => 100755 tools/testing/selftests/lib.mk create mode 100755 tools/testing/selftests/mktest.pl -- 2.39.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] selftests: Introduce script to validate required configs 2024-12-20 19:35 [PATCH v2 0/2] update kselftest framework to check for required configs Siddharth Menon @ 2024-12-20 19:35 ` Siddharth Menon 2025-01-07 13:44 ` Petr Mladek 2024-12-20 19:35 ` [PATCH v2 2/2] selftests/lib.mk: Introduce check " Siddharth Menon 1 sibling, 1 reply; 5+ messages in thread From: Siddharth Menon @ 2024-12-20 19:35 UTC (permalink / raw) To: simeddon, shuah; +Cc: skhan, pmladek, mbenes, linux-kselftest This patch adds a script to validate that the current kernel configuration satisfies the requirements for selftests. The script compares the current kernel configs against the required selftest configs. A config mismatch exits with error value 1 while matching configs or missing config files exit with value 0.In order to get debug output, set the environment variable LOCALMODCONFIG_DEBUG=1. The code for extracting the current kernel configs is adapted from scripts/kconfig/streamline_config.pl. Suggested-by: Petr Mladek <pmladek@suse.com> Suggested-by: Miroslav Benes <mbenes@suse.cz> Signed-off-by: Siddharth Menon <simeddon@gmail.com> --- v1->v2: created this perl script instead of checking directly in lib.mk tools/testing/selftests/mktest.pl | 138 ++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100755 tools/testing/selftests/mktest.pl diff --git a/tools/testing/selftests/mktest.pl b/tools/testing/selftests/mktest.pl new file mode 100755 index 000000000000..60462f323bde --- /dev/null +++ b/tools/testing/selftests/mktest.pl @@ -0,0 +1,138 @@ +#!/usr/bin/env perl +# SPDX-License-Identifier: GPL-2.0 +use warnings; +use strict; +use Getopt::Long; +use File::Spec; + +# 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 $uname = `uname -r`; +chomp $uname; + +my @searchconfigs = ( + { + "file" => ".config", + "exec" => "cat", + }, + { + "file" => "/proc/config.gz", + "exec" => "zcat", + }, + { + "file" => "/boot/config-$uname", + "exec" => "cat", + }, + { + "file" => "/boot/vmlinuz-$uname", + "exec" => "scripts/extract-ikconfig", + "test" => "scripts/extract-ikconfig", + }, + { + "file" => "vmlinux", + "exec" => "scripts/extract-ikconfig", + "test" => "scripts/extract-ikconfig", + }, + { + "file" => "/lib/modules/$uname/kernel/kernel/configs.ko", + "exec" => "scripts/extract-ikconfig", + "test" => "scripts/extract-ikconfig", + }, + { + "file" => "/lib/modules/$uname/build/.config", + "exec" => "cat", + }, + { + "file" => "kernel/configs.ko", + "exec" => "scripts/extract-ikconfig", + "test" => "scripts/extract-ikconfig", + }, + { + "file" => "kernel/configs.o", + "exec" => "scripts/extract-ikconfig", + "test" => "scripts/extract-ikconfig", + }, +); + +sub read_config { + foreach my $conf (@searchconfigs) { + my $file = $conf->{"file"}; + + next if (! -f "$file"); + + if (defined($conf->{"test"})) { + `$conf->{"test"} $conf->{"file"} 2>/dev/null`; + next if ($?); + } + + my $exec = $conf->{"exec"}; + + dprint "Kernel config: '$file'\n"; + + open(my $infile, '-|', "$exec $file") || die "Failed to run $exec $file"; + my @x = <$infile>; + close $infile; + return @x; + } + dprint "Unable to find kernel config file, skipping check\n"; + exit 0; +} + +# Check if selftest path is provided +die "Usage: $0 <selftest_path>\n" unless @ARGV == 1; + +my $file_path = $ARGV[0]; + +my @config_file = read_config(); + +my %kern_configs; +my $valid = "A-Za-z_0-9"; +foreach my $line (@config_file) { + chomp $line; + next if $line =~ /^\s*$/ || $line =~ /^#/; + + if ($line =~ /^(CONFIG_\w+)=(.+)$/) { + $kern_configs{$1} = $2; + } +} + +my %test_configs; +# Continue as normal if /config file does not exist +open(my $fh, '<', $file_path) or exit 0; + +while (my $line = <$fh>) { + chomp $line; + next if $line =~ /^\s*$/ || $line =~ /^#/; + + if ($line =~ /^(CONFIG_\w+)=(.+)$/) { + $test_configs{$1} = $2; + } +} +close $fh; + +# Check if all selftest configs match kernel config +my $all_match = 1; +my @missing_or_mismatched; + +foreach my $key (keys %test_configs) { + if (!exists $kern_configs{$key} || $kern_configs{$key} ne $test_configs{$key}) { + push @missing_or_mismatched, "Required: $key=$test_configs{$key}"; + $all_match = 0; + } +} + + +if ($all_match) { + exit 0; +} else { + dprint "$_\n" for @missing_or_mismatched; + exit 1; +} -- 2.39.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] selftests: Introduce script to validate required configs 2024-12-20 19:35 ` [PATCH v2 1/2] selftests: Introduce script to validate " Siddharth Menon @ 2025-01-07 13:44 ` Petr Mladek 0 siblings, 0 replies; 5+ messages in thread From: Petr Mladek @ 2025-01-07 13:44 UTC (permalink / raw) To: Siddharth Menon; +Cc: shuah, skhan, mbenes, linux-kselftest On Sat 2024-12-21 01:05:35, Siddharth Menon wrote: > This patch adds a script to validate that the current kernel configuration > satisfies the requirements for selftests. The script compares the current > kernel configs against the required selftest configs. > > A config mismatch exits with error value 1 while matching configs or missing > config files exit with value 0.In order to get debug output, set the ^ missing space before the next sentence > environment variable LOCALMODCONFIG_DEBUG=1. I would prefer to print the missing dependencies by default. Otherwise, it is far from clear why the test was skipped. Also LOCALMODCONFIG_DEBUG looks like a pretty non-standard way to get debug output. People will have troubles to discover this way and memorize the long name. > diff --git a/tools/testing/selftests/mktest.pl b/tools/testing/selftests/mktest.pl > new file mode 100755 > index 000000000000..60462f323bde > --- /dev/null > +++ b/tools/testing/selftests/mktest.pl I would give it a more meaningful name. For example, using the same name as for the makefile target just with .pl suffix. I mean "check_config_deps.pl". That said, I have just realized that that there already was "kselftest_deps.sh". It seems to check missing compile dependencies by scanning various variants of LDLIBS variables. It seems that "kselftest_deps.sh" should be called separately. I mean that it is not integrated with the make files. It is a bit strange to handle two types of dependencies differently. It would be nice somehow consolidate the two check scripts so that they behave similar way and use a similar name. I personally like the integration with the Makefile. > @@ -0,0 +1,138 @@ > +#!/usr/bin/env perl > +# SPDX-License-Identifier: GPL-2.0 > +use warnings; > +use strict; > +use Getopt::Long; > +use File::Spec; > + > +# 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 $uname = `uname -r`; > +chomp $uname; > + > +my @searchconfigs = ( > + { > + "file" => ".config", > + "exec" => "cat", Nit: I would prefer to use a tab instead of 8 spaces. At least, the 8 spaces are considered a bad formatting in the kernel .c sources. > + }, > + { > + "file" => "/proc/config.gz", > + "exec" => "zcat", > + }, > + { > + "file" => "/boot/config-$uname", > + "exec" => "cat", > + }, > + { > + "file" => "/boot/vmlinuz-$uname", > + "exec" => "scripts/extract-ikconfig", > + "test" => "scripts/extract-ikconfig", > + }, > + { > + "file" => "vmlinux", > + "exec" => "scripts/extract-ikconfig", > + "test" => "scripts/extract-ikconfig", > + }, > + { > + "file" => "/lib/modules/$uname/kernel/kernel/configs.ko", > + "exec" => "scripts/extract-ikconfig", > + "test" => "scripts/extract-ikconfig", > + }, > + { > + "file" => "/lib/modules/$uname/build/.config", > + "exec" => "cat", > + }, > + { > + "file" => "kernel/configs.ko", > + "exec" => "scripts/extract-ikconfig", > + "test" => "scripts/extract-ikconfig", > + }, > + { > + "file" => "kernel/configs.o", > + "exec" => "scripts/extract-ikconfig", > + "test" => "scripts/extract-ikconfig", > + }, > +); > + > +sub read_config { > + foreach my $conf (@searchconfigs) { > + my $file = $conf->{"file"}; > + > + next if (! -f "$file"); > + Nit: There are 4 spaces on the above empty line. "git am" complains about it. I guess that also ./scripts/checkpatch.pl would complain about it. > + if (defined($conf->{"test"})) { > + `$conf->{"test"} $conf->{"file"} 2>/dev/null`; > + next if ($?); > + } > + same here > + my $exec = $conf->{"exec"}; > + and here > + dprint "Kernel config: '$file'\n"; > + and here. > + open(my $infile, '-|', "$exec $file") || die "Failed to run $exec $file"; > + my @x = <$infile>; > + close $infile; > + return @x; > + } > + dprint "Unable to find kernel config file, skipping check\n"; > + exit 0; > +} > + > +# Check if selftest path is provided > +die "Usage: $0 <selftest_path>\n" unless @ARGV == 1; > + > +my $file_path = $ARGV[0]; > + > +my @config_file = read_config(); > + > +my %kern_configs; > +my $valid = "A-Za-z_0-9"; > +foreach my $line (@config_file) { > + chomp $line; > + next if $line =~ /^\s*$/ || $line =~ /^#/; > + another spaces on empty line > + if ($line =~ /^(CONFIG_\w+)=(.+)$/) { > + $kern_configs{$1} = $2; > + } > +} > + > +my %test_configs; > +# Continue as normal if /config file does not exist trailing space on the above line. > +open(my $fh, '<', $file_path) or exit 0; > + > +while (my $line = <$fh>) { > + chomp $line; > + next if $line =~ /^\s*$/ || $line =~ /^#/; > + spaces on empty line > + if ($line =~ /^(CONFIG_\w+)=(.+)$/) { > + $test_configs{$1} = $2; > + } > +} > +close $fh; > + Best Regards, Petr ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] selftests/lib.mk: Introduce check to validate required configs 2024-12-20 19:35 [PATCH v2 0/2] update kselftest framework to check for required configs Siddharth Menon 2024-12-20 19:35 ` [PATCH v2 1/2] selftests: Introduce script to validate " Siddharth Menon @ 2024-12-20 19:35 ` Siddharth Menon 2025-01-07 13:53 ` Petr Mladek 1 sibling, 1 reply; 5+ messages in thread From: Siddharth Menon @ 2024-12-20 19:35 UTC (permalink / raw) To: simeddon, shuah; +Cc: skhan, pmladek, mbenes, linux-kselftest Currently, kselftests does not have a generalised mechanism to skip compilation and run tests when required kernel configuration flags are missing. This patch introduces a check to validate the presence of required config flags specified in the selftest config files. In case scripts/config or the current kernel config is not found, this check is skipped. In order to view the missing config options required to compile the test, set the environment variable LOCALMODCONFIG_DEBUG=1. example usage: ``` LOCALMODCONFIG_DEBUG=1 make -C livepatch/ ``` Suggested-by: Petr Mladek <pmladek@suse.com> Suggested-by: Miroslav Benes <mbenes@suse.cz> Signed-off-by: Siddharth Menon <simeddon@gmail.com> --- v1->v2: moved the config checking logic to a separate perl script tools/testing/selftests/lib.mk | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) mode change 100644 => 100755 tools/testing/selftests/lib.mk diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk old mode 100644 new mode 100755 index d6edcfcb5be8..98dda6d8d702 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -97,7 +97,14 @@ TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS)) TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED)) TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES)) -all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \ +TEST_DIR := $(shell pwd) + +check_config_deps: + @$(selfdir)/mktest.pl $(TEST_DIR)/config || \ + { echo "Skipping test: $(notdir $(TEST_DIR))"; exit 1; } + + +all: check_config_deps $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \ $(if $(TEST_GEN_MODS_DIR),gen_mods_dir) define RUN_TESTS @@ -228,4 +235,4 @@ $(OUTPUT)/%:%.S $(LINK.S) $^ $(LDLIBS) -o $@ endif -.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir +.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir check_config_deps -- 2.39.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] selftests/lib.mk: Introduce check to validate required configs 2024-12-20 19:35 ` [PATCH v2 2/2] selftests/lib.mk: Introduce check " Siddharth Menon @ 2025-01-07 13:53 ` Petr Mladek 0 siblings, 0 replies; 5+ messages in thread From: Petr Mladek @ 2025-01-07 13:53 UTC (permalink / raw) To: Siddharth Menon; +Cc: shuah, skhan, mbenes, linux-kselftest On Sat 2024-12-21 01:05:36, Siddharth Menon wrote: > Currently, kselftests does not have a generalised mechanism to skip compilation > and run tests when required kernel configuration flags are missing. > > This patch introduces a check to validate the presence of required config flags > specified in the selftest config files. In case scripts/config or the current > kernel config is not found, this check is skipped. > > In order to view the missing config options required to compile the test, > set the environment variable LOCALMODCONFIG_DEBUG=1. As I wrote in the review for the 1st patch, I would prefer to print the missing config options by default. The LOCALMODCONFIG_DEBUG variable is pretty non-standard and hard to memorize thing. > --- a/tools/testing/selftests/lib.mk > +++ b/tools/testing/selftests/lib.mk > @@ -97,7 +97,14 @@ TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS)) > TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED)) > TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES)) > > -all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \ > +TEST_DIR := $(shell pwd) > + > +check_config_deps: > + @$(selfdir)/mktest.pl $(TEST_DIR)/config || \ > + { echo "Skipping test: $(notdir $(TEST_DIR))"; exit 1; } I would write a more meaningful message, e.g. { echo "Skipping test because of missing kernel features: $(notdir $(TEST_DIR))"; exit 1; } > + > +all: check_config_deps $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \ > $(if $(TEST_GEN_MODS_DIR),gen_mods_dir) > > define RUN_TESTS > @@ -228,4 +235,4 @@ $(OUTPUT)/%:%.S > $(LINK.S) $^ $(LDLIBS) -o $@ > endif Otherwise, it seems to work well for the livepatching selftests. I guess that it might prevent running some selftests because of too strict or outdated information in some tools/testing/selftests/<project>/config files. So that it might cause regressions. But I think that this is the right way to go. I am just not sure whether we should wait for complains from linux-next. Or if we should be more proactive in fixing the various <project>/config files. Best Regards, Petr ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-01-07 13:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-20 19:35 [PATCH v2 0/2] update kselftest framework to check for required configs Siddharth Menon 2024-12-20 19:35 ` [PATCH v2 1/2] selftests: Introduce script to validate " Siddharth Menon 2025-01-07 13:44 ` Petr Mladek 2024-12-20 19:35 ` [PATCH v2 2/2] selftests/lib.mk: Introduce check " Siddharth Menon 2025-01-07 13:53 ` Petr Mladek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox