On 2024/10/9 3:56, Shuah Khan wrote: > On 10/8/24 05:20, Yunsheng Lin wrote: >> The testing is done by ensuring that the fragment allocated >> from a frag_frag_cache instance is pushed into a ptr_ring >> instance in a kthread binded to a specified cpu, and a kthread >> binded to a specified cpu will pop the fragment from the >> ptr_ring and free the fragment. >> >> CC: Alexander Duyck <alexander.duyck@xxxxxxxxx> >> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx> >> Reviewed-by: Alexander Duyck <alexanderduyck@xxxxxx> > > Signed-off-by should be last. Same comment on all the other Hi, Shuah I used 'git am' to collect those tag, it seems that is the order the tool applied, and I checking other applied commit, it seems only Signed-off-by from the committer is the last, like the below recent mm commit: 6901cf55de22 ff7f5ad7bce4 > patches in this series. When you have 4 patches, it is a good > practice to add cover-letter. I guess the cover-letter meant below? https://lore.kernel.org/all/20241008112049.2279307-1-linyunsheng@xxxxxxxxxx/ > >> --- >> tools/testing/selftests/mm/Makefile | 3 + >> tools/testing/selftests/mm/page_frag/Makefile | 18 ++ >> .../selftests/mm/page_frag/page_frag_test.c | 173 ++++++++++++++++++ >> tools/testing/selftests/mm/run_vmtests.sh | 8 + >> tools/testing/selftests/mm/test_page_frag.sh | 171 +++++++++++++++++ >> 5 files changed, 373 insertions(+) >> create mode 100644 tools/testing/selftests/mm/page_frag/Makefile >> create mode 100644 tools/testing/selftests/mm/page_frag/page_frag_test.c >> create mode 100755 tools/testing/selftests/mm/test_page_frag.sh >> >> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile >> index 02e1204971b0..acec529baaca 100644 >> --- a/tools/testing/selftests/mm/Makefile >> +++ b/tools/testing/selftests/mm/Makefile >> @@ -36,6 +36,8 @@ MAKEFLAGS += --no-builtin-rules >> CFLAGS = -Wall -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES) >> LDLIBS = -lrt -lpthread -lm >> +TEST_GEN_MODS_DIR := page_frag >> + >> TEST_GEN_FILES = cow >> TEST_GEN_FILES += compaction_test >> TEST_GEN_FILES += gup_longterm >> @@ -126,6 +128,7 @@ TEST_FILES += test_hmm.sh >> TEST_FILES += va_high_addr_switch.sh >> TEST_FILES += charge_reserved_hugetlb.sh >> TEST_FILES += hugetlb_reparenting_test.sh >> +TEST_FILES += test_page_frag.sh >> # required by charge_reserved_hugetlb.sh >> TEST_FILES += write_hugetlb_memory.sh >> diff --git a/tools/testing/selftests/mm/page_frag/Makefile b/tools/testing/selftests/mm/page_frag/Makefile >> new file mode 100644 >> index 000000000000..58dda74d50a3 >> --- /dev/null >> +++ b/tools/testing/selftests/mm/page_frag/Makefile >> @@ -0,0 +1,18 @@ >> +PAGE_FRAG_TEST_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST))))) >> +KDIR ?= $(abspath $(PAGE_FRAG_TEST_DIR)/../../../../..) >> + >> +ifeq ($(V),1) >> +Q = >> +else >> +Q = @ >> +endif >> + >> +MODULES = page_frag_test.ko >> + >> +obj-m += page_frag_test.o >> + >> +all: >> + +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) modules >> + >> +clean: >> + +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) clean >> diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c >> new file mode 100644 >> index 000000000000..eeb2b6bc681a >> --- /dev/null >> +++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c >> @@ -0,0 +1,173 @@ >> +// SPDX-License-Identifier: GPL-2.0 > > I think this would throw a checkpatch warning about > comment should be "/*" and not "//" using "git grep 'SPDX-License' mm", "//" seems like a more common case. And I did using './scripts/checkpatch.pl --strict --codespell', and it does not throw a checkpatch warning. >> + >> +/* >> + * Test module for page_frag cache >> + * ... >> +function run_manual_check() >> +{ >> + # >> + # Validate passed parameters. If there is wrong one, >> + # the script exists and does not execute further. >> + # >> + validate_passed_args $@ >> + >> + echo "Run the test with following parameters: $@" > > Is this marker good enough to isolate the test results in the > dmesg? Include the test name in the message. > > >> + insmod $DRIVER $@ > /dev/null 2>&1 >> + echo "Done." > > Is this marker good enough to isolate the test results in the > dmesg? Include the test name in the message. > >> + echo "Check the kernel ring buffer to see the summary." > > Usually the test would run dmesg and filter out the test results > from the dmesg and include them in the test script output. > > You can refer to other tests that do that: powerpc/scripts/hmi.sh > is one example. Thanks, will check that.
