linux-kernel - Re: [PATCH v6 1/2] selftests: vm: bring common functions to a new file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c3df7c71-79c1-7876-e39a-15cffc444da3@collabora.com>
Date:   Wed, 21 Sep 2022 16:06:03 +0500
From:   Muhammad Usama Anjum <usama.anjum@...labora.com>
To:     Nico Pache <npache@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Shuah Khan <shuah@...nel.org>
Cc:     usama.anjum@...labora.com, kernel@...labora.com,
        krisman@...labora.com, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
        Joel Savitz <jsavitz@...hat.com>
Subject: Re: [PATCH v6 1/2] selftests: vm: bring common functions to a new
 file

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 <usama.anjum@...labora.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.

> 
> Cheers,
> -- Nico
> 

-- 
Muhammad Usama Anjum