On Wed, Sep 21, 2022 at 5:06 AM Muhammad Usama Anjum firstname.lastname@example.org wrote:
On 9/9/22 8:06 AM, Nico Pache wrote:
On 4/20/22 04:40, Muhammad Usama Anjum wrote:
Bring common functions to a new file while keeping code as much same as possible. These functions can be used in the new tests. This helps in avoiding code duplication.
This commit breaks a pattern in the way tests are run in the current scheme. Before this commit the only executable (or TEST_PROGS) that was executed was run_vmselftests.sh. Now both madv_populate and split_huge_page_test are being run as well.
Signed-off-by: Muhammad Usama Anjum email@example.com
Changes in V6:
- Correct header files inclusion
Changes in V5: Keep moved code as same as possible
- Updated macros names
- Removed macro used to show bit number of dirty bit, added a comment instead
- Corrected indentation
tools/testing/selftests/vm/Makefile | 7 +- tools/testing/selftests/vm/madv_populate.c | 34 +----- .../selftests/vm/split_huge_page_test.c | 79 +------------ tools/testing/selftests/vm/vm_util.c | 108 ++++++++++++++++++ tools/testing/selftests/vm/vm_util.h | 9 ++ 5 files changed, 124 insertions(+), 113 deletions(-) create mode 100644 tools/testing/selftests/vm/vm_util.c create mode 100644 tools/testing/selftests/vm/vm_util.h
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile index 5e43f072f5b76..4e68edb26d6b6 100644 --- a/tools/testing/selftests/vm/Makefile +++ b/tools/testing/selftests/vm/Makefile @@ -34,7 +34,7 @@ TEST_GEN_FILES += hugepage-mremap TEST_GEN_FILES += hugepage-shm TEST_GEN_FILES += hugepage-vmemmap TEST_GEN_FILES += khugepaged -TEST_GEN_FILES += madv_populate +TEST_GEN_PROGS = madv_populate
madv_populate is already being run in run_vmselftests.sh
TEST_GEN_FILES += map_fixed_noreplace TEST_GEN_FILES += map_hugetlb TEST_GEN_FILES += map_populate @@ -47,7 +47,7 @@ TEST_GEN_FILES += on-fault-limit TEST_GEN_FILES += thuge-gen TEST_GEN_FILES += transhuge-stress TEST_GEN_FILES += userfaultfd -TEST_GEN_FILES += split_huge_page_test +TEST_GEN_PROGS += split_huge_page_test TEST_GEN_FILES += ksm_tests
ifeq ($(MACHINE),x86_64) @@ -91,6 +91,9 @@ TEST_FILES := test_vmalloc.sh KSFT_KHDR_INSTALL := 1 include ../lib.mk
+$(OUTPUT)/madv_populate: vm_util.c +$(OUTPUT)/split_huge_page_test: vm_util.c
Not sure what this does but if we add a run entry for split_huge_page_test in run_vmselftests.sh we wont really need this patch.
I'm not sure the reduction in code size is worth the change in run behavior.
Unless I'm missing something I suggest we revert this patch and add a run entry for split_huge_page_test in run_vmselftests.sh. I can do this if no one objects.
The run behavior isn't being changed here. Only the build behavior is being changed as we want to keep the common code in one file. You can add the run entry as required. I don't know why do you think the Makefile has changed the run behavior.
Before your commit running `make TARGETS=vm; make TARGETS=vm run_tests` had the following run behavior: - The only thing executed via the run_tests wrapper is run_vmtests.sh - TAP output is 1/1: TAP version 13 1..1 # selftests: vm: run_vmtests.sh # arm64 ia64 mips64 parisc64 ppc64 ppc64le riscv64 s390x sh64 sparc64 x86_64 # ----------------------- # running ./hugepage-mmap # ----------------------- ....
After your commit: - Multiple executables (madv_populate, soft-dirty, split_huge_page_test, run_vmtests.sh) are defined and ran: # selftests: vm: madv_populate not ok 1 selftests: vm: madv_populate # exit=1 # selftests: vm: soft-dirty ok 2 selftests: vm: soft-dirty # selftests: vm: split_huge_page_test ok 3 selftests: vm: split_huge_page_test # selftests: vm: run_vmtests.sh ok 4 selftests: vm: run_vmtests.sh # SKIP
I'm not saying utilizing the TEST_GEN_PROG variable is incorrect, I'm just pointing out that we have a sudden change in run behavior via the run_test wrapper. I personally think the TEST_GEN_PROG output is cleaner, but having two different ways of outputting results may be harder/confusing to parser. Additionally there is still the issue that madv_populate is being run twice for no reason.
Cheers, -- Nico
Cheers, -- Nico
-- Muhammad Usama Anjum