From: Steven Price <steven.price@arm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>,
linux-mm@kvack.org, akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
christophe.leroy@csgroup.eu, gerald.schaefer@linux.ibm.com,
vgupta@synopsys.com, paul.walmsley@sifive.com
Subject: Re: [PATCH 2/2] mm/debug_vm_pgtable/basic: Iterate over entire protection_map[]
Date: Fri, 27 Nov 2020 09:14:23 +0000 [thread overview]
Message-ID: <4c3efec7-a454-951f-4f37-fd8ed1908b8c@arm.com> (raw)
In-Reply-To: <1606453584-15399-3-git-send-email-anshuman.khandual@arm.com>
On 27/11/2020 05:06, Anshuman Khandual wrote:
> Currently the basic tests just validate various page table transformations
> after starting with vm_get_page_prot(VM_READ|VM_WRITE|VM_EXEC) protection.
> Instead scan over the entire protection_map[] for better coverage. It also
> makes sure that all these basic page table tranformations checks hold true
> irrespective of the starting protection value for the page table entry.
> There is also a slight change in the debug print format for basic tests to
> capture the protection value it is being tested with. The modified output
> looks something like
>
> [pte_basic_tests ]: Validating PTE basic ()
> [pte_basic_tests ]: Validating PTE basic (read)
> [pte_basic_tests ]: Validating PTE basic (write)
> [pte_basic_tests ]: Validating PTE basic (read|write)
> [pte_basic_tests ]: Validating PTE basic (exec)
> [pte_basic_tests ]: Validating PTE basic (read|exec)
> [pte_basic_tests ]: Validating PTE basic (write|exec)
> [pte_basic_tests ]: Validating PTE basic (read|write|exec)
> [pte_basic_tests ]: Validating PTE basic (shared)
> [pte_basic_tests ]: Validating PTE basic (read|shared)
> [pte_basic_tests ]: Validating PTE basic (write|shared)
> [pte_basic_tests ]: Validating PTE basic (read|write|shared)
> [pte_basic_tests ]: Validating PTE basic (exec|shared)
> [pte_basic_tests ]: Validating PTE basic (read|exec|shared)
> [pte_basic_tests ]: Validating PTE basic (write|exec|shared)
> [pte_basic_tests ]: Validating PTE basic (read|write|exec|shared)
>
> This adds a missing argument 'struct mm_struct *' in pud_basic_tests() test
> . This never got exposed before as PUD based THP is available only on X86
> platform where mm_pmd_folded(mm) call gets macro replaced without requiring
> the mm_struct i.e __is_defined(__PAGETABLE_PMD_FOLDED).
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Reviewed-by: Steven Price <steven.price@arm.com>
> ---
> mm/debug_vm_pgtable.c | 47 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index a5be11210597..92b4a53d622b 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -58,11 +58,13 @@
> #define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK)
> #define RANDOM_NZVALUE GENMASK(7, 0)
>
> -static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
> +static void __init pte_basic_tests(unsigned long pfn, int idx)
> {
> + pgprot_t prot = protection_map[idx];
> pte_t pte = pfn_pte(pfn, prot);
> + unsigned long val = idx, *ptr = &val;
>
> - pr_debug("Validating PTE basic\n");
> + pr_debug("Validating PTE basic (%pGv)\n", ptr);
> WARN_ON(!pte_same(pte, pte));
> WARN_ON(!pte_young(pte_mkyoung(pte_mkold(pte))));
> WARN_ON(!pte_dirty(pte_mkdirty(pte_mkclean(pte))));
> @@ -130,14 +132,16 @@ static void __init pte_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
> +static void __init pmd_basic_tests(unsigned long pfn, int idx)
> {
> + pgprot_t prot = protection_map[idx];
> pmd_t pmd = pfn_pmd(pfn, prot);
> + unsigned long val = idx, *ptr = &val;
>
> if (!has_transparent_hugepage())
> return;
>
> - pr_debug("Validating PMD basic\n");
> + pr_debug("Validating PMD basic (%pGv)\n", ptr);
> WARN_ON(!pmd_same(pmd, pmd));
> WARN_ON(!pmd_young(pmd_mkyoung(pmd_mkold(pmd))));
> WARN_ON(!pmd_dirty(pmd_mkdirty(pmd_mkclean(pmd))));
> @@ -251,14 +255,16 @@ static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> }
>
> #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> -static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
> +static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx)
> {
> + pgprot_t prot = protection_map[idx];
> pud_t pud = pfn_pud(pfn, prot);
> + unsigned long val = idx, *ptr = &val;
>
> if (!has_transparent_hugepage())
> return;
>
> - pr_debug("Validating PUD basic\n");
> + pr_debug("Validating PUD basic (%pGv)\n", ptr);
> WARN_ON(!pud_same(pud, pud));
> WARN_ON(!pud_young(pud_mkyoung(pud_mkold(pud))));
> WARN_ON(!pud_write(pud_mkwrite(pud_wrprotect(pud))));
> @@ -362,7 +368,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> #endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
>
> #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> -static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> +static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { }
> static void __init pud_advanced_tests(struct mm_struct *mm,
> struct vm_area_struct *vma, pud_t *pudp,
> unsigned long pfn, unsigned long vaddr,
> @@ -375,8 +381,8 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> }
> #endif /* CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */
> -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
> -static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> +static void __init pmd_basic_tests(unsigned long pfn, int idx) { }
> +static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { }
> static void __init pmd_advanced_tests(struct mm_struct *mm,
> struct vm_area_struct *vma, pmd_t *pmdp,
> unsigned long pfn, unsigned long vaddr,
> @@ -902,6 +908,7 @@ static int __init debug_vm_pgtable(void)
> unsigned long vaddr, pte_aligned, pmd_aligned;
> unsigned long pud_aligned, p4d_aligned, pgd_aligned;
> spinlock_t *ptl = NULL;
> + int idx;
>
> pr_info("Validating architecture page table helpers\n");
> prot = vm_get_page_prot(VMFLAGS);
> @@ -966,9 +973,25 @@ static int __init debug_vm_pgtable(void)
> saved_pmdp = pmd_offset(pudp, 0UL);
> saved_ptep = pmd_pgtable(pmd);
>
> - pte_basic_tests(pte_aligned, prot);
> - pmd_basic_tests(pmd_aligned, prot);
> - pud_basic_tests(pud_aligned, prot);
> + /*
> + * Iterate over the protection_map[] to make sure that all
> + * the basic page table transformation validations just hold
> + * true irrespective of the starting protection value for a
> + * given page table entry.
> + */
> + for (idx = 0; idx < ARRAY_SIZE(protection_map); idx++) {
> + pte_basic_tests(pte_aligned, idx);
> + pmd_basic_tests(pmd_aligned, idx);
> + pud_basic_tests(mm, pud_aligned, idx);
> + }
> +
> + /*
> + * Both P4D and PGD level tests are very basic which do not
> + * involve creating page table entries from the protection
> + * value and the given pfn. Hence just keep them out from
> + * the above iteration for now to save some test execution
> + * time.
> + */
> p4d_basic_tests(p4d_aligned, prot);
> pgd_basic_tests(pgd_aligned, prot);
>
>
prev parent reply other threads:[~2020-11-27 9:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-27 5:06 [PATCH 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual
2020-11-27 5:06 ` [PATCH 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect Anshuman Khandual
2020-11-27 8:22 ` Christophe Leroy
2020-11-27 9:44 ` Catalin Marinas
2020-11-30 4:25 ` Anshuman Khandual
2020-11-30 9:38 ` Catalin Marinas
2020-11-30 10:58 ` Anshuman Khandual
2020-11-30 11:01 ` Catalin Marinas
2020-11-27 5:06 ` [PATCH 2/2] mm/debug_vm_pgtable/basic: Iterate over entire protection_map[] Anshuman Khandual
2020-11-27 9:14 ` Steven Price [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4c3efec7-a454-951f-4f37-fd8ed1908b8c@arm.com \
--to=steven.price@arm.com \
--cc=akpm@linux-foundation.org \
--cc=anshuman.khandual@arm.com \
--cc=catalin.marinas@arm.com \
--cc=christophe.leroy@csgroup.eu \
--cc=gerald.schaefer@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=paul.walmsley@sifive.com \
--cc=vgupta@synopsys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).