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 B3F28C77B7C for ; Wed, 25 Jun 2025 04:28:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CDF516B0089; Wed, 25 Jun 2025 00:28:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CB7A56B0092; Wed, 25 Jun 2025 00:28:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BF3F56B00AF; Wed, 25 Jun 2025 00:28:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id A9CA06B0089 for ; Wed, 25 Jun 2025 00:28:40 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B79D91A0B05 for ; Wed, 25 Jun 2025 04:28:39 +0000 (UTC) X-FDA: 83592641958.25.D62EAD5 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf22.hostedemail.com (Postfix) with ESMTP id B77D4C0013 for ; Wed, 25 Jun 2025 04:28:37 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1750825718; 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=Y/lklILdEV8h/1GnXCtzWgpir8wts9B4bfbKn45HVuM=; b=oD22SSmLvv9Hi+s1BV26apztnERiRQLGF0K9sgUT7OXxVEs0xqzb7pBtz0ZuMRegvmhumW qXJIp+M5nZANbhNNneFo2QaoJTlR7TRvD1J7BHT4ocMPVfqyAmaIXdBzRt+W6gfY2SkV9u RaMjyFcGfxcgsMkp2eHnGCsiPyzP38A= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750825718; a=rsa-sha256; cv=none; b=c0VKolY6pdVBqLtlnuU/Y6AVR/m5UXrmCGB7y4SBMx2DXEqQ9k1uHcSapiDnTwyrsrP8rg xNz/hr0WX0Z1clj1hqTkg4D+fJF5d03AityhB9QoEPzB7TvLYpNQ2AJ2TACc5UJhG9dO/8 eUpriA9ALCm8RwzTlkeAp2qB/DTNdYk= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com; dmarc=pass (policy=none) header.from=arm.com 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 94C4D113E; Tue, 24 Jun 2025 21:28:18 -0700 (PDT) Received: from [10.164.146.16] (J09HK2D2RT.blr.arm.com [10.164.146.16]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B39513F58B; Tue, 24 Jun 2025 21:28:34 -0700 (PDT) Message-ID: <9fb04185-5b71-46c0-b62c-0e0e6ee59e6e@arm.com> Date: Wed, 25 Jun 2025 09:58:31 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH 1/1] mm/debug_vm_pgtable: Use a swp_entry_t input value for swap tests To: Gerald Schaefer , Andrew Morton Cc: Matthew Wilcox , David Hildenbrand , LKML , linux-mm References: <20250623184321.927418-1-gerald.schaefer@linux.ibm.com> <20250623184321.927418-2-gerald.schaefer@linux.ibm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <20250623184321.927418-2-gerald.schaefer@linux.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: B77D4C0013 X-Stat-Signature: ya3ugrj7r41zswmtszwazf6gi1nhncc5 X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1750825717-507996 X-HE-Meta: U2FsdGVkX1+9bdMRbKVinbi9TIImHyGi4Pv/PFHkYEb53QVbirKxN3DJc8j/lp/nauLEoWfYl3xuLG/NJHImS6nCDlykfDWcLPBDM9Rv1vgLhfxjANYNfzAOas6EjLkUmUGaz8znfmPzox1167/16dd4nIebnHpM1XaZGqA0BdbcHwBmKE8LXXCLPSDZvtX/7d1cCNDqyyW3SvM0wpShSp5ZaOdmKR9wygTG2mbXJ5A/FcF0dTUywK/qE33tmhX1cpH+PwGgZsYAsW+E3DcFZCk/YOaBVa+5m2xEQEJHe9dj7Pp25HDWXcOPZ9YT0Q69ijBVlzlqqmHaJlpkD64wEFxbqSKtrHq6bQx9XdpSt1V8vzbXlyOGy4hh4pxsUeqkGUk6YA5OG/IoA8AxISK6dGsEeRxDARTAUrP04lci/4VmjKNMGNIVOghBC3IaSfR+NMNzFWJpsH87vxSD69lNofY3+Udmd9G8ii5ArQz2r75h4rgL5HHFCstGmPnsI81iZ5p+BWgSXvkdJJ/Wajs+/HWcX1bXJIRSAgFQTLNZLKBtM3oY6XEEYCrCa03A3o8Utqyye2tJT2lRRHm1JWlLMEy9+YQOydKNwYi7f114nKHE3//H393t6T/1QswLTywuzAovmpDg7/kaYjXtlZnt+5OFpbGTuShZrJaznTv4uUzJj9ubu27EFHKK8d6b2YLyYu5DVvu4Iad+fI5j/NZMYjdu8HCnlDmLiCxVfKbsWFB6y+tprUn14ob4TUvMmKvOv6yP+B2Ro5kRYhwX6kXY2mRB7XeSSeXoQ791e2PHjxcrUtVp3nHXqW/MLxXUbsNCDg8aJjcEvmHJYzWfA1NYtnPwvIeY2ytmG3AMr/MQFfyjgzGErzmEUVLyyFeP+jPRy7OevX17tCoV+VLMTumP/hxN86HxJ1zAQWWTzyTK2Akv/HiuVXvoz9L/Xu2nT1zcKbRBzh0fc/SDOL3Re7C /LK6tChx DYOpQH0IT59VAt8zOcK9NPNcy0kEx7+SmgN9ywztgVFsOWVFo7u3P9qLwlityAmTaIirRzXbzzDNK5Tjx8J2zBP7datWkW8jGkJK3dyi7jIkE89T5gwX7nVmU5j2ZAg9jblUKbSbB50Nme+q8Otkdo3EOtHVK2gYONxKNMu902UPqvpc7bvqeJxU/bVIphE5peDLH1OmsTB2lxE1ouEeY/IIFrot/+s6K5MLXoRQNTDlPHKV4rf3s/QtLww== 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/06/25 12:13 AM, Gerald Schaefer wrote: > The various __pte/pmd_to_swp_entry and __swp_entry_to_pte/pmd helper > functions are expected to operate on swapped PTE/PMD entries, not on > present and mapped entries. > > Reflect this in the swap tests by using a swp_entry_t as input value, > similar to how it is already done in pte_swap_exclusive_tests(). > Move the swap entry creation to init_args() and store it in args, so > it can also be used in other functions. Makes sense. > > The pte/pmd_swap_tests() are also changed to compare entries instead of > pfn values, because pte/pmd_pfn() helpers are not expected to operate on Swap entries compare is also happening now in pte_swap_exclusive_tests(). > swapped entries. E.g. on s390, pmd_pfn() needs different shifts for leaf > (large) and non-leaf PMDs. > > Also update documentation, to reflect that the helpers operate on > swapped and not mapped entries, and use correct names, i.e. > __swp_to_pte/pmd_entry -> __swp_entry_to_pte/pmd. > > For consistency, also change pte/pmd_swap_soft_dirty_tests() to use > args->swp_entry instead of a present and mapped PTE/PMD. Makes sense. > > Signed-off-by: Gerald Schaefer > --- > Documentation/mm/arch_pgtable_helpers.rst | 8 ++-- > mm/debug_vm_pgtable.c | 55 ++++++++++++++--------- > 2 files changed, 38 insertions(+), 25 deletions(-) > > diff --git a/Documentation/mm/arch_pgtable_helpers.rst b/Documentation/mm/arch_pgtable_helpers.rst > index af245161d8e7..e2ac76202a85 100644 > --- a/Documentation/mm/arch_pgtable_helpers.rst > +++ b/Documentation/mm/arch_pgtable_helpers.rst > @@ -242,13 +242,13 @@ SWAP Page Table Helpers > ======================== > > +---------------------------+--------------------------------------------------+ > -| __pte_to_swp_entry | Creates a swapped entry (arch) from a mapped PTE | > +| __pte_to_swp_entry | Creates a swap entry (arch) from a swapped PTE | > +---------------------------+--------------------------------------------------+ > -| __swp_to_pte_entry | Creates a mapped PTE from a swapped entry (arch) | > +| __swp_entry_to_pte | Creates a swapped PTE from a swap entry (arch) | > +---------------------------+--------------------------------------------------+ > -| __pmd_to_swp_entry | Creates a swapped entry (arch) from a mapped PMD | > +| __pmd_to_swp_entry | Creates a swap entry (arch) from a swapped PMD | > +---------------------------+--------------------------------------------------+ > -| __swp_to_pmd_entry | Creates a mapped PMD from a swapped entry (arch) | > +| __swp_entry_to_pmd | Creates a swapped PMD from a swap entry (arch) | > +---------------------------+--------------------------------------------------+ > | is_migration_entry | Tests a migration (read or write) swapped entry | > +-------------------------------+----------------------------------------------+ __pte_to_swp_entry() and __pmd_to_swp_entry() are still being used (and tested) even after applying this patch. Should not their entries be preserved ? > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c > index 7731b238b534..3b0f83ed6c2e 100644 > --- a/mm/debug_vm_pgtable.c > +++ b/mm/debug_vm_pgtable.c > @@ -73,6 +73,8 @@ struct pgtable_debug_args { > unsigned long fixed_pud_pfn; > unsigned long fixed_pmd_pfn; > unsigned long fixed_pte_pfn; > + > + swp_entry_t swp_entry; > }; > > static void __init pte_basic_tests(struct pgtable_debug_args *args, int idx) > @@ -754,12 +756,15 @@ static void __init pte_soft_dirty_tests(struct pgtable_debug_args *args) > > static void __init pte_swap_soft_dirty_tests(struct pgtable_debug_args *args) > { > - pte_t pte = pfn_pte(args->fixed_pte_pfn, args->page_prot); > + pte_t pte; > > if (!IS_ENABLED(CONFIG_MEM_SOFT_DIRTY)) > return; > > pr_debug("Validating PTE swap soft dirty\n"); > + pte = swp_entry_to_pte(args->swp_entry); > + WARN_ON(!is_swap_pte(pte)); > + > WARN_ON(!pte_swp_soft_dirty(pte_swp_mksoft_dirty(pte))); > WARN_ON(pte_swp_soft_dirty(pte_swp_clear_soft_dirty(pte))); > } > @@ -793,7 +798,9 @@ static void __init pmd_swap_soft_dirty_tests(struct pgtable_debug_args *args) > return; > > pr_debug("Validating PMD swap soft dirty\n"); > - pmd = pfn_pmd(args->fixed_pmd_pfn, args->page_prot); > + pmd = swp_entry_to_pmd(args->swp_entry); > + WARN_ON(!is_swap_pmd(pmd)); > + > WARN_ON(!pmd_swp_soft_dirty(pmd_swp_mksoft_dirty(pmd))); > WARN_ON(pmd_swp_soft_dirty(pmd_swp_clear_soft_dirty(pmd))); > } > @@ -804,17 +811,11 @@ static void __init pmd_swap_soft_dirty_tests(struct pgtable_debug_args *args) { > > static void __init pte_swap_exclusive_tests(struct pgtable_debug_args *args) > { > - unsigned long max_swap_offset; > swp_entry_t entry, entry2; > pte_t pte; > > pr_debug("Validating PTE swap exclusive\n"); > - > - /* See generic_max_swapfile_size(): probe the maximum offset */ > - max_swap_offset = swp_offset(pte_to_swp_entry(swp_entry_to_pte(swp_entry(0, ~0UL)))); > - > - /* Create a swp entry with all possible bits set */ > - entry = swp_entry((1 << MAX_SWAPFILES_SHIFT) - 1, max_swap_offset); > + entry = args->swp_entry; args->swp_entry should be reused here as well. > > pte = swp_entry_to_pte(entry); > WARN_ON(pte_swp_exclusive(pte)); > @@ -838,30 +839,36 @@ static void __init pte_swap_exclusive_tests(struct pgtable_debug_args *args) > > static void __init pte_swap_tests(struct pgtable_debug_args *args) > { > - swp_entry_t swp; > - pte_t pte; > + swp_entry_t entry, arch_entry; > + pte_t pte, pte2; A very small nit - s/pte2/pte as the first one is pmd not pte or make it pte1, pte2 if preferred. > > pr_debug("Validating PTE swap\n"); > - pte = pfn_pte(args->fixed_pte_pfn, args->page_prot); > - swp = __pte_to_swp_entry(pte); > - pte = __swp_entry_to_pte(swp); > - WARN_ON(args->fixed_pte_pfn != pte_pfn(pte)); > + entry = args->swp_entry; Should args->swp_entry be used directly here and 'entry' local variable be dropped ? > + > + pte = swp_entry_to_pte(entry); > + WARN_ON(!is_swap_pte(pte)); > + arch_entry = __pte_to_swp_entry(pte); > + pte2 = __swp_entry_to_pte(arch_entry); > + WARN_ON(memcmp(&pte, &pte2, sizeof(pte))); > } > > #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION > static void __init pmd_swap_tests(struct pgtable_debug_args *args) > { > - swp_entry_t swp; > - pmd_t pmd; > + swp_entry_t entry, arch_entry; > + pmd_t pmd, pmd2; A very small nit - s/pmd2/pmd1 as the first one is pmd not pmd1 or just make it pmd1, pmd2 if preferred. > > if (!has_transparent_hugepage()) > return; > > pr_debug("Validating PMD swap\n"); > - pmd = pfn_pmd(args->fixed_pmd_pfn, args->page_prot); > - swp = __pmd_to_swp_entry(pmd); > - pmd = __swp_entry_to_pmd(swp); > - WARN_ON(args->fixed_pmd_pfn != pmd_pfn(pmd)); > + entry = args->swp_entry; > + pmd = swp_entry_to_pmd(entry); > + WARN_ON(!is_swap_pmd(pmd)); Good to have the WARN_ON() to test whether it is a swap PMD or not. > + > + arch_entry = __pmd_to_swp_entry(pmd); > + pmd2 = __swp_entry_to_pmd(arch_entry); > + WARN_ON(memcmp(&pmd, &pmd2, sizeof(pmd))); Sounds good. > } > #else /* !CONFIG_ARCH_ENABLE_THP_MIGRATION */ > static void __init pmd_swap_tests(struct pgtable_debug_args *args) { } > @@ -1166,6 +1173,7 @@ static void __init init_fixed_pfns(struct pgtable_debug_args *args) > > static int __init init_args(struct pgtable_debug_args *args) > { > + unsigned long max_swap_offset; > struct page *page = NULL; > int ret = 0; > > @@ -1248,6 +1256,11 @@ static int __init init_args(struct pgtable_debug_args *args) > > init_fixed_pfns(args); > > + /* See generic_max_swapfile_size(): probe the maximum offset */ > + max_swap_offset = swp_offset(pte_to_swp_entry(swp_entry_to_pte(swp_entry(0, ~0UL)))); Why not directly use generic_max_swapfile_size() which is doing exact same thing. unsigned long generic_max_swapfile_size(void) { return swp_offset(pte_to_swp_entry( swp_entry_to_pte(swp_entry(0, ~0UL)))) + 1; } > + /* Create a swp entry with all possible bits set */ > + args->swp_entry = swp_entry((1 << MAX_SWAPFILES_SHIFT) - 1, max_swap_offset); > + Makes sense to use maximum possible bits while creating the swap entry for testing. > /* > * Allocate (huge) pages because some of the tests need to access > * the data in the pages. The corresponding tests will be skipped