[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f3f882bf-4120-4daa-b35f-8b1b4e0deb2d@huawei.com>
Date: Wed, 9 Oct 2024 11:59:47 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Shuah Khan <skhan@...uxfoundation.org>, <davem@...emloft.net>,
<kuba@...nel.org>, <pabeni@...hat.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Alexander Duyck
<alexander.duyck@...il.com>, Alexander Duyck <alexanderduyck@...com>, Andrew
Morton <akpm@...ux-foundation.org>, Shuah Khan <shuah@...nel.org>,
<linux-mm@...ck.org>, <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH net-next v20 01/14] mm: page_frag: add a test module for
page_frag
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@...il.com>
>> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
>> Reviewed-by: Alexander Duyck <alexanderduyck@...com>
>
> 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@huawei.com/
>
>> ---
>> 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.
Powered by blists - more mailing lists