Skip to contentSkip to conversation

kselftest framework to introduce TEST_CONFIG_DEPS

PATCH In review Siddharth Menon 3 participants 12 messages Dec 5, 2024 – Dec 12, 2024

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover kselftest framework to introduce TEST_CONFIG_DEPS

Currently, kselftests does not have a generalised mechanism to skip compilation and run tests when required kernel configuration options are disabled.

This patch series addresses this limitation by introducing a new flag, 'TEST_CONFIG_DEPS' in lib.mk, along with corresponding updates to the documentation.
The selftests/livepatch/Makefile has been updated to utilize TEST_CONFIG_DEPS.

Siddharth Menon (3):
docs/kselftests: Explain the usage of TEST_CONFIG_DEPS selftests/lib.mk: Introduce check to validate required configs selftests/livepatch: Check if required config options are enabled

 Documentation/dev-tools/kselftest.rst      |  3 +++
 tools/testing/selftests/lib.mk             | 18 ++++++++++++++++--
 tools/testing/selftests/livepatch/Makefile |  1 +
 3 files changed, 20 insertions(+), 2 deletions(-)
Signature · 1 line
2.39.5
#1 lore raw

PATCH 1/3 docs/kselftests: Explain the usage of TEST_CONFIG_DEPS

Update documentation to explain the TEST_CONFIG_DEPS flag in lib.mk. TEST_CONFIG_DEPS is used to validate the presence of required config flags specified in the selftest makefile before compiling or running a test.

---
 Documentation/dev-tools/kselftest.rst | 3 +++
 1 file changed, 3 insertions(+)
Documentation/dev-tools/kselftest.rst +3 -0
index fdb1df86783a..e816b282363f 100644
@@ -301,6 +301,9 @@ Contributing new tests (details)
 
    e.g: tools/testing/selftests/android/config
 
+ * Use TEST_CONFIG_DEPS to specify required config options to be enabled 
+   before a test is allowed to run or compile.
+
  * Create a .gitignore file inside test directory and add all generated objects
    in it.
 
Signature · 1 line
2.39.5
maintainer
#2 lore raw

Re: [PATCH 1/3] docs/kselftests: Explain the usage of TEST_CONFIG_DEPS

Quoted 14 lines · Siddharth Menon

Update documentation to explain the TEST_CONFIG_DEPS flag in lib.mk. TEST_CONFIG_DEPS is used to validate the presence of required config flags specified in the selftest makefile before compiling or running a test.

---
 Documentation/dev-tools/kselftest.rst | 3 +++
 1 file changed, 3 insertions(+)
Documentation/dev-tools/kselftest.rst +0 -0
index fdb1df86783a..e816b282363f 100644

Documentation/dev-tools/kselftest.rst Contributing new tests (details) · line 308

Quoted 8 lines
Documentation/dev-tools/kselftest.rst +3 -0
@@ -301,6 +301,9 @@ Contributing new tests (details)
 
    e.g: tools/testing/selftests/android/config
 
+ * Use TEST_CONFIG_DEPS to specify required config options to be enabled 
+   before a test is allowed to run or compile.
+
  * Create a .gitignore file inside test directory and add all generated objects
    in it.

It might be a matter of taste. It is a chicken & egg problem. I personally find it weird to document something which does not exist yet.

Please, either update the documentation together with the code or later :-)

Best Regards,
Petr

PS: I haven't got this mail. I have got only 2nd and 3rd patch.
I prefer to see the entire patchset. I suggest to always send all patches to the same list of people and mailing lists.

#3 lore raw

PATCH 2/3 selftests/lib.mk: Introduce check to validate required configs

In reply to Siddharth Menon

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 makefile. In case scripts/config is not found, this check is skipped.

Use TEST_CONFIG_DEPS to check for specific config options before compiling, example usage:

```
TEST_CONFIG_DEPS := CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG
```

Here it checks whether CONFIG_LIVEPATCH and CONFIG_DYNAMIC_DEBUG are enabled.

---
 tools/testing/selftests/lib.mk | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
tools/testing/selftests/lib.mk +16 -2
index d6edcfcb5be8..7ca713237bf7 100644
@@ -97,7 +97,21 @@ 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) \
+KDIR ?= /lib/modules/$(shell uname -r)/build
+
+define CHECK_CONFIG_DEPS
+    $(if $(wildcard $(KDIR)/scripts/config),
+        $(eval MISSING_FLAGS := $(filter-out 1,$(foreach cfg,$(TEST_CONFIG_DEPS),\
+            $(shell cd $(KDIR) && scripts/config --state $(cfg) | grep -q '^\(y\|m\)$$' && echo 1 || echo $(cfg))))),
+        $(info Skipping CHECK_GEN_REQ: $(KDIR)/scripts/config not found)
+    )
+    $(if $(MISSING_FLAGS),$(error Missing required config flags: $(MISSING_FLAGS)))
+endef
+
+check_config_deps:
+	$(call CHECK_CONFIG_DEPS)
+
+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 +242,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
Signature · 1 line
2.39.5
#4 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

Quoted 2 lines · Siddharth Menon

Currently, kselftests does not have a generalised mechanism to skip compilation and run tests when required kernel configuration flags are missing.

Should this be a build dependency or only a runtime dependency, or should these be separate options for cases where the selftest builds a module? If people are building the selftests once and then using them with a bunch of kernel builds it might be surprising if some of the binaries vanish.

tools/testing/selftests/lib.mk TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS)) · line 101

Quoted 3 lines
tools/testing/selftests/lib.mk +2 -1
-all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \
+KDIR ?= /lib/modules/$(shell uname -r)/build
+

Shouldn't we try the current kernel tree, and for runtime checks /proc/config.gz would be good to check when it's enabled?

tools/testing/selftests/lib.mk TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS)) · line 109

Quoted 8 lines
tools/testing/selftests/lib.mk +8 -0
+define CHECK_CONFIG_DEPS
+    $(if $(wildcard $(KDIR)/scripts/config),
+        $(eval MISSING_FLAGS := $(filter-out 1,$(foreach cfg,$(TEST_CONFIG_DEPS),\
+            $(shell cd $(KDIR) && scripts/config --state $(cfg) | grep -q '^\(y\|m\)$$' && echo 1 || echo $(cfg))))),
+        $(info Skipping CHECK_GEN_REQ: $(KDIR)/scripts/config not found)
+    )
+    $(if $(MISSING_FLAGS),$(error Missing required config flags: $(MISSING_FLAGS)))
+endef

This is going to use a separate set of config options to those listed in the config file in the selftest directory which is perhaps a bit surprising. OTOH we do have a lot of the selftests directories where not every test needs all the options so that's probably a good choice unless we make things finer grained which might be more trouble than it's worth.

#5 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

Quoted 3 lines · Mark Brown

Should this be a build dependency or only a runtime dependency, or should these be separate options for cases where the selftest builds a module?

Hello, thanks for looking through my patch. Some tests fail to compile and throw errors in case their required config options are not enabled. This optional check was created to prevent such issues.

Quoted 3 lines

If people are building the selftests once and then using them with a bunch of kernel builds it might be surprising if some of the binaries vanish.

I'm not really familiar with packaging selftests, but this patch does not remove existing binaries when running tests. If I misunderstood what you are referring to, please let me know.

Quoted 2 lines

Shouldn't we try the current kernel tree, and for runtime checks /proc/config.gz would be good to check when it's enabled?

When I looked into this, it seems (according to the config.gz man page) that the contents of /proc/config.gz are the same as /lib/modules/$(uname -r)/build/.config, but /proc/config.gz is only available if the kernel was compiled with CONFIG_IKCONFIG_PROC enabled.

It does seem like /lib/modules/$(uname -r)/build/scripts is not always available though, so will send in a new patch directly checking build/.config after checking
it out on a few more systems.

#6 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

Quoted 9 lines · BiscuitBobby
Quoted 3 lines · Mark Brown

If people are building the selftests once and then using them with a bunch of kernel builds it might be surprising if some of the binaries vanish.

I'm not really familiar with packaging selftests, but this patch does not remove existing binaries when running tests. If I misunderstood what you are referring to, please let me know.

It doesn't remove existing barriers but it's also not obvious that this isn't just a generic dependency mechanism that should be used for any old dependency rather than things that would actually break the build, and it's one config list for both build and runtime checks. If the intention is that this should be infrequently used it probably needs to be a bit clearer that that's the case. As things stand I'd expect there to be some confusion about the interaction between this and the existing system with specifying config fragments.

Quoted 8 lines
Quoted 2 lines

Shouldn't we try the current kernel tree, and for runtime checks /proc/config.gz would be good to check when it's enabled?

When I looked into this, it seems (according to the config.gz man page) that the contents of /proc/config.gz are the same as /lib/modules/$(uname -r)/build/.config, but /proc/config.gz is only available if the kernel was compiled with CONFIG_IKCONFIG_PROC enabled.

Quoted 4 lines

It does seem like /lib/modules/$(uname -r)/build/scripts is not always available though, so will send in a new patch directly checking build/.config after checking
it out on a few more systems.

Indeed, when deploying a kernel for test people don't always deploy modules onto the target filesystem, there may not be any modules at all if CONFIG_MODULES is disabled, or people might just not install modules that do get built if they're not needed on a given platform.

#7 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

Quoted 9 lines · Mark Brown

It doesn't remove existing barriers but it's also not obvious that this isn't just a generic dependency mechanism that should be used for any old dependency rather than things that would actually break the build, and it's one config list for both build and runtime checks. If the intention is that this should be infrequently used it probably needs to be a bit clearer that that's the case. As things stand I'd expect there to be some confusion about the interaction between this and the existing system with specifying config fragments.

I shall update the commit and documentation to make this more clear. Thank you for the feedback.

maintainer
#8 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

In reply to Siddharth Menon
Quoted 11 lines · Siddharth Menon

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 makefile. In case scripts/config is not found, this check is skipped.

Use TEST_CONFIG_DEPS to check for specific config options before compiling, example usage:
```
TEST_CONFIG_DEPS := CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG

What is the reason to add another set of dependencies, please?

Both CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG are already mentioned in tools/testing/selftests/livepatch/config

IMHO, the new check should read the dependencies from the existing tools/testing/selftests/<test>/config file.

Quoted 7 lines
tools/testing/selftests/lib.mk +0 -1
@@ -97,7 +97,21 @@ 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) \

tools/testing/selftests/lib.mk TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS)) · line 109

Quoted 10 lines
tools/testing/selftests/lib.mk +10 -0
+KDIR ?= /lib/modules/$(shell uname -r)/build
+
+define CHECK_CONFIG_DEPS
+    $(if $(wildcard $(KDIR)/scripts/config),
+        $(eval MISSING_FLAGS := $(filter-out 1,$(foreach cfg,$(TEST_CONFIG_DEPS),\
+            $(shell cd $(KDIR) && scripts/config --state $(cfg) | grep -q '^\(y\|m\)$$' && echo 1 || echo $(cfg))))),
+        $(info Skipping CHECK_GEN_REQ: $(KDIR)/scripts/config not found)
+    )
+    $(if $(MISSING_FLAGS),$(error Missing required config flags: $(MISSING_FLAGS)))
+endef

It somehow does not work here. I get:

tools/testing/selftests/livepatch # make run_tests
grep: .config: No such file or directory
grep: .config: No such file or directory
grep: .config: No such file or directory
grep: .config: No such file or directory
../lib.mk:112: *** Missing required config flags: CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG. Stop.

I run the livepatch tests the following way.

1. On my workstation, I build the kernel RPMs using

make rpm-pkg

2. In qemu test system, I mount the build directory from the workstation and install both kernel and kernel-devel packages:

    rpm -ivh rpmbuild/RPMS/x86_64/kernel-6.12.0_default+-35.x86_64.rpm
    rpm -ivh rpmbuild/RPMS/x86_64/kernel-devel-6.12.0_default+-35.x86_64.rpm

and reboot

3. In rebooted qemu test system, I mount once again the build directory from the workstation and run the tests:

cd tools/testing/selftests/livepatch
make run_tests

The "grep" errors come from the "scripts/config" command. For example:

tools/testing/selftests/livepatch # /lib/modules/6.12.0-default+/build/scripts/config -s CONFIG_LIVEPATCH
grep: .config: No such file or directory
grep: .config: No such file or directory
undef

It helps to define patch to the config file installed by the devel package:

   tools/testing/selftests/livepatch # /lib/modules/6.12.0-default+/build/scripts/config --file /lib/modules/6.12.0-default+/config -s CONFIG_LIVEPATCH
   y

But I am not sure if this works when people run the "make" in the original build directory on the workstation.

Best Regards,
Petr

#9 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

Quoted 2 lines · Petr Mladek

What is the reason to add another set of dependencies, please?

I had done this because not every test required all the options specified in tools/testing/selftests/<test>/config. I thought it would not be desirable to prevent these tests from compiling/running.

Quoted 2 lines

Both CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG are already mentioned in tools/testing/selftests/livepatch/config

This particular test only required CONFIG_LIVEPATCH to compile, but I had included CONFIG_DYNAMIC_DEBUG, as Miroslav had expressed wanting both of them checked.

Quoted 2 lines

IMHO, the new check should read the dependencies from the existing tools/testing/selftests/<test>/config file.

I shall check tools/testing/selftests/<test>/config in my next patch as suggested.

Quoted 12 lines

I run the livepatch tests the following way.

1. On my workstation, I build the kernel RPMs using

make rpm-pkg

2. In qemu test system, I mount the build directory from the workstation and install both kernel and kernel-devel packages:

    rpm -ivh rpmbuild/RPMS/x86_64/kernel-6.12.0_default+-35.x86_64.rpm
    rpm -ivh rpmbuild/RPMS/x86_64/kernel-devel-6.12.0_default+-35.x86_64.rpm
Quoted 7 lines

and reboot

3. In rebooted qemu test system, I mount once again the build directory from the workstation and run the tests:

cd tools/testing/selftests/livepatch
make run_tests

Thanks, I will test building kernel RPMs when I update the check to be more distro agnostic.

Sincerely,
Siddharth Menon

maintainer
#10 lore raw

Re: [PATCH 2/3] selftests/lib.mk: Introduce check to validate required configs

Quoted 7 lines · BiscuitBobby
Quoted 2 lines · Petr Mladek

What is the reason to add another set of dependencies, please?

I had done this because not every test required all the options specified in tools/testing/selftests/<test>/config. I thought it would not be desirable to prevent these tests from compiling/running.

The biggest problem is that tools/testing/selftests/<test>/config are not used during build or tests at the moment. It means that they are not tested and might be outdated.

If we add the dependency then some <test>/config files might need to get fixed.

I am not sure how many problems it might cause. But it might be worth the effort.

Quoted 6 lines
Quoted 2 lines

Both CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG are already mentioned in tools/testing/selftests/livepatch/config

This particular test only required CONFIG_LIVEPATCH to compile, but I had included CONFIG_DYNAMIC_DEBUG, as Miroslav had expressed wanting both of them checked.

I see. The build succeeds even without CONFIG_DYNAMIC_DEBUG. But it must be enabled on the kernel where the test modules are loaded. Otherwise, the tests would fail.

Honestly, I think that this is rather an exception. It works only because all the needed pr_debug() messages are in the livepatch core code. The test modules do not use pr_debug() on its own.

I believe that most options in tools/testing/selftests/<test>/config have to be enabled on both compile and runtime. Otherwise, the test binaries might not have access to the needed API.

I suggest to keep it simple and require all <test>/config options at both compile and run time. IMHO, most people build and run the tests on the same kernel anyway.

Best Regards,
Petr

#11 lore raw

PATCH 3/3 selftests/livepatch: Check if required config options are enabled

In reply to Siddharth Menon

When CONFIG_LIVEPATCH is disabled, compilation fails due to the required structs from the livepatch header file being undefined. This checks for whether CONFIG_LIVEPATCH and CONFIG_DYNAMIC_DEBUG are enabled before compiling livepatch self-tests.

---
 tools/testing/selftests/livepatch/Makefile | 1 +
 1 file changed, 1 insertion(+)
tools/testing/selftests/livepatch/Makefile +1 -0
index a080eb54a215..14b5c60663cd 100644
@@ -14,5 +14,6 @@ TEST_PROGS := \
 	test-kprobe.sh
 
 TEST_FILES := settings
+TEST_CONFIG_DEPS := CONFIG_LIVEPATCH CONFIG_DYNAMIC_DEBUG
 
 include ../lib.mk
Signature · 1 line
2.39.5