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 E9F1FC4167B for ; Mon, 27 Nov 2023 10:48:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3920F6B02F2; Mon, 27 Nov 2023 05:48:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 31ADF6B02F3; Mon, 27 Nov 2023 05:48:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1E2B06B02F4; Mon, 27 Nov 2023 05:48:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 088216B02F2 for ; Mon, 27 Nov 2023 05:48:21 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id CA3ED40198 for ; Mon, 27 Nov 2023 10:48:20 +0000 (UTC) X-FDA: 81503409960.13.01F1AE1 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf14.hostedemail.com (Postfix) with ESMTP id D16AB10000A for ; Mon, 27 Nov 2023 10:48:18 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf14.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701082098; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LnQw2Hp1RPlkei6LzW2GcC2P148roXk5Os8DzrfLi1E=; b=TKJYiCG7aYzH4r6xthpqA6Z3W1FEy5D+jLM5ew2QmlR3BR/Fbc6qg4c/lbXx37yBOu+T+7 RP5AbKhNXuH5O0G4Vi92dZW/nzhUz0ngzdmuI4nz1B3e+3BnVTgV/RnGmCj71LpZHA4Tzv C7Vy8M4XuyCJeLWbglSBiIs+S4d+bH8= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf14.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701082099; a=rsa-sha256; cv=none; b=ljRy2+/EzmTyBSTKamIqkAXSZ2OhJTwsltcEp/3ZHsschty7HIWn7cPbpQXEbFccqqPlkk 6K4WVa+SKPzxHRNMONCzBiMNA5V1DahPowrMnbXXopQ4RVY651AUbKI0CEkoPsGB4zLiRs yvmyqQjVcNiDqcgRfhiTtvxHlEoQ0sw= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 630E3C15; Mon, 27 Nov 2023 02:49:05 -0800 (PST) Received: from [10.57.73.191] (unknown [10.57.73.191]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 240883F73F; Mon, 27 Nov 2023 02:48:15 -0800 (PST) Message-ID: <15c288aa-feab-4d3a-af33-b87481eaffe3@arm.com> Date: Mon, 27 Nov 2023 10:48:13 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RESEND PATCH v7 09/10] selftests/mm/cow: Generalize do_run_with_thp() helper Content-Language: en-GB To: David Hildenbrand , Andrew Morton , Matthew Wilcox , Yin Fengwei , Yu Zhao , Catalin Marinas , Anshuman Khandual , Yang Shi , "Huang, Ying" , Zi Yan , Luis Chamberlain , Itaru Kitayama , "Kirill A. Shutemov" , John Hubbard , David Rientjes , Vlastimil Babka , Hugh Dickins , Kefeng Wang Cc: linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20231122162950.3854897-1-ryan.roberts@arm.com> <20231122162950.3854897-10-ryan.roberts@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: wunb5yrx9af4buo9w33jwj65prie9xh5 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: D16AB10000A X-HE-Tag: 1701082098-649182 X-HE-Meta: U2FsdGVkX1/qm5HfrzzRZc+SWXVpNNVVFXY9tEajnhUa+dNLTjOzbmWACYeLg3uSm74KUaIWP+DXrj9viMY8+8GQbWKXpoxhFdy8lmQjz1cSxb8Tk5nFyhxpZxVLD9xXcLFcQA9vyesYKG97OE+++7GExaknKm7WKB9JQd+9CJwLmWXBXeKS1O0KnYiR5rHiQehP60H2+zZns66t8ZVHMWxb6GvL4O9j7klxzRbjjw1Q58kNxXlfewktA9/Rj7UkcRlZjL8qmejYgr/fDrNEEX8TeejlV4BZ+/31YzYqDXYrZJs/qeLfsn6sbFhUBl7ZZXGCbLG3IXdwHJMASNbcDcdvuucRMCE4UJ7Ib06/t6oOJnbrF3ybELB+JCpbpmsVMzYj9wLt3BrjSlb404fNirYuzbieMU2Jkax0PfOcTo1QZX88/jcT/tmkA49VrP+kmlsQ2dyShLAUeQpJ7NGacgCNKj7+aopJ+qNIDaWZ6qvGUwFQDzZ/jrGxQgY8cMq/wSETXIoGJvYHesGrjU4rudjA+b3ENikwUrIfPaHeyj5VWQVnxILJSIZAueE+OWVIL0qV9XrVjZKtpAmzKKowno41WuuBWeMB6cjf5+EuKwYF3epW7PYvLc5XCgDNTZUiy/svNdZrJ717VdpK5Klr42ZYjzC4orRgpqMcCH0jjVqgq/49BaAfbRln9Qprytrt804IhEdykXg4OzTfb9sJBMSGyjrOH6hDuucS+khBsDe1tJOPbi7yxZ2gnLEQrjXjAxCz5E4IOmLq6xpkiM+8mpPPSdkj3UQ0dCFmucPZDJU8tqbs4f/VNBAEQMe6+hq4TB4pyCHi81T+Etyr0kPn6fwm1Vz0t0hUIAf94YamVGTECCz2JAA5SJzNXfY2rOf82V7VORRhZch1xB/dd9gKU35wop6wtw2mcpemPpk+RYAzO+ufQjVSn6UGnYu/Ifq/WHCU9IzlG6UING+aOEr 3tG3CPQM YzcjtpuxcecGXPGVMTHkEMz7UR+7Z63+9feaKvpzIuwfa3/iux/wJ5LlIklcivtxNKx/8hlGRFaU4UU12Y6BA77iY6AIRPKZTn9Kw29I6u6fDU0imN/BFovITs5m7c8xTR6S4UkZuVg1wL5tKA2aYlpO81xU2NTCZk3cQCv2VpQO+vS2Fgjv7WmJoUVzs8oPqzuEtIgMo55/p8ZrO8D5CQ0lc6q220ZEdrxAVj41nMYvdxKWly1rqeSHdSv39A4mnvrIpdAJOv1J0JVhsFhpcbXl0Wl4p0VNWInjmsKTGUT4X1QXuoh0Ufy3dDB5ZNzm7VnxfV1KVYNhaJd0= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 24/11/2023 17:48, David Hildenbrand wrote: > On 22.11.23 17:29, Ryan Roberts wrote: >> do_run_with_thp() prepares (PMD-sized) THP memory into different states >> before running tests. With the introduction of small-sized THP, we would >> like to reuse this logic to also test those smaller THP sizes. So let's >> add a size parameter which tells the function what size THP it should >> operate on. >> >> A separate commit will utilize this change to add new tests for >> small-sized THP, where available. >> >> Signed-off-by: Ryan Roberts >> --- >>   tools/testing/selftests/mm/cow.c | 146 +++++++++++++++++-------------- >>   1 file changed, 79 insertions(+), 67 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c >> index 7324ce5363c0..d03c453cfd5c 100644 >> --- a/tools/testing/selftests/mm/cow.c >> +++ b/tools/testing/selftests/mm/cow.c >> @@ -32,7 +32,7 @@ >> >>   static size_t pagesize; >>   static int pagemap_fd; >> -static size_t thpsize; >> +static size_t pmdsize; >>   static int nr_hugetlbsizes; >>   static size_t hugetlbsizes[10]; >>   static int gup_fd; >> @@ -734,14 +734,14 @@ enum thp_run { >>       THP_RUN_PARTIAL_SHARED, >>   }; >> >> -static void do_run_with_thp(test_fn fn, enum thp_run thp_run) >> +static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t size) > > Nit: can we still call it "thpsize" in this function? That makes it clearer IMHO > and avoids most renaming. Yep no problem. Will fix in next version. > >>   { >>       char *mem, *mmap_mem, *tmp, *mremap_mem = MAP_FAILED; >> -    size_t size, mmap_size, mremap_size; >> +    size_t mmap_size, mremap_size; >>       int ret; >> >> -    /* For alignment purposes, we need twice the thp size. */ >> -    mmap_size = 2 * thpsize; >> +    /* For alignment purposes, we need twice the requested size. */ >> +    mmap_size = 2 * size; >>       mmap_mem = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, >>               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >>       if (mmap_mem == MAP_FAILED) { >> @@ -749,36 +749,40 @@ static void do_run_with_thp(test_fn fn, enum thp_run >> thp_run) >>           return; >>       } >> >> -    /* We need a THP-aligned memory area. */ >> -    mem = (char *)(((uintptr_t)mmap_mem + thpsize) & ~(thpsize - 1)); >> +    /* We need to naturally align the memory area. */ >> +    mem = (char *)(((uintptr_t)mmap_mem + size) & ~(size - 1)); >> >> -    ret = madvise(mem, thpsize, MADV_HUGEPAGE); >> +    ret = madvise(mem, size, MADV_HUGEPAGE); >>       if (ret) { >>           ksft_test_result_fail("MADV_HUGEPAGE failed\n"); >>           goto munmap; >>       } >> >>       /* >> -     * Try to populate a THP. Touch the first sub-page and test if we get >> -     * another sub-page populated automatically. >> +     * Try to populate a THP. Touch the first sub-page and test if >> +     * we get the last sub-page populated automatically. >>        */ >>       mem[0] = 0; >> -    if (!pagemap_is_populated(pagemap_fd, mem + pagesize)) { >> +    if (!pagemap_is_populated(pagemap_fd, mem + size - pagesize)) { >>           ksft_test_result_skip("Did not get a THP populated\n"); >>           goto munmap; >>       } > > Yes! I have a patch lying around here that does that same. :) > > I guess there is no need to set MADV_NOHUGEPAGE on the remainder of the mmap'ed > are: > > Assume we want a 64KiB thp. We mmap'ed 128KiB. If we get a reasonably aligned > area, we might populate a 128KiB THP. > > But I assume the MADV_HUGEPAGE will in all configurations properly create a > separate 64KiB VMA and we'll never get 128 KiB populated. So this should work > reliably. Yes agreed. And also, we explicitly only enable a single THP size at a time so should only allocate a THP of the expected size. Perhaps we should mark the whole mmap area with MADV_HUGEPAGE since that will serve as a test that we only get the smaller size we configured? > >> -    memset(mem, 0, thpsize); >> +    memset(mem, 0, size); >> >> -    size = thpsize; >>       switch (thp_run) { >>       case THP_RUN_PMD: >>       case THP_RUN_PMD_SWAPOUT: >> +        if (size != pmdsize) { >> +            ksft_test_result_fail("test bug: can't PMD-map size\n"); >> +            goto munmap; >> +        } > > Maybe rather "assert()" because that's a real BUG in the test? Yep will do. > > [...] > >> +    pmdsize = read_pmd_pagesize(); >> +    if (pmdsize) >> +        ksft_print_msg("[INFO] detected PMD-mapped THP size: %zu KiB\n", > > Maybe simply: "detected PMD size". Zes, we read it via the THP interface, but > that shouldn't matter much. Err, just want to clarify what you are suggesting. With the current patch you will see something like: [INFO] detected PMD-mapped THP size: 2048 KiB [INFO] detected small-sized THP size: 64 KiB [INFO] detected small-sized THP size: 128 KiB ... [INFO] detected small-sized THP size: 1024 KiB Are you suggesting something like this: [INFO] detected PMD size: 2048 KiB [INFO] detected THP size: 64 KiB [INFO] detected THP size: 128 KiB ... [INFO] detected THP size: 2048 KiB >