[PATCH v2 0/4] Improvements to livepatch kselftests on top of kselftest-next

[PATCH v2 0/4] Improvements to livepatch kselftests on top of kselftest-next

[Marcos Paulo de Souza]

The changes on lib.mk are both for simplification and also
clarification, like in the case of not handling TEST_GEN_MODS_DIR
directly. There is a new patch to solve one issue reported by build bot.

These changes apply on top of the current kselftest-next branch. Please
review!

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Changes in v2:
- Added a new patch to avoid building the modules/running the tests if
  kernel-devel is not installed. Resolving an issue reported by the
  build bot.
- Reordered the patches, showing the more simple ones first. Besides
  patch 0002, all the other three didn't changed since v1.
- Link to v1: https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-0-89f4a6f5cddc@suse.com

---
Marcos Paulo de Souza (4):
      selftests: livepatch: Add initial .gitignore
      selftests: livepatch: Avoid running the tests if kernel-devel is missing
      selftests: lib.mk: Do not process TEST_GEN_MODS_DIR
      selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling

 tools/testing/selftests/lib.mk                        | 19 +++++++------------
 tools/testing/selftests/livepatch/.gitignore          |  1 +
 tools/testing/selftests/livepatch/functions.sh        | 13 +++++++++++++
 .../testing/selftests/livepatch/test_modules/Makefile |  6 ++++++
 4 files changed, 27 insertions(+), 12 deletions(-)
---
base-commit: 6f1a214d446b2f2f9c8c4b96755a8f0316ba4436
change-id: 20240215-lp-selftests-fixes-7d4bab3c0712

Best regards,
-- 
Marcos Paulo de Souza <mpdesouza@suse.com>

[PATCH v2 1/4] selftests: livepatch: Add initial .gitignore

[Marcos Paulo de Souza]

Ignore the binary used to test livepatching a syscall.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/livepatch/.gitignore b/tools/testing/selftests/livepatch/.gitignore
new file mode 100644
index 000000000000..f1e9c2a20e99
--- /dev/null
+++ b/tools/testing/selftests/livepatch/.gitignore
@@ -0,0 +1 @@
+test_klp-call_getpid

-- 
2.42.1

[PATCH v2 2/4] selftests: livepatch: Avoid running the tests if kernel-devel is missing

[Marcos Paulo de Souza]

By checking if KDIR is a valid directory we can safely skip the tests if
kernel-devel isn't installed (default value of KDIR), or if KDIR
variable passed doesn't exists.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202402191417.XULH88Ct-lkp@intel.com/
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/livepatch/functions.sh          | 13 +++++++++++++
 tools/testing/selftests/livepatch/test_modules/Makefile |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/tools/testing/selftests/livepatch/functions.sh b/tools/testing/selftests/livepatch/functions.sh
index e60cf09491a6..f0ed148d3c51 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -34,6 +34,18 @@ function is_root() {
 	fi
 }
 
+# Check if we can compile the modules before loading them
+function has_kdir() {
+	if [ -z "$KDIR" ]; then
+		KDIR="/lib/modules/$(uname -r)/build"
+	fi
+
+	if [ ! -d "$KDIR" ]; then
+		echo "skip all tests: KDIR ($KDIR) not available to compile modules."
+		exit $ksft_skip
+	fi
+}
+
 # die(msg) - game over, man
 #	msg - dying words
 function die() {
@@ -108,6 +120,7 @@ function cleanup() {
 #		 the ftrace_enabled sysctl.
 function setup_config() {
 	is_root
+	has_kdir
 	push_config
 	set_dynamic_debug
 	set_ftrace_enabled 1
diff --git a/tools/testing/selftests/livepatch/test_modules/Makefile b/tools/testing/selftests/livepatch/test_modules/Makefile
index f5e880269bff..e6e638c4bcba 100644
--- a/tools/testing/selftests/livepatch/test_modules/Makefile
+++ b/tools/testing/selftests/livepatch/test_modules/Makefile
@@ -13,8 +13,14 @@ obj-m += test_klp_atomic_replace.o \
 	test_klp_shadow_vars.o \
 	test_klp_syscall.o
 
+# Ensure that KDIR exists, otherwise skip the compilation
 modules:
+ifneq ("$(wildcard $(KDIR))", "")
 	$(Q)$(MAKE) -C $(KDIR) modules KBUILD_EXTMOD=$(TESTMODS_DIR)
+endif
 
+# Ensure that KDIR exists, otherwise skip the clean target
 clean:
+ifneq ("$(wildcard $(KDIR))", "")
 	$(Q)$(MAKE) -C $(KDIR) clean KBUILD_EXTMOD=$(TESTMODS_DIR)
+endif

-- 
2.42.1

[PATCH v2 3/4] selftests: lib.mk: Do not process TEST_GEN_MODS_DIR

[Marcos Paulo de Souza]

The directory itself doesn't need have path handling, since it's only to
mean where is the directory that contains modules to be built.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/lib.mk | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 0d8b7db92715..286ce0ee102b 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -54,12 +54,9 @@ endif
 # 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: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) \
 	$(if $(TEST_GEN_MODS_DIR),gen_mods_dir)

-- 
2.42.1

[PATCH v2 4/4] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling

[Marcos Paulo de Souza]

We don't need new targets only to run two make modules and make clean.
We can test if TEST_GEN_MODS_DIR is specified, and then run the
commands.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 tools/testing/selftests/lib.mk | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 286ce0ee102b..eddcd4a849dc 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -58,8 +58,9 @@ 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) \
-	$(if $(TEST_GEN_MODS_DIR),gen_mods_dir)
+all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
+	$(if $(TEST_GEN_MODS_DIR), \
+		$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR))
 
 define RUN_TESTS
 	BASE_DIR="$(selfdir)";			\
@@ -85,11 +86,6 @@ 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))
@@ -133,9 +129,11 @@ endif
 
 define CLEAN
 	$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN)
+	$(if $(TEST_GEN_MODS_DIR), \
+		$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean)
 endef
 
-clean: $(if $(TEST_GEN_MODS_DIR),clean_mods_dir)
+clean:
 	$(CLEAN)
 
 # Enables to extend CFLAGS and LDFLAGS from command line, e.g.
@@ -166,4 +164,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

-- 
2.42.1

Re: [PATCH v2 4/4] selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling

[Marcos Paulo de Souza]

On Wed, 21 Feb 2024 14:17:06 -0300 Marcos Paulo de Souza <mpdesouza@suse.com> wrote:

Shuah, please skip this one. The simplification is not worthy if it causes a new
warning. This reply[1] explains what's going on.

Thanks in advance,
  Marcos

[1]: https://lore.kernel.org/linux-kselftest/20240223161244.17709-1-mpdesouza@suse.com/

> We don't need new targets only to run two make modules and make clean.
> We can test if TEST_GEN_MODS_DIR is specified, and then run the
> commands.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  tools/testing/selftests/lib.mk | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 286ce0ee102b..eddcd4a849dc 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -58,8 +58,9 @@ 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) \
> -	$(if $(TEST_GEN_MODS_DIR),gen_mods_dir)
> +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
> +	$(if $(TEST_GEN_MODS_DIR), \
> +		$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR))
>  
>  define RUN_TESTS
>  	BASE_DIR="$(selfdir)";			\
> @@ -85,11 +86,6 @@ 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))
> @@ -133,9 +129,11 @@ endif
>  
>  define CLEAN
>  	$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN)
> +	$(if $(TEST_GEN_MODS_DIR), \
> +		$(Q)$(MAKE) -C $(TEST_GEN_MODS_DIR) clean)
>  endef
>  
> -clean: $(if $(TEST_GEN_MODS_DIR),clean_mods_dir)
> +clean:
>  	$(CLEAN)
>  
>  # Enables to extend CFLAGS and LDFLAGS from command line, e.g.
> @@ -166,4 +164,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
> 
> -- 
> 2.42.1

Re: [PATCH v2 0/4] Improvements to livepatch kselftests on top of kselftest-next

[Shuah Khan]

On 2/21/24 10:17, Marcos Paulo de Souza wrote:
> The changes on lib.mk are both for simplification and also
> clarification, like in the case of not handling TEST_GEN_MODS_DIR
> directly. There is a new patch to solve one issue reported by build bot.
> 
> These changes apply on top of the current kselftest-next branch. Please
> review!
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> Changes in v2:
> - Added a new patch to avoid building the modules/running the tests if
>    kernel-devel is not installed. Resolving an issue reported by the
>    build bot.
> - Reordered the patches, showing the more simple ones first. Besides
>    patch 0002, all the other three didn't changed since v1.
> - Link to v1: https://lore.kernel.org/r/20240215-lp-selftests-fixes-v1-0-89f4a6f5cddc@suse.com
> 
> ---
> Marcos Paulo de Souza (4):
>        selftests: livepatch: Add initial .gitignore
>        selftests: livepatch: Avoid running the tests if kernel-devel is missing
>        selftests: lib.mk: Do not process TEST_GEN_MODS_DIR
>        selftests: lib.mk: Simplify TEST_GEN_MODS_DIR handling
> 
>   tools/testing/selftests/lib.mk                        | 19 +++++++------------
>   tools/testing/selftests/livepatch/.gitignore          |  1 +
>   tools/testing/selftests/livepatch/functions.sh        | 13 +++++++++++++
>   .../testing/selftests/livepatch/test_modules/Makefile |  6 ++++++
>   4 files changed, 27 insertions(+), 12 deletions(-)
> ---
> base-commit: 6f1a214d446b2f2f9c8c4b96755a8f0316ba4436
> change-id: 20240215-lp-selftests-fixes-7d4bab3c0712
> 
> Best regards,

Applied all except the last patch to linux-kelftest next for Linux 6.9-rc1

thanks,
-- Shuah