[PATCH V5 1/4] mm/debug_vm_pgtable: Add tests validating arch helpers for core MM features

[Steven Price]

On 13/07/2020 04:23, Anshuman Khandual wrote:
> This adds new tests validating arch page table helpers for these following
> core memory features. These tests create and test specific mapping types at
> various page table levels.
> 
> 1. SPECIAL mapping
> 2. PROTNONE mapping
> 3. DEVMAP mapping
> 4. SOFTDIRTY mapping
> 5. SWAP mapping
> 6. MIGRATION mapping
> 7. HUGETLB mapping
> 8. THP mapping
> 
> Cc: Andrew Morton <akpm at linux-foundation.org>
> Cc: Gerald Schaefer <gerald.schaefer at de.ibm.com>
> Cc: Christophe Leroy <christophe.leroy at c-s.fr>
> Cc: Mike Rapoport <rppt at linux.ibm.com>
> Cc: Vineet Gupta <vgupta at synopsys.com>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Will Deacon <will at kernel.org>
> Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> Cc: Paul Mackerras <paulus at samba.org>
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
> Cc: Vasily Gorbik <gor at linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger at de.ibm.com>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Ingo Molnar <mingo at redhat.com>
> Cc: Borislav Petkov <bp at alien8.de>
> Cc: "H. Peter Anvin" <hpa at zytor.com>
> Cc: Kirill A. Shutemov <kirill at shutemov.name>
> Cc: Paul Walmsley <paul.walmsley at sifive.com>
> Cc: Palmer Dabbelt <palmer at dabbelt.com>
> Cc: linux-snps-arc at lists.infradead.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linuxppc-dev at lists.ozlabs.org
> Cc: linux-s390 at vger.kernel.org
> Cc: linux-riscv at lists.infradead.org
> Cc: x86 at kernel.org
> Cc: linux-mm at kvack.org
> Cc: linux-arch at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Tested-by: Vineet Gupta <vgupta at synopsys.com>	#arc
> Reviewed-by: Zi Yan <ziy at nvidia.com>
> Suggested-by: Catalin Marinas <catalin.marinas at arm.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual at arm.com>
> ---
>   mm/debug_vm_pgtable.c | 302 +++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 301 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 61ab16fb2e36..2fac47db3eb7 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
[...]
> +
> +static void __init pte_swap_tests(unsigned long pfn, pgprot_t prot)
> +{
> +	swp_entry_t swp;
> +	pte_t pte;
> +
> +	pte = pfn_pte(pfn, prot);
> +	swp = __pte_to_swp_entry(pte);

Minor issue: this doesn't look necessarily valid - there's no reason a 
normal PTE can be turned into a swp_entry. In practise this is likely to 
work on all architectures because there's no reason not to use (at 
least) all the PFN bits for the swap entry, but it doesn't exactly seem 
correct.

Can we start with a swp_entry_t (from __swp_entry()) and check the round 
trip of that?

It would also seem sensible to have a check that 
is_swap_pte(__swp_entry_to_pte(__swp_entry(x,y))) is true.

> +	pte = __swp_entry_to_pte(swp);
> +	WARN_ON(pfn != pte_pfn(pte));
> +}
> +
> +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> +static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot)
> +{
> +	swp_entry_t swp;
> +	pmd_t pmd;
> +
> +	pmd = pfn_pmd(pfn, prot);
> +	swp = __pmd_to_swp_entry(pmd);
> +	pmd = __swp_entry_to_pmd(swp);
> +	WARN_ON(pfn != pmd_pfn(pmd));
> +}
> +#else  /* !CONFIG_ARCH_ENABLE_THP_MIGRATION */
> +static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot) { }
> +#endif /* CONFIG_ARCH_ENABLE_THP_MIGRATION */
> +
> +static void __init swap_migration_tests(void)
> +{
> +	struct page *page;
> +	swp_entry_t swp;
> +
> +	if (!IS_ENABLED(CONFIG_MIGRATION))
> +		return;
> +	/*
> +	 * swap_migration_tests() requires a dedicated page as it needs to
> +	 * be locked before creating a migration entry from it. Locking the
> +	 * page that actually maps kernel text ('start_kernel') can be real
> +	 * problematic. Lets allocate a dedicated page explicitly for this

NIT: s/Lets/Let's

Otherwise looks good to me.

Steve