| Message ID | 20231220-send-lp-kselftests-v4-1-3458ec1b1a38@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | livepatch: Move modules to selftests and add a new test | expand |
On Wed, Dec 20, 2023 at 01:53:12PM -0300, Marcos Paulo de Souza wrote: > Add TEST_GEN_MODS_DIR variable for kselftests. It can point to > a directory containing kernel modules that will be used by > selftest scripts. > > The modules are built as external modules for the running kernel. > As a result they are always binary compatible and the same tests > can be used for older or newer kernels. > > The build requires "kernel-devel" package to be installed. > For example, in the upstream sources, the rpm devel package > is produced by "make rpm-pkg" > > The modules can be built independently by > > make -C tools/testing/selftests/livepatch/ > > or they will be automatically built before running the tests via > > make -C tools/testing/selftests/livepatch/ run_tests > > Note that they are _not_ built when running the standalone > tests by calling, for example, ./test-state.sh. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > --- > Documentation/dev-tools/kselftest.rst | 4 ++++ > tools/testing/selftests/lib.mk | 20 +++++++++++++++----- > 2 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst > index ab376b316c36..7f3582a67318 100644 > --- a/Documentation/dev-tools/kselftest.rst > +++ b/Documentation/dev-tools/kselftest.rst > @@ -245,6 +245,10 @@ Contributing new tests (details) > TEST_PROGS, TEST_GEN_PROGS mean it is the executable tested by > default. > > + TEST_GEN_MODS_DIR should be used by tests that require modules to be built > + before the test starts. The variable will contain the name of the directory > + containing the modules. > + > TEST_CUSTOM_PROGS should be used by tests that require custom build > rules and prevent common build rule use. > > diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk > index 118e0964bda9..6c7c5a0112cf 100644 > --- a/tools/testing/selftests/lib.mk > +++ b/tools/testing/selftests/lib.mk > @@ -70,12 +70,15 @@ KHDR_INCLUDES := -isystem $(KHDR_DIR) > # TEST_PROGS are for test shell scripts. > # TEST_CUSTOM_PROGS and TEST_PROGS will be run by common run_tests > # and install targets. Common clean doesn't touch them. > +# TEST_GEN_MODS_DIR is used to specify a directory with modules to be built > +# before the test executes. These modules are cleaned on the clean target as well. > 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)) > +TEST_GEN_MODS_DIR := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_MODS_DIR)) > > all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \ > - $(TEST_GEN_FILES) > + $(TEST_GEN_FILES) $(if $(TEST_GEN_MODS_DIR),gen_mods_dir) > > kernel_header_files: > @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ > @@ -105,8 +108,8 @@ endef > > run_tests: all > ifdef building_out_of_srctree > - @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \ > - rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \ > + @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)$(TEST_GEN_MODS_DIR)" != "X" ]; then \ > + rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(TEST_GEN_MODS_DIR) $(OUTPUT); \ > fi > @if [ "X$(TEST_PROGS)" != "X" ]; then \ > $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \ > @@ -118,6 +121,12 @@ else > @$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS)) > endif > > +gen_mods_dir: > + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) > + > +clean_mods_dir: > + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean > + > define INSTALL_SINGLE_RULE > $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)) > $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/) > @@ -131,6 +140,7 @@ define INSTALL_RULE > $(eval INSTALL_LIST = $(TEST_CUSTOM_PROGS)) $(INSTALL_SINGLE_RULE) > $(eval INSTALL_LIST = $(TEST_GEN_PROGS_EXTENDED)) $(INSTALL_SINGLE_RULE) > $(eval INSTALL_LIST = $(TEST_GEN_FILES)) $(INSTALL_SINGLE_RULE) > + $(eval INSTALL_LIST = $(TEST_GEN_MODS_DIR)) $(INSTALL_SINGLE_RULE) Hi Marcos, Sorry for the late reply on this, but I'm reviewing this version by trying to retrofit it into our selftest packaging (pre-build the test module .ko's and stash those into an rpm rather than building on the test host). Since $TEST_GEN_MODS_DIR is treated as a directory, I found that the selftest install target copies a bunch of intermediate object and kbuild files: $ mkdir /tmp/test-install $ make KDIR=$(pwd) INSTALL_PATH=/tmp/test-install TARGETS=livepatch \ -C tools/testing/selftests/ install [ ... builds livepatch selftests ... ] the rsync in question: rsync -a --copy-unsafe-links /home/jolawren/src/kernel/tools/testing/selftests/livepatch/test_modules /tmp/test-install/livepatch/ ... and then looking at the destination: $ tree -a /tmp/test-install/ /tmp/test-install/ ├── kselftest │ ├── module.sh │ ├── prefix.pl │ └── runner.sh ├── kselftest-list.txt ├── livepatch │ ├── config │ ├── functions.sh │ ├── settings │ ├── test-callbacks.sh │ ├── test-ftrace.sh │ ├── test_klp-call_getpid │ ├── test-livepatch.sh │ ├── test_modules │ │ ├── Makefile │ │ ├── modules.order │ │ ├── .modules.order.cmd │ │ ├── Module.symvers │ │ ├── .Module.symvers.cmd │ │ ├── test_klp_atomic_replace.c │ │ ├── test_klp_atomic_replace.ko │ │ ├── .test_klp_atomic_replace.ko.cmd │ │ ├── test_klp_atomic_replace.mod │ │ ├── test_klp_atomic_replace.mod.c │ │ ├── .test_klp_atomic_replace.mod.cmd │ │ ├── test_klp_atomic_replace.mod.o │ │ ├── .test_klp_atomic_replace.mod.o.cmd │ │ ├── test_klp_atomic_replace.o │ │ ├── .test_klp_atomic_replace.o.cmd ... On the other hand, variables like $TEST_GEN_FILES specify individual files, so only final binaries like test_klp-call_getpid (and not test_klp-call_getpid.c) are copied to $INSTALL_PATH. Since the selftest module builds appear to ignore CONFIG_MODULE_COMPRESS_* the smallest tweak I can think of to avoid the above scenario is: --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -106,7 +106,7 @@ define INSTALL_RULE $(eval INSTALL_LIST = $(TEST_CUSTOM_PROGS)) $(INSTALL_SINGLE_RULE) $(eval INSTALL_LIST = $(TEST_GEN_PROGS_EXTENDED)) $(INSTALL_SINGLE_RULE) $(eval INSTALL_LIST = $(TEST_GEN_FILES)) $(INSTALL_SINGLE_RULE) - $(eval INSTALL_LIST = $(TEST_GEN_MODS_DIR)) $(INSTALL_SINGLE_RULE) + $(eval INSTALL_LIST = $(shell sed 's/.o$$/.ko/' $(TEST_GEN_MODS_DIR)/modules.order)) $(INSTALL_SINGLE_RULE) $(eval INSTALL_LIST = $(wildcard config settings)) $(INSTALL_SINGLE_RULE) endef However, that will copy .ko's directly into $INSTALL_PATH and out of the $TEST_GEN_MODS_DIR subdirectory(s), so maybe not a great solution after all. Anyway, I thought I might mention this in case it runs against the spirit of the selftest install target. I only tripped over it while digging into the bowels of our kernel specfile and discovered that it invoked this target. -- Joe
On 1/2/24 15:31, Joe Lawrence wrote: > On Wed, Dec 20, 2023 at 01:53:12PM -0300, Marcos Paulo de Souza wrote: >> Add TEST_GEN_MODS_DIR variable for kselftests. It can point to >> a directory containing kernel modules that will be used by >> selftest scripts. >> >> The modules are built as external modules for the running kernel. >> As a result they are always binary compatible and the same tests >> can be used for older or newer kernels. >> >> The build requires "kernel-devel" package to be installed. >> For example, in the upstream sources, the rpm devel package >> is produced by "make rpm-pkg" >> >> The modules can be built independently by >> >> make -C tools/testing/selftests/livepatch/ >> >> or they will be automatically built before running the tests via >> >> make -C tools/testing/selftests/livepatch/ run_tests >> >> Note that they are _not_ built when running the standalone >> tests by calling, for example, ./test-state.sh. >> >> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> >> --- >> Documentation/dev-tools/kselftest.rst | 4 ++++ >> tools/testing/selftests/lib.mk | 20 +++++++++++++++----- >> 2 files changed, 19 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst >> index ab376b316c36..7f3582a67318 100644 >> --- a/Documentation/dev-tools/kselftest.rst >> +++ b/Documentation/dev-tools/kselftest.rst >> @@ -245,6 +245,10 @@ Contributing new tests (details) >> TEST_PROGS, TEST_GEN_PROGS mean it is the executable tested by >> default. >> >> + TEST_GEN_MODS_DIR should be used by tests that require modules to be built >> + before the test starts. The variable will contain the name of the directory >> + containing the modules. >> + >> TEST_CUSTOM_PROGS should be used by tests that require custom build >> rules and prevent common build rule use. >> >> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk >> index 118e0964bda9..6c7c5a0112cf 100644 >> --- a/tools/testing/selftests/lib.mk >> +++ b/tools/testing/selftests/lib.mk >> @@ -70,12 +70,15 @@ KHDR_INCLUDES := -isystem $(KHDR_DIR) >> # TEST_PROGS are for test shell scripts. >> # TEST_CUSTOM_PROGS and TEST_PROGS will be run by common run_tests >> # and install targets. Common clean doesn't touch them. >> +# TEST_GEN_MODS_DIR is used to specify a directory with modules to be built >> +# before the test executes. These modules are cleaned on the clean target as well. >> 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)) >> +TEST_GEN_MODS_DIR := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_MODS_DIR)) >> >> all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \ >> - $(TEST_GEN_FILES) >> + $(TEST_GEN_FILES) $(if $(TEST_GEN_MODS_DIR),gen_mods_dir) >> >> kernel_header_files: >> @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ >> @@ -105,8 +108,8 @@ endef >> >> run_tests: all >> ifdef building_out_of_srctree >> - @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \ >> - rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \ >> + @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)$(TEST_GEN_MODS_DIR)" != "X" ]; then \ >> + rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(TEST_GEN_MODS_DIR) $(OUTPUT); \ >> fi >> @if [ "X$(TEST_PROGS)" != "X" ]; then \ >> $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \ >> @@ -118,6 +121,12 @@ else >> @$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS)) >> endif >> >> +gen_mods_dir: >> + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) >> + >> +clean_mods_dir: >> + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean >> + >> define INSTALL_SINGLE_RULE >> $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)) >> $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/) >> @@ -131,6 +140,7 @@ define INSTALL_RULE >> $(eval INSTALL_LIST = $(TEST_CUSTOM_PROGS)) $(INSTALL_SINGLE_RULE) >> $(eval INSTALL_LIST = $(TEST_GEN_PROGS_EXTENDED)) $(INSTALL_SINGLE_RULE) >> $(eval INSTALL_LIST = $(TEST_GEN_FILES)) $(INSTALL_SINGLE_RULE) >> + $(eval INSTALL_LIST = $(TEST_GEN_MODS_DIR)) $(INSTALL_SINGLE_RULE) > > Hi Marcos, > > Sorry for the late reply on this, but I'm reviewing this version by > trying to retrofit it into our selftest packaging (pre-build the test > module .ko's and stash those into an rpm rather than building on the > test host). > > Since $TEST_GEN_MODS_DIR is treated as a directory, I found that the > selftest install target copies a bunch of intermediate object and kbuild > files: > > $ mkdir /tmp/test-install > $ make KDIR=$(pwd) INSTALL_PATH=/tmp/test-install TARGETS=livepatch \ > -C tools/testing/selftests/ install > > [ ... builds livepatch selftests ... ] > > the rsync in question: > > rsync -a --copy-unsafe-links /home/jolawren/src/kernel/tools/testing/selftests/livepatch/test_modules /tmp/test-install/livepatch/ > ... > > and then looking at the destination: > > $ tree -a /tmp/test-install/ > /tmp/test-install/ > ├── kselftest > │ ├── module.sh > │ ├── prefix.pl > │ └── runner.sh > ├── kselftest-list.txt > ├── livepatch > │ ├── config > │ ├── functions.sh > │ ├── settings > │ ├── test-callbacks.sh > │ ├── test-ftrace.sh > │ ├── test_klp-call_getpid > │ ├── test-livepatch.sh > │ ├── test_modules > │ │ ├── Makefile > │ │ ├── modules.order > │ │ ├── .modules.order.cmd > │ │ ├── Module.symvers > │ │ ├── .Module.symvers.cmd > │ │ ├── test_klp_atomic_replace.c > │ │ ├── test_klp_atomic_replace.ko > │ │ ├── .test_klp_atomic_replace.ko.cmd > │ │ ├── test_klp_atomic_replace.mod > │ │ ├── test_klp_atomic_replace.mod.c > │ │ ├── .test_klp_atomic_replace.mod.cmd > │ │ ├── test_klp_atomic_replace.mod.o > │ │ ├── .test_klp_atomic_replace.mod.o.cmd > │ │ ├── test_klp_atomic_replace.o > │ │ ├── .test_klp_atomic_replace.o.cmd > ... > > On the other hand, variables like $TEST_GEN_FILES specify individual > files, so only final binaries like test_klp-call_getpid (and not > test_klp-call_getpid.c) are copied to $INSTALL_PATH. Thank you Joe for finding this problem. Copying source files and object files doesn't sound right. This isn't how the ksleftest installs work. Let's fix this. thanks, --Shuah
On Wed, 2024-01-03 at 15:09 -0700, Shuah Khan wrote: > On 1/2/24 15:31, Joe Lawrence wrote: > > On Wed, Dec 20, 2023 at 01:53:12PM -0300, Marcos Paulo de Souza > > wrote: > > > Add TEST_GEN_MODS_DIR variable for kselftests. It can point to > > > a directory containing kernel modules that will be used by > > > selftest scripts. > > > > > > The modules are built as external modules for the running kernel. > > > As a result they are always binary compatible and the same tests > > > can be used for older or newer kernels. > > > > > > The build requires "kernel-devel" package to be installed. > > > For example, in the upstream sources, the rpm devel package > > > is produced by "make rpm-pkg" > > > > > > The modules can be built independently by > > > > > > make -C tools/testing/selftests/livepatch/ > > > > > > or they will be automatically built before running the tests via > > > > > > make -C tools/testing/selftests/livepatch/ run_tests > > > > > > Note that they are _not_ built when running the standalone > > > tests by calling, for example, ./test-state.sh. > > > > > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > > > --- > > > Documentation/dev-tools/kselftest.rst | 4 ++++ > > > tools/testing/selftests/lib.mk | 20 +++++++++++++++----- > > > 2 files changed, 19 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/dev-tools/kselftest.rst > > > b/Documentation/dev-tools/kselftest.rst > > > index ab376b316c36..7f3582a67318 100644 > > > --- a/Documentation/dev-tools/kselftest.rst > > > +++ b/Documentation/dev-tools/kselftest.rst > > > @@ -245,6 +245,10 @@ Contributing new tests (details) > > > TEST_PROGS, TEST_GEN_PROGS mean it is the executable tested > > > by > > > default. > > > > > > + TEST_GEN_MODS_DIR should be used by tests that require > > > modules to be built > > > + before the test starts. The variable will contain the name of > > > the directory > > > + containing the modules. > > > + > > > TEST_CUSTOM_PROGS should be used by tests that require > > > custom build > > > rules and prevent common build rule use. > > > > > > diff --git a/tools/testing/selftests/lib.mk > > > b/tools/testing/selftests/lib.mk > > > index 118e0964bda9..6c7c5a0112cf 100644 > > > --- a/tools/testing/selftests/lib.mk > > > +++ b/tools/testing/selftests/lib.mk > > > @@ -70,12 +70,15 @@ KHDR_INCLUDES := -isystem $(KHDR_DIR) > > > # TEST_PROGS are for test shell scripts. > > > # TEST_CUSTOM_PROGS and TEST_PROGS will be run by common > > > run_tests > > > # and install targets. Common clean doesn't touch them. > > > +# TEST_GEN_MODS_DIR is used to specify a directory with modules > > > to be built > > > +# before the test executes. These modules are cleaned on the > > > clean target as well. > > > 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)) > > > +TEST_GEN_MODS_DIR := $(patsubst > > > %,$(OUTPUT)/%,$(TEST_GEN_MODS_DIR)) > > > > > > all: kernel_header_files $(TEST_GEN_PROGS) > > > $(TEST_GEN_PROGS_EXTENDED) \ > > > - $(TEST_GEN_FILES) > > > + $(TEST_GEN_FILES) $(if $(TEST_GEN_MODS_DIR),gen_mods_dir) > > > > > > kernel_header_files: > > > @ls $(KHDR_DIR)/linux/*.h >/dev/null > > > 2>/dev/null; \ > > > @@ -105,8 +108,8 @@ endef > > > > > > run_tests: all > > > ifdef building_out_of_srctree > > > - @if [ > > > "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; > > > then \ > > > - rsync -aq --copy-unsafe-links $(TEST_PROGS) > > > $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \ > > > + @if [ > > > "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)$(TEST_GEN_MODS > > > _DIR)" != "X" ]; then \ > > > + rsync -aq --copy-unsafe-links $(TEST_PROGS) > > > $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(TEST_GEN_MODS_DIR) > > > $(OUTPUT); \ > > > fi > > > @if [ "X$(TEST_PROGS)" != "X" ]; then \ > > > $(call RUN_TESTS, $(TEST_GEN_PROGS) > > > $(TEST_CUSTOM_PROGS) \ > > > @@ -118,6 +121,12 @@ else > > > @$(call RUN_TESTS, $(TEST_GEN_PROGS) > > > $(TEST_CUSTOM_PROGS) $(TEST_PROGS)) > > > endif > > > > > > +gen_mods_dir: > > > + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) > > > + > > > +clean_mods_dir: > > > + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean > > > + > > > define INSTALL_SINGLE_RULE > > > $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)) > > > $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links > > > $(INSTALL_LIST) $(INSTALL_PATH)/) > > > @@ -131,6 +140,7 @@ define INSTALL_RULE > > > $(eval INSTALL_LIST = $(TEST_CUSTOM_PROGS)) > > > $(INSTALL_SINGLE_RULE) > > > $(eval INSTALL_LIST = $(TEST_GEN_PROGS_EXTENDED)) > > > $(INSTALL_SINGLE_RULE) > > > $(eval INSTALL_LIST = $(TEST_GEN_FILES)) > > > $(INSTALL_SINGLE_RULE) > > > + $(eval INSTALL_LIST = $(TEST_GEN_MODS_DIR)) > > > $(INSTALL_SINGLE_RULE) > > > > Hi Marcos, > > > > Sorry for the late reply on this, but I'm reviewing this version by > > trying to retrofit it into our selftest packaging (pre-build the > > test > > module .ko's and stash those into an rpm rather than building on > > the > > test host). > > > > Since $TEST_GEN_MODS_DIR is treated as a directory, I found that > > the > > selftest install target copies a bunch of intermediate object and > > kbuild > > files: > > > > $ mkdir /tmp/test-install > > $ make KDIR=$(pwd) INSTALL_PATH=/tmp/test-install > > TARGETS=livepatch \ > > -C tools/testing/selftests/ install > > > > [ ... builds livepatch selftests ... ] > > > > the rsync in question: > > > > rsync -a --copy-unsafe-links > > /home/jolawren/src/kernel/tools/testing/selftests/livepatch/test_mo > > dules /tmp/test-install/livepatch/ > > ... > > > > and then looking at the destination: > > > > $ tree -a /tmp/test-install/ > > /tmp/test-install/ > > ├── kselftest > > │ ├── module.sh > > │ ├── prefix.pl > > │ └── runner.sh > > ├── kselftest-list.txt > > ├── livepatch > > │ ├── config > > │ ├── functions.sh > > │ ├── settings > > │ ├── test-callbacks.sh > > │ ├── test-ftrace.sh > > │ ├── test_klp-call_getpid > > │ ├── test-livepatch.sh > > │ ├── test_modules > > │ │ ├── Makefile > > │ │ ├── modules.order > > │ │ ├── .modules.order.cmd > > │ │ ├── Module.symvers > > │ │ ├── .Module.symvers.cmd > > │ │ ├── test_klp_atomic_replace.c > > │ │ ├── test_klp_atomic_replace.ko > > │ │ ├── .test_klp_atomic_replace.ko.cmd > > │ │ ├── test_klp_atomic_replace.mod > > │ │ ├── test_klp_atomic_replace.mod.c > > │ │ ├── .test_klp_atomic_replace.mod.cmd > > │ │ ├── test_klp_atomic_replace.mod.o > > │ │ ├── .test_klp_atomic_replace.mod.o.cmd > > │ │ ├── test_klp_atomic_replace.o > > │ │ ├── .test_klp_atomic_replace.o.cmd > > ... > > > > On the other hand, variables like $TEST_GEN_FILES specify > > individual > > files, so only final binaries like test_klp-call_getpid (and not > > test_klp-call_getpid.c) are copied to $INSTALL_PATH. Hi Joe, thanks for catching this issue. I crafted the attached patch and it fixes the issue for me, copying only the resulting .ko objects as expected. Can you please check if this fixes the issue for you? > > > Thank you Joe for finding this problem. > > Copying source files and object files doesn't sound right. This isn't > how the ksleftest installs work. Let's fix this. Hi Shuah, what do you think about the proposed solution? Could you please amend the fix into the first patch if you think it's the right approach? Thanks in advance! Marcos > > thanks, > --Shuah >
On 1/8/24 10:13, Marcos Paulo de Souza wrote: > On Wed, 2024-01-03 at 15:09 -0700, Shuah Khan wrote: >> >> Copying source files and object files doesn't sound right. This isn't >> how the ksleftest installs work. Let's fix this. > > Hi Shuah, > > what do you think about the proposed solution? Could you please amend > the fix into the first patch if you think it's the right approach? > I would like to see a new revision of the patch series with the fix to the problem. I will pull this into a separate test branch for us all to test different scenarios. I would like to make sure the repo will stay clean after install in the case of when out of tree builds. Sorry I can't amend the patch as this isn't a trivial merge change. This change requires more testing. thanks, -- Shuah
On Tue, 2024-01-09 at 12:31 -0700, Shuah Khan wrote: > On 1/8/24 10:13, Marcos Paulo de Souza wrote: > > On Wed, 2024-01-03 at 15:09 -0700, Shuah Khan wrote: > > > > > > > Copying source files and object files doesn't sound right. This > > > isn't > > > how the ksleftest installs work. Let's fix this. > > > > Hi Shuah, > > > > what do you think about the proposed solution? Could you please > > amend > > the fix into the first patch if you think it's the right approach? > > > > I would like to see a new revision of the patch series with the fix > to > the problem. I will pull this into a separate test branch for us all > to test different scenarios. I would like to make sure the repo will > stay clean after install in the case of when out of tree builds. > > Sorry I can't amend the patch as this isn't a trivial merge change. > This change requires more testing. I sent a v5 of the patches. This new version has that diff that I sent earlier to avoid copying the Kbuild files. It worked on make install and with gen_tar. Feel free to use this version in your test branch then. Thanks in advance, Marcos > > thanks, > -- Shuah > > >
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index ab376b316c36..7f3582a67318 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -245,6 +245,10 @@ Contributing new tests (details) TEST_PROGS, TEST_GEN_PROGS mean it is the executable tested by default. + TEST_GEN_MODS_DIR should be used by tests that require modules to be built + before the test starts. The variable will contain the name of the directory + containing the modules. + TEST_CUSTOM_PROGS should be used by tests that require custom build rules and prevent common build rule use. diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 118e0964bda9..6c7c5a0112cf 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -70,12 +70,15 @@ KHDR_INCLUDES := -isystem $(KHDR_DIR) # TEST_PROGS are for test shell scripts. # TEST_CUSTOM_PROGS and TEST_PROGS will be run by common run_tests # and install targets. Common clean doesn't touch them. +# TEST_GEN_MODS_DIR is used to specify a directory with modules to be built +# before the test executes. These modules are cleaned on the clean target as well. 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)) +TEST_GEN_MODS_DIR := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_MODS_DIR)) all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \ - $(TEST_GEN_FILES) + $(TEST_GEN_FILES) $(if $(TEST_GEN_MODS_DIR),gen_mods_dir) kernel_header_files: @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ @@ -105,8 +108,8 @@ endef run_tests: all ifdef building_out_of_srctree - @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \ - rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \ + @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)$(TEST_GEN_MODS_DIR)" != "X" ]; then \ + rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(TEST_GEN_MODS_DIR) $(OUTPUT); \ fi @if [ "X$(TEST_PROGS)" != "X" ]; then \ $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \ @@ -118,6 +121,12 @@ else @$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS)) endif +gen_mods_dir: + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) + +clean_mods_dir: + $(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean + define INSTALL_SINGLE_RULE $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)) $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/) @@ -131,6 +140,7 @@ define INSTALL_RULE $(eval INSTALL_LIST = $(TEST_CUSTOM_PROGS)) $(INSTALL_SINGLE_RULE) $(eval INSTALL_LIST = $(TEST_GEN_PROGS_EXTENDED)) $(INSTALL_SINGLE_RULE) $(eval INSTALL_LIST = $(TEST_GEN_FILES)) $(INSTALL_SINGLE_RULE) + $(eval INSTALL_LIST = $(TEST_GEN_MODS_DIR)) $(INSTALL_SINGLE_RULE) $(eval INSTALL_LIST = $(wildcard config settings)) $(INSTALL_SINGLE_RULE) endef @@ -156,7 +166,7 @@ define CLEAN $(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN) endef -clean: +clean: $(if $(TEST_GEN_MODS_DIR),clean_mods_dir) $(CLEAN) # Enables to extend CFLAGS and LDFLAGS from command line, e.g. @@ -187,4 +197,4 @@ $(OUTPUT)/%:%.S $(LINK.S) $^ $(LDLIBS) -o $@ endif -.PHONY: run_tests all clean install emit_tests +.PHONY: run_tests all clean install emit_tests gen_mods_dir clean_mods_dir
Add TEST_GEN_MODS_DIR variable for kselftests. It can point to a directory containing kernel modules that will be used by selftest scripts. The modules are built as external modules for the running kernel. As a result they are always binary compatible and the same tests can be used for older or newer kernels. The build requires "kernel-devel" package to be installed. For example, in the upstream sources, the rpm devel package is produced by "make rpm-pkg" The modules can be built independently by make -C tools/testing/selftests/livepatch/ or they will be automatically built before running the tests via make -C tools/testing/selftests/livepatch/ run_tests Note that they are _not_ built when running the standalone tests by calling, for example, ./test-state.sh. Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- Documentation/dev-tools/kselftest.rst | 4 ++++ tools/testing/selftests/lib.mk | 20 +++++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-)