From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A06C3C07E9D for ; Fri, 23 Sep 2022 22:35:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D27C480028; Fri, 23 Sep 2022 18:35:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CD70C80016; Fri, 23 Sep 2022 18:35:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B9ED380028; Fri, 23 Sep 2022 18:35:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id AB32680016 for ; Fri, 23 Sep 2022 18:35:51 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 7EF1B1A0739 for ; Fri, 23 Sep 2022 22:35:51 +0000 (UTC) X-FDA: 79944808902.02.2D1B0DE Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf01.hostedemail.com (Postfix) with ESMTP id 575D240004 for ; Fri, 23 Sep 2022 22:35:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1663972548; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=J+2c5YW+EA7plWyqyC2+l7gyp9odW6tPIthlfYxVMK0=; b=Fky66IgpBEzZ0l5b5ueeqma2/xy4U7NpFdk7AyQGuhvLp2bKyRA+1uqxfvoQ78VxDqoGDr PGOUqDw1+hLq1Gh6S6wlCGeEofH7oZNXK/qN07WEA9hjTbr7uWEMYBrdvBh2Mm9nefxo0k fDLamrXNnhVVhRu1nfg6BFJe8VMrHE0= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-421-Ip1b1j8lNEKnnhWrJO_xig-1; Fri, 23 Sep 2022 18:35:47 -0400 X-MC-Unique: Ip1b1j8lNEKnnhWrJO_xig-1 Received: by mail-ed1-f71.google.com with SMTP id dz21-20020a0564021d5500b0045217702048so913380edb.5 for ; Fri, 23 Sep 2022 15:35:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=J+2c5YW+EA7plWyqyC2+l7gyp9odW6tPIthlfYxVMK0=; b=RfLls5awhox7JQS0DHUvHI5fBa9+3pta8gPeqoEUQ5uCw/Uq0N1KmTG0GiSD+Yusns cy2WOGKAMmi2Mn92a3ed4njBuPsoLETvfoG9lOWC6wqGbLx7bQgv55VfHkdY5oJDEvB8 hJ/3ydmVVIcirgKvfGiems7HAKCow2KX3QzZO0L8K40YhxZTwK3w0s/RBPKqV5n5erEw JZpjeRmO50uzagQr/XqTCDcGWr+pflgpzFqxLiU4YMjGTs/U2Etr3sbnYbS7vVNQLX2s WHwuI3vsLhzE4V6AYnPcrgsqma7XCki89z17Fmev1qKdz3yWxOCYi05jYrwOeE75lC8V j6Ow== X-Gm-Message-State: ACrzQf3aIq7vhtvI1j083FzPlBxyrPHvG1qKJ1evEORhVNddq1WuFn74 CDNFysbXGxwknosaxTSVNJdLTDhn6VKVQSnSfYjwv2ewBJLfIO2tm/M3C6iT7h/z5JsmOb0u8It nKGkIfLoP5jCi2W7p9/EsiZ9ReME= X-Received: by 2002:a05:6402:43cf:b0:450:eae1:c9d3 with SMTP id p15-20020a05640243cf00b00450eae1c9d3mr10723246edc.231.1663972546225; Fri, 23 Sep 2022 15:35:46 -0700 (PDT) X-Google-Smtp-Source: AMsMyM61hUVlIihf1mB+9peSUfCPpl2lY1axiVlb+Hc695DbpTg/y1XovHLdPOCHpv2yeQfoafdsaHoX9kl8fnmcvjE= X-Received: by 2002:a05:6402:43cf:b0:450:eae1:c9d3 with SMTP id p15-20020a05640243cf00b00450eae1c9d3mr10723224edc.231.1663972545995; Fri, 23 Sep 2022 15:35:45 -0700 (PDT) MIME-Version: 1.0 References: <20220420084036.4101604-1-usama.anjum@collabora.com> <01f64e01-580e-abca-2b86-aa586d987bf4@redhat.com> In-Reply-To: From: Nico Pache Date: Fri, 23 Sep 2022 16:35:19 -0600 Message-ID: Subject: Re: [PATCH v6 1/2] selftests: vm: bring common functions to a new file To: Muhammad Usama Anjum Cc: Andrew Morton , Shuah Khan , kernel@collabora.com, krisman@collabora.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, Joel Savitz X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1663972551; a=rsa-sha256; cv=none; b=MGA0UARbyymR5Xc2/Y94A1dAIZ1ZV/6HRpxhXe4pWJmogQXUljSOLouPI5LjnGOnlz4iAi oiIwrQMy9nI+ZsM/5LcetDhdvIG1fGftI6hurIlnXfr5/QVMEQqmyRFPeghX4dvTxoonpK zxiCFpRpZi6AFlwTp5M9EZCy7y6mlhc= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Fky66Igp; spf=pass (imf01.hostedemail.com: domain of npache@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=npache@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1663972551; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=J+2c5YW+EA7plWyqyC2+l7gyp9odW6tPIthlfYxVMK0=; b=COc/VxeifLT+3v1BCafM/NvXglE4gEE4uBW95SuEfOLXwNE5VEsc4XLgAnRrNNp4W1uBlq 7UGN8xIT4YrcixqMKkNCk8Ydrc+KfRZJhCaVn2riaWNle6eexM0j/Asrt3jrvfVjRG6EgF Z4wsqZOMlo3Fr7gDpB8ljpt4qDLR52c= X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 575D240004 Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Fky66Igp; spf=pass (imf01.hostedemail.com: domain of npache@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=npache@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspam-User: X-Stat-Signature: hdm3t51dswoe13ifdsoacbtd6tcjfgkd X-HE-Tag: 1663972549-959246 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Sep 21, 2022 at 5:06 AM Muhammad Usama Anjum 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 > >> --- > >> 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 >