linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V8] mm/debug: Add tests validating architecture page table helpers
@ 2019-10-28  5:29 Anshuman Khandual
  2019-10-29 10:31 ` Qian Cai
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Anshuman Khandual @ 2019-10-28  5:29 UTC (permalink / raw)
  To: linux-mm
  Cc: Anshuman Khandual, Andrew Morton, Vlastimil Babka,
	Greg Kroah-Hartman, Thomas Gleixner, Mike Rapoport,
	Jason Gunthorpe, Dan Williams, Peter Zijlstra, Michal Hocko,
	Mark Rutland, Mark Brown, Steven Price, Ard Biesheuvel,
	Masahiro Yamada, Kees Cook, Tetsuo Handa, Matthew Wilcox,
	Sri Krishna chowdary, Dave Hansen, Russell King - ARM Linux,
	Michael Ellerman, Paul Mackerras, Martin Schwidefsky,
	Heiko Carstens, David S. Miller, Vineet Gupta, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel

This adds tests which will validate architecture page table helpers and
other accessors in their compliance with expected generic MM semantics.
This will help various architectures in validating changes to existing
page table helpers or addition of new ones.

This test covers basic page table entry transformations including but not
limited to old, young, dirty, clean, write, write protect etc at various
level along with populating intermediate entries with next page table page
and validating them.

Test page table pages are allocated from system memory with required size
and alignments. The mapped pfns at page table levels are derived from a
real pfn representing a valid kernel text symbol. This test gets called
right after page_alloc_init_late().

This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
arm64. Going forward, other architectures too can enable this after fixing
build or runtime problems (if any) with their page table helpers.

Folks interested in making sure that a given platform's page table helpers
conform to expected generic MM semantics should enable the above config
which will just trigger this test during boot. Any non conformity here will
be reported as an warning which would need to be fixed. This test will help
catch any changes to the agreed upon semantics expected from generic MM and
enable platforms to accommodate it thereafter.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steven Price <Steven.Price@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Sri Krishna chowdary <schowdary@nvidia.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org

Tested-by: Christophe Leroy <christophe.leroy@c-s.fr>		#PPC32
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---

This adds a test validation for architecture exported page table helpers.
Patch adds basic transformation tests at various levels of the page table.

This test was originally suggested by Catalin during arm64 THP migration
RFC discussion earlier. Going forward it can include more specific tests
with respect to various generic MM functions like THP, HugeTLB etc and
platform specific tests.

https://lore.kernel.org/linux-mm/20190628102003.GA56463@arrakis.emea.arm.com/

Needs to be applied on linux-next (next-20191025).

Changes in V8:

- Enabled ARCH_HAS_DEBUG_VM_PGTABLE on PPC32 platform per Christophe
- Updated feature documentation as DEBUG_VM_PGTABLE is now enabled on PPC32 platform
- Moved ARCH_HAS_DEBUG_VM_PGTABLE earlier to indent it with DEBUG_VM per Christophe
- Added an information message in debug_vm_pgtable() per Christophe
- Dropped random_vaddr boundary condition checks per Christophe and Qian
- Replaced virt_addr_valid() check with pfn_valid() check in debug_vm_pgtable()
- Slightly changed pr_fmt(fmt) information

Changes in V7: (https://patchwork.kernel.org/project/linux-mm/list/?series=193051)

- Memory allocation and free routines for mapped pages have been droped
- Mapped pfns are derived from standard kernel text symbol per Matthew
- Moved debug_vm_pgtaable() after page_alloc_init_late() per Michal and Qian 
- Updated the commit message per Michal
- Updated W=1 GCC warning problem on x86 per Qian Cai
- Addition of new alloc_contig_pages() helper has been submitted separately

Changes in V6: (https://patchwork.kernel.org/project/linux-mm/list/?series=187589)

- Moved alloc_gigantic_page_order() into mm/page_alloc.c per Michal
- Moved alloc_gigantic_page_order() within CONFIG_CONTIG_ALLOC in the test
- Folded Andrew's include/asm-generic/pgtable.h fix into the test patch 2/2

Changes in V5: (https://patchwork.kernel.org/project/linux-mm/list/?series=185991)

- Redefined and moved X86 mm_p4d_folded() into a different header per Kirill/Ingo
- Updated the config option comment per Ingo and dropped 'kernel module' reference
- Updated the commit message and dropped 'kernel module' reference
- Changed DEBUG_ARCH_PGTABLE_TEST into DEBUG_VM_PGTABLE per Ingo
- Moved config option from mm/Kconfig.debug into lib/Kconfig.debug
- Renamed core test function arch_pgtable_tests() as debug_vm_pgtable()
- Renamed mm/arch_pgtable_test.c as mm/debug_vm_pgtable.c
- debug_vm_pgtable() gets called from kernel_init_freeable() after init_mm_internals()
- Added an entry in Documentation/features/debug/ per Ingo
- Enabled the test on arm64 and x86 platforms for now

Changes in V4: (https://patchwork.kernel.org/project/linux-mm/list/?series=183465)

- Disable DEBUG_ARCH_PGTABLE_TEST for ARM and IA64 platforms

Changes in V3: (https://lore.kernel.org/patchwork/project/lkml/list/?series=411216)

- Changed test trigger from module format into late_initcall()
- Marked all functions with __init to be freed after completion
- Changed all __PGTABLE_PXX_FOLDED checks as mm_pxx_folded()
- Folded in PPC32 fixes from Christophe

Changes in V2:

https://lore.kernel.org/linux-mm/1568268173-31302-1-git-send-email-anshuman.khandual@arm.com/T/#t

- Fixed small typo error in MODULE_DESCRIPTION()
- Fixed m64k build problems for lvalue concerns in pmd_xxx_tests()
- Fixed dynamic page table level folding problems on x86 as per Kirril
- Fixed second pointers during pxx_populate_tests() per Kirill and Gerald
- Allocate and free pte table with pte_alloc_one/pte_free per Kirill
- Modified pxx_clear_tests() to accommodate s390 lower 12 bits situation
- Changed RANDOM_NZVALUE value from 0xbe to 0xff
- Changed allocation, usage, free sequence for saved_ptep
- Renamed VMA_FLAGS as VMFLAGS
- Implemented a new method for random vaddr generation
- Implemented some other cleanups
- Dropped extern reference to mm_alloc()
- Created and exported new alloc_gigantic_page_order()
- Dropped the custom allocator and used new alloc_gigantic_page_order()

Changes in V1:

https://lore.kernel.org/linux-mm/1567497706-8649-1-git-send-email-anshuman.khandual@arm.com/

- Added fallback mechanism for PMD aligned memory allocation failure

Changes in RFC V2:

https://lore.kernel.org/linux-mm/1565335998-22553-1-git-send-email-anshuman.khandual@arm.com/T/#u

- Moved test module and it's config from lib/ to mm/
- Renamed config TEST_ARCH_PGTABLE as DEBUG_ARCH_PGTABLE_TEST
- Renamed file from test_arch_pgtable.c to arch_pgtable_test.c
- Added relevant MODULE_DESCRIPTION() and MODULE_AUTHOR() details
- Dropped loadable module config option
- Basic tests now use memory blocks with required size and alignment
- PUD aligned memory block gets allocated with alloc_contig_range()
- If PUD aligned memory could not be allocated it falls back on PMD aligned
  memory block from page allocator and pud_* tests are skipped
- Clear and populate tests now operate on real in memory page table entries
- Dummy mm_struct gets allocated with mm_alloc()
- Dummy page table entries get allocated with [pud|pmd|pte]_alloc_[map]()
- Simplified [p4d|pgd]_basic_tests(), now has random values in the entries

Original RFC V1:

https://lore.kernel.org/linux-mm/1564037723-26676-1-git-send-email-anshuman.khandual@arm.com/

 .../debug/debug-vm-pgtable/arch-support.txt   |  35 ++
 arch/arm64/Kconfig                            |   1 +
 arch/powerpc/Kconfig                          |   1 +
 arch/x86/Kconfig                              |   1 +
 arch/x86/include/asm/pgtable_64.h             |   6 +
 include/asm-generic/pgtable.h                 |   6 +
 init/main.c                                   |   1 +
 lib/Kconfig.debug                             |  21 +
 mm/Makefile                                   |   1 +
 mm/debug_vm_pgtable.c                         | 387 ++++++++++++++++++
 10 files changed, 460 insertions(+)
 create mode 100644 Documentation/features/debug/debug-vm-pgtable/arch-support.txt
 create mode 100644 mm/debug_vm_pgtable.c

diff --git a/Documentation/features/debug/debug-vm-pgtable/arch-support.txt b/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
new file mode 100644
index 000000000000..764397de9283
--- /dev/null
+++ b/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
@@ -0,0 +1,35 @@
+#
+# Feature name:          debug-vm-pgtable
+#         Kconfig:       ARCH_HAS_DEBUG_VM_PGTABLE
+#         description:   arch supports pgtable tests for semantics compliance
+#
+    -----------------------
+    |         arch |status|
+    -----------------------
+    |       alpha: | TODO |
+    |         arc: | TODO |
+    |         arm: | TODO |
+    |       arm64: |  ok  |
+    |         c6x: | TODO |
+    |        csky: | TODO |
+    |       h8300: | TODO |
+    |     hexagon: | TODO |
+    |        ia64: | TODO |
+    |        m68k: | TODO |
+    |  microblaze: | TODO |
+    |        mips: | TODO |
+    |       nds32: | TODO |
+    |       nios2: | TODO |
+    |    openrisc: | TODO |
+    |      parisc: | TODO |
+    |     powerpc: | TODO |
+    |       ppc32: |  ok  |
+    |       riscv: | TODO |
+    |        s390: | TODO |
+    |          sh: | TODO |
+    |       sparc: | TODO |
+    |          um: | TODO |
+    |   unicore32: | TODO |
+    |         x86: |  ok  |
+    |      xtensa: | TODO |
+    -----------------------
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 8f2544bdcbe6..5768d639d1d4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -11,6 +11,7 @@ config ARM64
 	select ACPI_PPTT if ACPI
 	select ARCH_CLOCKSOURCE_DATA
 	select ARCH_HAS_DEBUG_VIRTUAL
+	select ARCH_HAS_DEBUG_VM_PGTABLE
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_DMA_COHERENT_TO_PFN
 	select ARCH_HAS_DMA_PREP_COHERENT
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 4c4a0fcd1674..27c5aa438587 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -120,6 +120,7 @@ config PPC
 	#
 	select ARCH_32BIT_OFF_T if PPC32
 	select ARCH_HAS_DEBUG_VIRTUAL
+	select ARCH_HAS_DEBUG_VM_PGTABLE if PPC32
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FORTIFY_SOURCE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 45699e458057..3b4f096c68cc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -61,6 +61,7 @@ config X86
 	select ARCH_CLOCKSOURCE_INIT
 	select ARCH_HAS_ACPI_TABLE_UPGRADE	if ACPI
 	select ARCH_HAS_DEBUG_VIRTUAL
+	select ARCH_HAS_DEBUG_VM_PGTABLE
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FAST_MULTIPLIER
diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 0b6c4042942a..fb0e76d254b3 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -53,6 +53,12 @@ static inline void sync_initial_page_table(void) { }
 
 struct mm_struct;
 
+#define mm_p4d_folded mm_p4d_folded
+static inline bool mm_p4d_folded(struct mm_struct *mm)
+{
+	return !pgtable_l5_enabled();
+}
+
 void set_pte_vaddr_p4d(p4d_t *p4d_page, unsigned long vaddr, pte_t new_pte);
 void set_pte_vaddr_pud(pud_t *pud_page, unsigned long vaddr, pte_t new_pte);
 
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 9cdcbc7c0b7b..9eb02e105162 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -1168,6 +1168,12 @@ static inline bool arch_has_pfn_modify_check(void)
 # define PAGE_KERNEL_EXEC PAGE_KERNEL
 #endif
 
+#ifdef CONFIG_DEBUG_VM_PGTABLE
+extern void debug_vm_pgtable(void);
+#else
+static inline void debug_vm_pgtable(void) { }
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 #ifndef io_remap_pfn_range
diff --git a/init/main.c b/init/main.c
index 91f6ebb30ef0..af8379ed53dc 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1185,6 +1185,7 @@ static noinline void __init kernel_init_freeable(void)
 	sched_init_smp();
 
 	page_alloc_init_late();
+	debug_vm_pgtable();
 	/* Initialize page ext after all struct pages are initialized. */
 	page_ext_init();
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b48f5e974ed7..187101961b13 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -664,6 +664,12 @@ config SCHED_STACK_END_CHECK
 	  data corruption or a sporadic crash at a later stage once the region
 	  is examined. The runtime overhead introduced is minimal.
 
+config ARCH_HAS_DEBUG_VM_PGTABLE
+	bool
+	help
+	  An architecture should select this when it can successfully
+	  build and run DEBUG_VM_PGTABLE.
+
 config DEBUG_VM
 	bool "Debug VM"
 	depends on DEBUG_KERNEL
@@ -699,6 +705,21 @@ config DEBUG_VM_PGFLAGS
 
 	  If unsure, say N.
 
+config DEBUG_VM_PGTABLE
+	bool "Debug arch page table for semantics compliance"
+	depends on MMU
+	depends on DEBUG_VM
+	depends on ARCH_HAS_DEBUG_VM_PGTABLE
+	help
+	  This option provides a debug method which can be used to test
+	  architecture page table helper functions on various platforms in
+	  verifying if they comply with expected generic MM semantics. This
+	  will help architecture code in making sure that any changes or
+	  new additions of these helpers still conform to expected
+	  semantics of the generic MM.
+
+	  If unsure, say N.
+
 config ARCH_HAS_DEBUG_VIRTUAL
 	bool
 
diff --git a/mm/Makefile b/mm/Makefile
index d996846697ef..2f085b971d34 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_HWPOISON_INJECT) += hwpoison-inject.o
 obj-$(CONFIG_DEBUG_KMEMLEAK) += kmemleak.o
 obj-$(CONFIG_DEBUG_KMEMLEAK_TEST) += kmemleak-test.o
 obj-$(CONFIG_DEBUG_RODATA_TEST) += rodata_test.o
+obj-$(CONFIG_DEBUG_VM_PGTABLE) += debug_vm_pgtable.o
 obj-$(CONFIG_PAGE_OWNER) += page_owner.o
 obj-$(CONFIG_CLEANCACHE) += cleancache.o
 obj-$(CONFIG_MEMORY_ISOLATION) += page_isolation.o
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
new file mode 100644
index 000000000000..621ac09e78b3
--- /dev/null
+++ b/mm/debug_vm_pgtable.c
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * This kernel test validates architecture page table helpers and
+ * accessors and helps in verifying their continued compliance with
+ * expected generic MM semantics.
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ *
+ * Author: Anshuman Khandual <anshuman.khandual@arm.com>
+ */
+#define pr_fmt(fmt) "debug_vm_pgtable: %s: " fmt, __func__
+
+#include <linux/gfp.h>
+#include <linux/highmem.h>
+#include <linux/hugetlb.h>
+#include <linux/kernel.h>
+#include <linux/kconfig.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+#include <linux/mm_types.h>
+#include <linux/module.h>
+#include <linux/pfn_t.h>
+#include <linux/printk.h>
+#include <linux/random.h>
+#include <linux/spinlock.h>
+#include <linux/swap.h>
+#include <linux/swapops.h>
+#include <linux/start_kernel.h>
+#include <linux/sched/mm.h>
+#include <asm/pgalloc.h>
+#include <asm/pgtable.h>
+
+/*
+ * Basic operations
+ *
+ * mkold(entry)			= An old and not a young entry
+ * mkyoung(entry)		= A young and not an old entry
+ * mkdirty(entry)		= A dirty and not a clean entry
+ * mkclean(entry)		= A clean and not a dirty entry
+ * mkwrite(entry)		= A write and not a write protected entry
+ * wrprotect(entry)		= A write protected and not a write entry
+ * pxx_bad(entry)		= A mapped and non-table entry
+ * pxx_same(entry1, entry2)	= Both entries hold the exact same value
+ */
+#define VMFLAGS	(VM_READ|VM_WRITE|VM_EXEC)
+
+/*
+ * On s390 platform, the lower 12 bits are used to identify given page table
+ * entry type and for other arch specific requirements. But these bits might
+ * affect the ability to clear entries with pxx_clear(). So while loading up
+ * the entries skip all lower 12 bits in order to accommodate s390 platform.
+ * It does not have affect any other platform.
+ */
+#define RANDOM_ORVALUE	(0xfffffffffffff000UL)
+#define RANDOM_NZVALUE	(0xff)
+
+static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
+{
+	pte_t pte = pfn_pte(pfn, prot);
+
+	WARN_ON(!pte_same(pte, pte));
+	WARN_ON(!pte_young(pte_mkyoung(pte)));
+	WARN_ON(!pte_dirty(pte_mkdirty(pte)));
+	WARN_ON(!pte_write(pte_mkwrite(pte)));
+	WARN_ON(pte_young(pte_mkold(pte)));
+	WARN_ON(pte_dirty(pte_mkclean(pte)));
+	WARN_ON(pte_write(pte_wrprotect(pte)));
+}
+
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
+static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
+{
+	pmd_t pmd = pfn_pmd(pfn, prot);
+
+	WARN_ON(!pmd_same(pmd, pmd));
+	WARN_ON(!pmd_young(pmd_mkyoung(pmd)));
+	WARN_ON(!pmd_dirty(pmd_mkdirty(pmd)));
+	WARN_ON(!pmd_write(pmd_mkwrite(pmd)));
+	WARN_ON(pmd_young(pmd_mkold(pmd)));
+	WARN_ON(pmd_dirty(pmd_mkclean(pmd)));
+	WARN_ON(pmd_write(pmd_wrprotect(pmd)));
+	/*
+	 * A huge page does not point to next level page table
+	 * entry. Hence this must qualify as pmd_bad().
+	 */
+	WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
+}
+#else
+static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
+{
+	pud_t pud = pfn_pud(pfn, prot);
+
+	WARN_ON(!pud_same(pud, pud));
+	WARN_ON(!pud_young(pud_mkyoung(pud)));
+	WARN_ON(!pud_write(pud_mkwrite(pud)));
+	WARN_ON(pud_write(pud_wrprotect(pud)));
+	WARN_ON(pud_young(pud_mkold(pud)));
+
+	if (mm_pmd_folded(mm) || __is_defined(ARCH_HAS_4LEVEL_HACK))
+		return;
+
+	/*
+	 * A huge page does not point to next level page table
+	 * entry. Hence this must qualify as pud_bad().
+	 */
+	WARN_ON(!pud_bad(pud_mkhuge(pud)));
+}
+#else
+static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
+
+static void __init p4d_basic_tests(unsigned long pfn, pgprot_t prot)
+{
+	p4d_t p4d;
+
+	memset(&p4d, RANDOM_NZVALUE, sizeof(p4d_t));
+	WARN_ON(!p4d_same(p4d, p4d));
+}
+
+static void __init pgd_basic_tests(unsigned long pfn, pgprot_t prot)
+{
+	pgd_t pgd;
+
+	memset(&pgd, RANDOM_NZVALUE, sizeof(pgd_t));
+	WARN_ON(!pgd_same(pgd, pgd));
+}
+
+#ifndef __ARCH_HAS_4LEVEL_HACK
+static void __init pud_clear_tests(struct mm_struct *mm, pud_t *pudp)
+{
+	pud_t pud = READ_ONCE(*pudp);
+
+	if (mm_pmd_folded(mm))
+		return;
+
+	pud = __pud(pud_val(pud) | RANDOM_ORVALUE);
+	WRITE_ONCE(*pudp, pud);
+	pud_clear(pudp);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(!pud_none(pud));
+}
+
+static void __init pud_populate_tests(struct mm_struct *mm, pud_t *pudp,
+				      pmd_t *pmdp)
+{
+	pud_t pud;
+
+	if (mm_pmd_folded(mm))
+		return;
+	/*
+	 * This entry points to next level page table page.
+	 * Hence this must not qualify as pud_bad().
+	 */
+	pmd_clear(pmdp);
+	pud_clear(pudp);
+	pud_populate(mm, pudp, pmdp);
+	pud = READ_ONCE(*pudp);
+	WARN_ON(pud_bad(pud));
+}
+#else
+static void __init pud_clear_tests(struct mm_struct *mm, pud_t *pudp) { }
+static void __init pud_populate_tests(struct mm_struct *mm, pud_t *pudp,
+				      pmd_t *pmdp)
+{
+}
+#endif
+
+#ifndef __ARCH_HAS_5LEVEL_HACK
+static void __init p4d_clear_tests(struct mm_struct *mm, p4d_t *p4dp)
+{
+	p4d_t p4d = READ_ONCE(*p4dp);
+
+	if (mm_pud_folded(mm))
+		return;
+
+	p4d = __p4d(p4d_val(p4d) | RANDOM_ORVALUE);
+	WRITE_ONCE(*p4dp, p4d);
+	p4d_clear(p4dp);
+	p4d = READ_ONCE(*p4dp);
+	WARN_ON(!p4d_none(p4d));
+}
+
+static void __init p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp,
+				      pud_t *pudp)
+{
+	p4d_t p4d;
+
+	if (mm_pud_folded(mm))
+		return;
+
+	/*
+	 * This entry points to next level page table page.
+	 * Hence this must not qualify as p4d_bad().
+	 */
+	pud_clear(pudp);
+	p4d_clear(p4dp);
+	p4d_populate(mm, p4dp, pudp);
+	p4d = READ_ONCE(*p4dp);
+	WARN_ON(p4d_bad(p4d));
+}
+
+static void __init pgd_clear_tests(struct mm_struct *mm, pgd_t *pgdp)
+{
+	pgd_t pgd = READ_ONCE(*pgdp);
+
+	if (mm_p4d_folded(mm))
+		return;
+
+	pgd = __pgd(pgd_val(pgd) | RANDOM_ORVALUE);
+	WRITE_ONCE(*pgdp, pgd);
+	pgd_clear(pgdp);
+	pgd = READ_ONCE(*pgdp);
+	WARN_ON(!pgd_none(pgd));
+}
+
+static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
+				      p4d_t *p4dp)
+{
+	pgd_t pgd;
+
+	if (mm_p4d_folded(mm))
+		return;
+
+	/*
+	 * This entry points to next level page table page.
+	 * Hence this must not qualify as pgd_bad().
+	 */
+	p4d_clear(p4dp);
+	pgd_clear(pgdp);
+	pgd_populate(mm, pgdp, p4dp);
+	pgd = READ_ONCE(*pgdp);
+	WARN_ON(pgd_bad(pgd));
+}
+#else
+static void __init p4d_clear_tests(struct mm_struct *mm, p4d_t *p4dp) { }
+static void __init pgd_clear_tests(struct mm_struct *mm, pgd_t *pgdp) { }
+static void __init p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp,
+				      pud_t *pudp)
+{
+}
+static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
+				      p4d_t *p4dp)
+{
+}
+#endif
+
+static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep)
+{
+	pte_t pte = READ_ONCE(*ptep);
+
+	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
+	WRITE_ONCE(*ptep, pte);
+	pte_clear(mm, 0, ptep);
+	pte = READ_ONCE(*ptep);
+	WARN_ON(!pte_none(pte));
+}
+
+static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp)
+{
+	pmd_t pmd = READ_ONCE(*pmdp);
+
+	pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE);
+	WRITE_ONCE(*pmdp, pmd);
+	pmd_clear(pmdp);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(!pmd_none(pmd));
+}
+
+static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
+				      pgtable_t pgtable)
+{
+	pmd_t pmd;
+
+	/*
+	 * This entry points to next level page table page.
+	 * Hence this must not qualify as pmd_bad().
+	 */
+	pmd_clear(pmdp);
+	pmd_populate(mm, pmdp, pgtable);
+	pmd = READ_ONCE(*pmdp);
+	WARN_ON(pmd_bad(pmd));
+}
+
+static unsigned long __init get_random_vaddr(void)
+{
+	unsigned long random_vaddr, random_pages, total_user_pages;
+
+	total_user_pages = (TASK_SIZE - FIRST_USER_ADDRESS) / PAGE_SIZE;
+
+	random_pages = get_random_long() % total_user_pages;
+	random_vaddr = FIRST_USER_ADDRESS + random_pages * PAGE_SIZE;
+
+	return random_vaddr;
+}
+
+void __init debug_vm_pgtable(void)
+{
+	struct mm_struct *mm;
+	pgd_t *pgdp;
+	p4d_t *p4dp, *saved_p4dp;
+	pud_t *pudp, *saved_pudp;
+	pmd_t *pmdp, *saved_pmdp, pmd;
+	pte_t *ptep;
+	pgtable_t saved_ptep;
+	pgprot_t prot;
+	phys_addr_t paddr;
+	unsigned long vaddr, pte_aligned, pmd_aligned;
+	unsigned long pud_aligned, p4d_aligned, pgd_aligned;
+
+	pr_info("Validating architecture page table helpers\n");
+	prot = vm_get_page_prot(VMFLAGS);
+	vaddr = get_random_vaddr();
+	mm = mm_alloc();
+	if (!mm) {
+		pr_err("mm_struct allocation failed\n");
+		return;
+	}
+
+	/*
+	 * PFN for mapping at PTE level is determined from a standard kernel
+	 * text symbol. But pfns for higher page table levels are derived by
+	 * masking lower bits of this real pfn. These derived pfns might not
+	 * exist on the platform but that does not really matter as pfn_pxx()
+	 * helpers will still create appropriate entries for the test. This
+	 * helps avoid large memory block allocations to be used for mapping
+	 * at higher page table levels.
+	 */
+	paddr = __pa(&start_kernel);
+
+	pte_aligned = (paddr & PAGE_MASK) >> PAGE_SHIFT;
+	pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT;
+	pud_aligned = (paddr & PUD_MASK) >> PAGE_SHIFT;
+	p4d_aligned = (paddr & P4D_MASK) >> PAGE_SHIFT;
+	pgd_aligned = (paddr & PGDIR_MASK) >> PAGE_SHIFT;
+	WARN_ON(!pfn_valid(pte_aligned));
+
+	pgdp = pgd_offset(mm, vaddr);
+	p4dp = p4d_alloc(mm, pgdp, vaddr);
+	pudp = pud_alloc(mm, p4dp, vaddr);
+	pmdp = pmd_alloc(mm, pudp, vaddr);
+	ptep = pte_alloc_map(mm, pmdp, vaddr);
+
+	/*
+	 * Save all the page table page addresses as the page table
+	 * entries will be used for testing with random or garbage
+	 * values. These saved addresses will be used for freeing
+	 * page table pages.
+	 */
+	pmd = READ_ONCE(*pmdp);
+	saved_p4dp = p4d_offset(pgdp, 0UL);
+	saved_pudp = pud_offset(p4dp, 0UL);
+	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);
+	p4d_basic_tests(p4d_aligned, prot);
+	pgd_basic_tests(pgd_aligned, prot);
+
+	pte_clear_tests(mm, ptep);
+	pmd_clear_tests(mm, pmdp);
+	pud_clear_tests(mm, pudp);
+	p4d_clear_tests(mm, p4dp);
+	pgd_clear_tests(mm, pgdp);
+
+	pte_unmap(ptep);
+
+	pmd_populate_tests(mm, pmdp, saved_ptep);
+	pud_populate_tests(mm, pudp, saved_pmdp);
+	p4d_populate_tests(mm, p4dp, saved_pudp);
+	pgd_populate_tests(mm, pgdp, saved_p4dp);
+
+	p4d_free(mm, saved_p4dp);
+	pud_free(mm, saved_pudp);
+	pmd_free(mm, saved_pmdp);
+	pte_free(mm, saved_ptep);
+
+	mm_dec_nr_puds(mm);
+	mm_dec_nr_pmds(mm);
+	mm_dec_nr_ptes(mm);
+	__mmdrop(mm);
+}
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-10-28  5:29 [PATCH V8] mm/debug: Add tests validating architecture page table helpers Anshuman Khandual
@ 2019-10-29 10:31 ` Qian Cai
  2019-11-04  2:15   ` Anshuman Khandual
  2019-11-05  0:00 ` Anshuman Khandual
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Qian Cai @ 2019-10-29 10:31 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Christophe Leroy,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



> On Oct 28, 2019, at 1:29 AM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
> 
> This adds tests which will validate architecture page table helpers and
> other accessors in their compliance with expected generic MM semantics.
> This will help various architectures in validating changes to existing
> page table helpers or addition of new ones.
> 
> This test covers basic page table entry transformations including but not
> limited to old, young, dirty, clean, write, write protect etc at various
> level along with populating intermediate entries with next page table page
> and validating them.
> 
> Test page table pages are allocated from system memory with required size
> and alignments. The mapped pfns at page table levels are derived from a
> real pfn representing a valid kernel text symbol. This test gets called
> right after page_alloc_init_late().
> 
> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
> arm64. Going forward, other architectures too can enable this after fixing
> build or runtime problems (if any) with their page table helpers.
> 
> Folks interested in making sure that a given platform's page table helpers
> conform to expected generic MM semantics should enable the above config
> which will just trigger this test during boot. Any non conformity here will
> be reported as an warning which would need to be fixed. This test will help
> catch any changes to the agreed upon semantics expected from generic MM and
> enable platforms to accommodate it thereafter.

This looks like a perfect candidate to streamline with the new kunit framework, no?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-10-29 10:31 ` Qian Cai
@ 2019-11-04  2:15   ` Anshuman Khandual
  0 siblings, 0 replies; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-04  2:15 UTC (permalink / raw)
  To: Qian Cai
  Cc: linux-mm, Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Christophe Leroy,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



On 10/29/2019 04:01 PM, Qian Cai wrote:
> 
> 
>> On Oct 28, 2019, at 1:29 AM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
>>
>> This adds tests which will validate architecture page table helpers and
>> other accessors in their compliance with expected generic MM semantics.
>> This will help various architectures in validating changes to existing
>> page table helpers or addition of new ones.
>>
>> This test covers basic page table entry transformations including but not
>> limited to old, young, dirty, clean, write, write protect etc at various
>> level along with populating intermediate entries with next page table page
>> and validating them.
>>
>> Test page table pages are allocated from system memory with required size
>> and alignments. The mapped pfns at page table levels are derived from a
>> real pfn representing a valid kernel text symbol. This test gets called
>> right after page_alloc_init_late().
>>
>> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
>> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
>> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
>> arm64. Going forward, other architectures too can enable this after fixing
>> build or runtime problems (if any) with their page table helpers.
>>
>> Folks interested in making sure that a given platform's page table helpers
>> conform to expected generic MM semantics should enable the above config
>> which will just trigger this test during boot. Any non conformity here will
>> be reported as an warning which would need to be fixed. This test will help
>> catch any changes to the agreed upon semantics expected from generic MM and
>> enable platforms to accommodate it thereafter.
> 
> This looks like a perfect candidate to streamline with the new kunit framework, no?

I have not been following the kunit test framework. But being highly dependent on
existing MM accessors (generic or platform) and very much page table modification
centric, mm/ is the best place for this test IMHO. It is now also part of DEBUG_VM
set of tests. Probably in future all existing MM tests (mm/ or lib/) might move to
kunit framework but for now it should remain with DEBUG_VM set of tests.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-10-28  5:29 [PATCH V8] mm/debug: Add tests validating architecture page table helpers Anshuman Khandual
  2019-10-29 10:31 ` Qian Cai
@ 2019-11-05  0:00 ` Anshuman Khandual
  2019-11-05 22:30   ` Vineet Gupta
  2019-11-05  0:30 ` Anshuman Khandual
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-05  0:00 UTC (permalink / raw)
  To: linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Christophe Leroy,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel, Michal Hocko, Matthew Wilcox



On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
> This adds tests which will validate architecture page table helpers and
> other accessors in their compliance with expected generic MM semantics.
> This will help various architectures in validating changes to existing
> page table helpers or addition of new ones.
> 
> This test covers basic page table entry transformations including but not
> limited to old, young, dirty, clean, write, write protect etc at various
> level along with populating intermediate entries with next page table page
> and validating them.
> 
> Test page table pages are allocated from system memory with required size
> and alignments. The mapped pfns at page table levels are derived from a
> real pfn representing a valid kernel text symbol. This test gets called
> right after page_alloc_init_late().
> 
> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
> arm64. Going forward, other architectures too can enable this after fixing
> build or runtime problems (if any) with their page table helpers.
> 
> Folks interested in making sure that a given platform's page table helpers
> conform to expected generic MM semantics should enable the above config
> which will just trigger this test during boot. Any non conformity here will
> be reported as an warning which would need to be fixed. This test will help
> catch any changes to the agreed upon semantics expected from generic MM and
> enable platforms to accommodate it thereafter.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steven Price <Steven.Price@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Sri Krishna chowdary <schowdary@nvidia.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: linux-snps-arc@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Tested-by: Christophe Leroy <christophe.leroy@c-s.fr>		#PPC32
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 
> This adds a test validation for architecture exported page table helpers.
> Patch adds basic transformation tests at various levels of the page table.
> 
> This test was originally suggested by Catalin during arm64 THP migration
> RFC discussion earlier. Going forward it can include more specific tests
> with respect to various generic MM functions like THP, HugeTLB etc and
> platform specific tests.
> 
> https://lore.kernel.org/linux-mm/20190628102003.GA56463@arrakis.emea.arm.com/
> 
> Needs to be applied on linux-next (next-20191025).
> 
> Changes in V8:
> 
> - Enabled ARCH_HAS_DEBUG_VM_PGTABLE on PPC32 platform per Christophe
> - Updated feature documentation as DEBUG_VM_PGTABLE is now enabled on PPC32 platform
> - Moved ARCH_HAS_DEBUG_VM_PGTABLE earlier to indent it with DEBUG_VM per Christophe
> - Added an information message in debug_vm_pgtable() per Christophe
> - Dropped random_vaddr boundary condition checks per Christophe and Qian
> - Replaced virt_addr_valid() check with pfn_valid() check in debug_vm_pgtable()

Hello Andrew,

Just wondering if this version looks okay or is there anything else which still
needs to be accommodated here first, before this test can be considered for merging ?
Thank you.

- Anshuman


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-10-28  5:29 [PATCH V8] mm/debug: Add tests validating architecture page table helpers Anshuman Khandual
  2019-10-29 10:31 ` Qian Cai
  2019-11-05  0:00 ` Anshuman Khandual
@ 2019-11-05  0:30 ` Anshuman Khandual
  2019-11-05 19:36 ` Gerald Schaefer
  2019-11-06  3:22 ` Anshuman Khandual
  4 siblings, 0 replies; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-05  0:30 UTC (permalink / raw)
  To: linux-mm
  Cc: Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Christophe Leroy,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel

On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
> This adds tests which will validate architecture page table helpers and
> other accessors in their compliance with expected generic MM semantics.
> This will help various architectures in validating changes to existing
> page table helpers or addition of new ones.
> 
> This test covers basic page table entry transformations including but not
> limited to old, young, dirty, clean, write, write protect etc at various
> level along with populating intermediate entries with next page table page
> and validating them.
> 
> Test page table pages are allocated from system memory with required size
> and alignments. The mapped pfns at page table levels are derived from a
> real pfn representing a valid kernel text symbol. This test gets called
> right after page_alloc_init_late().
> 
> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
> arm64. Going forward, other architectures too can enable this after fixing
> build or runtime problems (if any) with their page table helpers.
> 
> Folks interested in making sure that a given platform's page table helpers
> conform to expected generic MM semantics should enable the above config
> which will just trigger this test during boot. Any non conformity here will
> be reported as an warning which would need to be fixed. This test will help
> catch any changes to the agreed upon semantics expected from generic MM and
> enable platforms to accommodate it thereafter.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steven Price <Steven.Price@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Sri Krishna chowdary <schowdary@nvidia.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: linux-snps-arc@lists.infradead.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Tested-by: Christophe Leroy <christophe.leroy@c-s.fr>		#PPC32
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> 
> This adds a test validation for architecture exported page table helpers.
> Patch adds basic transformation tests at various levels of the page table.
> 
> This test was originally suggested by Catalin during arm64 THP migration
> RFC discussion earlier. Going forward it can include more specific tests
> with respect to various generic MM functions like THP, HugeTLB etc and
> platform specific tests.
> 
> https://lore.kernel.org/linux-mm/20190628102003.GA56463@arrakis.emea.arm.com/
> 
> Needs to be applied on linux-next (next-20191025).
> 
> Changes in V8:
> 
> - Enabled ARCH_HAS_DEBUG_VM_PGTABLE on PPC32 platform per Christophe
> - Updated feature documentation as DEBUG_VM_PGTABLE is now enabled on PPC32 platform
> - Moved ARCH_HAS_DEBUG_VM_PGTABLE earlier to indent it with DEBUG_VM per Christophe
> - Added an information message in debug_vm_pgtable() per Christophe
> - Dropped random_vaddr boundary condition checks per Christophe and Qian
> - Replaced virt_addr_valid() check with pfn_valid() check in debug_vm_pgtable()
> - Slightly changed pr_fmt(fmt) information
Hello Andrew,

Just wondering if this version looks okay or is there anything else which still
needs to be accommodated here first, before this test can be considered for merging ?
Thank you.

- Anshuman


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-10-28  5:29 [PATCH V8] mm/debug: Add tests validating architecture page table helpers Anshuman Khandual
                   ` (2 preceding siblings ...)
  2019-11-05  0:30 ` Anshuman Khandual
@ 2019-11-05 19:36 ` Gerald Schaefer
  2019-11-06  3:10   ` Anshuman Khandual
  2019-11-06  3:22 ` Anshuman Khandual
  4 siblings, 1 reply; 19+ messages in thread
From: Gerald Schaefer @ 2019-11-05 19:36 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Christophe Leroy, Ingo Molnar,
	linux-snps-arc, linux-mips, linux-arm-kernel, linux-ia64,
	linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel

On Mon, 28 Oct 2019 10:59:22 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:

> This adds tests which will validate architecture page table helpers and
> other accessors in their compliance with expected generic MM semantics.
> This will help various architectures in validating changes to existing
> page table helpers or addition of new ones.
> 
> This test covers basic page table entry transformations including but not
> limited to old, young, dirty, clean, write, write protect etc at various
> level along with populating intermediate entries with next page table page
> and validating them.
> 
> Test page table pages are allocated from system memory with required size
> and alignments. The mapped pfns at page table levels are derived from a
> real pfn representing a valid kernel text symbol. This test gets called
> right after page_alloc_init_late().
> 
> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
> arm64. Going forward, other architectures too can enable this after fixing
> build or runtime problems (if any) with their page table helpers.

I've prepared a couple of commits to our arch code to make this work on s390,
they will go upstream in the next merge window. After that, we can add s390
to the supported architectures.

We had some issues, e.g. because we do not report large entries as bad in
pxd_bad(), do not check for folded page tables in pxd_free(), or assume
that primitives like pmd_mkdirty() will only be called after pmd_mkhuge().
None of those should have any impact on current code, but your test module
revealed that we do not behave like other architectures in some aspects,
and it's good to find and fix such things to prevent possible future issues.

Thanks a lot for the effort!

Regards,
Gerald



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-05  0:00 ` Anshuman Khandual
@ 2019-11-05 22:30   ` Vineet Gupta
  2019-11-06  3:03     ` Anshuman Khandual
  0 siblings, 1 reply; 19+ messages in thread
From: Vineet Gupta @ 2019-11-05 22:30 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel

Hi Anshuman,

On 11/4/19 4:00 PM, Anshuman Khandual wrote:
> On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
>> This adds tests which will validate architecture page table helpers and
>> other accessors in their compliance with expected generic MM semantics.
>> This will help various architectures in validating changes to existing
>> page table helpers or addition of new ones.
>>
>> This test covers basic page table entry transformations including but not
>> limited to old, young, dirty, clean, write, write protect etc at various
>> level along with populating intermediate entries with next page table page
>> and validating them.
>>
>> Test page table pages are allocated from system memory with required size
>> and alignments. The mapped pfns at page table levels are derived from a
>> real pfn representing a valid kernel text symbol. This test gets called
>> right after page_alloc_init_late().
>>
>> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
>> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
>> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
>> arm64. Going forward, other architectures too can enable this after fixing
>> build or runtime problems (if any) with their page table helpers.
>>
>> Folks interested in making sure that a given platform's page table helpers
>> conform to expected generic MM semantics should enable the above config
>> which will just trigger this test during boot. Any non conformity here will
>> be reported as an warning which would need to be fixed. This test will help
>> catch any changes to the agreed upon semantics expected from generic MM and
>> enable platforms to accommodate it thereafter.
>>

I tried enabling this on ARC and ran into a build issue

../mm/debug_vm_pgtable.c: In function ‘pmd_basic_tests’:
../mm/debug_vm_pgtable.c:73:14: error: implicit declaration of function ‘pfn_pmd’;
did you mean ‘pfn_pte’? [-Werror=implicit-function-declaration]
  pmd_t pmd = pfn_pmd(pfn, prot);
              ^~~~~~~

The reason being THP was not enabled (although ARC supports THP) - for the
combination below
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
# CONFIG_TRANSPARENT_HUGEPAGE is not set

I think you need to use latter for guarding pmd_basic_tests()

Other than that the tests pass for !THP and THP too. So once fixed, you could
enable that for ARC as well
Thx for doing this.

-Vineet

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-05 22:30   ` Vineet Gupta
@ 2019-11-06  3:03     ` Anshuman Khandual
  2019-11-06 18:07       ` Vineet Gupta
  0 siblings, 1 reply; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-06  3:03 UTC (permalink / raw)
  To: Vineet Gupta, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel



On 11/06/2019 04:00 AM, Vineet Gupta wrote:
> Hi Anshuman,

Hello Vineet,

> 
> On 11/4/19 4:00 PM, Anshuman Khandual wrote:
>> On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
>>> This adds tests which will validate architecture page table helpers and
>>> other accessors in their compliance with expected generic MM semantics.
>>> This will help various architectures in validating changes to existing
>>> page table helpers or addition of new ones.
>>>
>>> This test covers basic page table entry transformations including but not
>>> limited to old, young, dirty, clean, write, write protect etc at various
>>> level along with populating intermediate entries with next page table page
>>> and validating them.
>>>
>>> Test page table pages are allocated from system memory with required size
>>> and alignments. The mapped pfns at page table levels are derived from a
>>> real pfn representing a valid kernel text symbol. This test gets called
>>> right after page_alloc_init_late().
>>>
>>> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
>>> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
>>> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
>>> arm64. Going forward, other architectures too can enable this after fixing
>>> build or runtime problems (if any) with their page table helpers.
>>>
>>> Folks interested in making sure that a given platform's page table helpers
>>> conform to expected generic MM semantics should enable the above config
>>> which will just trigger this test during boot. Any non conformity here will
>>> be reported as an warning which would need to be fixed. This test will help
>>> catch any changes to the agreed upon semantics expected from generic MM and
>>> enable platforms to accommodate it thereafter.
>>>
> 
> I tried enabling this on ARC and ran into a build issue
> 
> ../mm/debug_vm_pgtable.c: In function ‘pmd_basic_tests’:
> ../mm/debug_vm_pgtable.c:73:14: error: implicit declaration of function ‘pfn_pmd’;
> did you mean ‘pfn_pte’? [-Werror=implicit-function-declaration]
>   pmd_t pmd = pfn_pmd(pfn, prot);
>               ^~~~~~~
> 
> The reason being THP was not enabled (although ARC supports THP) - for the
> combination below
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
> # CONFIG_TRANSPARENT_HUGEPAGE is not set
> 
> I think you need to use latter for guarding pmd_basic_tests()

So the build complains that pfn_pmd() is not defined when the following config
combination is in place.

CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
CONFIG_TRANSPARENT_HUGEPAGE=n

But should not pfn_pmd() be encapsulated inside HAVE_ARCH_TRANSPARENT_HUGEPAGE
at the minimum (but I would say it should be available always, nonetheless) when
the platform subscribes to THP irrespective of whether THP is enabled or not.

I could see in the file (arch/arc/include/asm/pgtable.h) that fetching pfn_pmd()
and all other basic PMD definitions is conditional on CONFIG_TRANSPARENT_HUGEPAGE.

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
#include <asm/hugepage.h>
#endif

IIUC, CONFIG_TRANSPARENT_HUGEPAGE should only encapsulate PMD page table helpers
which are expected from generic THP code (pmd_trans_huge, pmdp_set_access_flags
etc) but not the basic PMD helpers like pmd_pfn, pmd_mkyoung, pmd_mkdirty,
pmd_mkclean etc. Hence wondering will it be possible to accommodate following
code change on arc platform (not even compiled) in order to fix the problem ?

diff --git a/arch/arc/include/asm/hugepage.h b/arch/arc/include/asm/hugepage.h
index 9a74ce7..2ae15a8 100644
--- a/arch/arc/include/asm/hugepage.h
+++ b/arch/arc/include/asm/hugepage.h
@@ -36,11 +36,11 @@ static inline pmd_t pte_pmd(pte_t pte)
 #define pmd_dirty(pmd)         pte_dirty(pmd_pte(pmd))
 
 #define mk_pmd(page, prot)     pte_pmd(mk_pte(page, prot))
+#define pfn_pmd(pfn, prot)     (__pmd(((pfn) << PAGE_SHIFT) | pgprot_val(prot)))
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 #define pmd_trans_huge(pmd)    (pmd_val(pmd) & _PAGE_HW_SZ)
 
-#define pfn_pmd(pfn, prot)     (__pmd(((pfn) << PAGE_SHIFT) | pgprot_val(prot)))
-
 static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
 {
         /*
@@ -73,5 +73,6 @@ extern void flush_pmd_tlb_range(struct vm_area_struct *vma, unsigned long start,
 
 /* We don't have hardware dirty/accessed bits, generic_pmdp_establish is fine.*/
 #define pmdp_establish generic_pmdp_establish
+#endif
 
 #endif
diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h
index 9019ed9..20395f1 100644
--- a/arch/arc/include/asm/pgtable.h
+++ b/arch/arc/include/asm/pgtable.h
@@ -385,7 +385,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
  * remap a physical page `pfn' of size `size' with page protection `prot'
  * into virtual address `from'
  */
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
 #include <asm/hugepage.h>
 #endif

> 
> Other than that the tests pass for !THP and THP too. So once fixed, you could

Glad that it works on arc platform as well.

> enable that for ARC as well> Thx for doing this.
> 
> -Vineet
> 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-05 19:36 ` Gerald Schaefer
@ 2019-11-06  3:10   ` Anshuman Khandual
  0 siblings, 0 replies; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-06  3:10 UTC (permalink / raw)
  To: Gerald Schaefer
  Cc: linux-mm, Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Christophe Leroy, Ingo Molnar,
	linux-snps-arc, linux-mips, linux-arm-kernel, linux-ia64,
	linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



On 11/06/2019 01:06 AM, Gerald Schaefer wrote:
> On Mon, 28 Oct 2019 10:59:22 +0530
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
>> This adds tests which will validate architecture page table helpers and
>> other accessors in their compliance with expected generic MM semantics.
>> This will help various architectures in validating changes to existing
>> page table helpers or addition of new ones.
>>
>> This test covers basic page table entry transformations including but not
>> limited to old, young, dirty, clean, write, write protect etc at various
>> level along with populating intermediate entries with next page table page
>> and validating them.
>>
>> Test page table pages are allocated from system memory with required size
>> and alignments. The mapped pfns at page table levels are derived from a
>> real pfn representing a valid kernel text symbol. This test gets called
>> right after page_alloc_init_late().
>>
>> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
>> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
>> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
>> arm64. Going forward, other architectures too can enable this after fixing
>> build or runtime problems (if any) with their page table helpers.
> 
> I've prepared a couple of commits to our arch code to make this work on s390,
> they will go upstream in the next merge window. After that, we can add s390
> to the supported architectures.

Thats good.

> 
> We had some issues, e.g. because we do not report large entries as bad in
> pxd_bad(), do not check for folded page tables in pxd_free(), or assume
> that primitives like pmd_mkdirty() will only be called after pmd_mkhuge().
> None of those should have any impact on current code, but your test module
> revealed that we do not behave like other architectures in some aspects,
> and it's good to find and fix such things to prevent possible future issues.

Right and those s390 fixes are the testimony for the usefulness of this test.

> 
> Thanks a lot for the effort!
> 
> Regards,
> Gerald
> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-10-28  5:29 [PATCH V8] mm/debug: Add tests validating architecture page table helpers Anshuman Khandual
                   ` (3 preceding siblings ...)
  2019-11-05 19:36 ` Gerald Schaefer
@ 2019-11-06  3:22 ` Anshuman Khandual
  2019-11-06  6:41   ` Christophe Leroy
  4 siblings, 1 reply; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-06  3:22 UTC (permalink / raw)
  To: linux-mm
  Cc: Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Christophe Leroy,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
> +    -----------------------
> +    |         arch |status|
> +    -----------------------
> +    |       alpha: | TODO |
> +    |         arc: | TODO |
> +    |         arm: | TODO |
> +    |       arm64: |  ok  |
> +    |         c6x: | TODO |
> +    |        csky: | TODO |
> +    |       h8300: | TODO |
> +    |     hexagon: | TODO |
> +    |        ia64: | TODO |
> +    |        m68k: | TODO |
> +    |  microblaze: | TODO |
> +    |        mips: | TODO |
> +    |       nds32: | TODO |
> +    |       nios2: | TODO |
> +    |    openrisc: | TODO |
> +    |      parisc: | TODO |
> +    |     powerpc: | TODO |
> +    |       ppc32: |  ok  |
> +    |       riscv: | TODO |
> +    |        s390: | TODO |
> +    |          sh: | TODO |
> +    |       sparc: | TODO |
> +    |          um: | TODO |
> +    |   unicore32: | TODO |
> +    |         x86: |  ok  |
> +    |      xtensa: | TODO |
> +    -----------------------

While here, are there some volunteers to test this on any of the
'yet to be tested and supported' platforms ?

- Anshuman


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-06  3:22 ` Anshuman Khandual
@ 2019-11-06  6:41   ` Christophe Leroy
  2019-11-06  7:34     ` Anshuman Khandual
  0 siblings, 1 reply; 19+ messages in thread
From: Christophe Leroy @ 2019-11-06  6:41 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm
  Cc: Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Ingo Molnar,
	linux-snps-arc, linux-mips, linux-arm-kernel, linux-ia64,
	linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



Le 06/11/2019 à 04:22, Anshuman Khandual a écrit :
> 
> 
> On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
>> +    -----------------------
>> +    |         arch |status|
>> +    -----------------------
>> +    |       alpha: | TODO |
>> +    |         arc: | TODO |
>> +    |         arm: | TODO |
>> +    |       arm64: |  ok  |
>> +    |         c6x: | TODO |
>> +    |        csky: | TODO |
>> +    |       h8300: | TODO |
>> +    |     hexagon: | TODO |
>> +    |        ia64: | TODO |
>> +    |        m68k: | TODO |
>> +    |  microblaze: | TODO |
>> +    |        mips: | TODO |
>> +    |       nds32: | TODO |
>> +    |       nios2: | TODO |
>> +    |    openrisc: | TODO |
>> +    |      parisc: | TODO |
>> +    |     powerpc: | TODO |
>> +    |       ppc32: |  ok  |

Note that ppc32 is a part of powerpc, not a standalone arch.

Maybe something like the following would be more correct:
|  powerpc/32: |  ok  |
|  powerpc/64: | TODO |

Christophe

>> +    |       riscv: | TODO |
>> +    |        s390: | TODO |
>> +    |          sh: | TODO |
>> +    |       sparc: | TODO |
>> +    |          um: | TODO |
>> +    |   unicore32: | TODO |
>> +    |         x86: |  ok  |
>> +    |      xtensa: | TODO |
>> +    -----------------------
> 
> While here, are there some volunteers to test this on any of the
> 'yet to be tested and supported' platforms ?
> 
> - Anshuman
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-06  6:41   ` Christophe Leroy
@ 2019-11-06  7:34     ` Anshuman Khandual
  2019-11-07 12:54       ` Michael Ellerman
  0 siblings, 1 reply; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-06  7:34 UTC (permalink / raw)
  To: Christophe Leroy, linux-mm
  Cc: Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller,
	Vineet Gupta, James Hogan, Paul Burton, Ralf Baechle,
	Kirill A . Shutemov, Gerald Schaefer, Ingo Molnar,
	linux-snps-arc, linux-mips, linux-arm-kernel, linux-ia64,
	linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



On 11/06/2019 12:11 PM, Christophe Leroy wrote:
> 
> 
> Le 06/11/2019 à 04:22, Anshuman Khandual a écrit :
>>
>>
>> On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
>>> +    -----------------------
>>> +    |         arch |status|
>>> +    -----------------------
>>> +    |       alpha: | TODO |
>>> +    |         arc: | TODO |
>>> +    |         arm: | TODO |
>>> +    |       arm64: |  ok  |
>>> +    |         c6x: | TODO |
>>> +    |        csky: | TODO |
>>> +    |       h8300: | TODO |
>>> +    |     hexagon: | TODO |
>>> +    |        ia64: | TODO |
>>> +    |        m68k: | TODO |
>>> +    |  microblaze: | TODO |
>>> +    |        mips: | TODO |
>>> +    |       nds32: | TODO |
>>> +    |       nios2: | TODO |
>>> +    |    openrisc: | TODO |
>>> +    |      parisc: | TODO |
>>> +    |     powerpc: | TODO |
>>> +    |       ppc32: |  ok  |
> 
> Note that ppc32 is a part of powerpc, not a standalone arch.

Right, I understand. But we are yet to hear about how this test
came about on powerpc server platforms. Will update 'powerpc'
arch listing above once we get some confirmation. May be once
this works on all relevant powerpc platforms, we can just merge
'powerpc' and 'ppc32' entries here as just 'powerpc'.

> 
> Maybe something like the following would be more correct:
> |  powerpc/32: |  ok  |
> |  powerpc/64: | TODO |
> 
> Christophe
> 
>>> +    |       riscv: | TODO |
>>> +    |        s390: | TODO |
>>> +    |          sh: | TODO |
>>> +    |       sparc: | TODO |
>>> +    |          um: | TODO |
>>> +    |   unicore32: | TODO |
>>> +    |         x86: |  ok  |
>>> +    |      xtensa: | TODO |
>>> +    -----------------------
>>
>> While here, are there some volunteers to test this on any of the
>> 'yet to be tested and supported' platforms ?
>>
>> - Anshuman
>>
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-06  3:03     ` Anshuman Khandual
@ 2019-11-06 18:07       ` Vineet Gupta
  2019-11-07  4:44         ` Anshuman Khandual
  0 siblings, 1 reply; 19+ messages in thread
From: Vineet Gupta @ 2019-11-06 18:07 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel

On 11/5/19 7:03 PM, Anshuman Khandual wrote:
> But should not pfn_pmd() be encapsulated inside HAVE_ARCH_TRANSPARENT_HUGEPAGE
> at the minimum (but I would say it should be available always, nonetheless) when
> the platform subscribes to THP irrespective of whether THP is enabled or not.

For ARC it was only introduced/needed when I added THP support so it is dependent
in some way.

> I could see in the file (arch/arc/include/asm/pgtable.h) that fetching pfn_pmd()
> and all other basic PMD definitions is conditional on CONFIG_TRANSPARENT_HUGEPAGE.
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> #include <asm/hugepage.h>
> #endif
>
> IIUC, CONFIG_TRANSPARENT_HUGEPAGE should only encapsulate PMD page table helpers
> which are expected from generic THP code (pmd_trans_huge, pmdp_set_access_flags
> etc) but not the basic PMD helpers like pmd_pfn, pmd_mkyoung, pmd_mkdirty,
> pmd_mkclean etc. 

ARC only has 2 levels of paging, so these don't make any sense in general and
needed only for THP case.
I case of arch/arm you see it is only defined in pgtable-3level.h

> Hence wondering will it be possible to accommodate following
> code change on arc platform (not even compiled) in order to fix the problem ?

I'm open to making changes in ARC code but lets do the right thing.

>   */
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>  #include <asm/hugepage.h>
>  #endif

This in wrong.  CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE is a just a glue toggle,
used only in Kconfig files (and not in any "C" code).  It enables generic Kconfig
code to allow visibility of CONFIG_TRANSPARENT_HUGEPAGE w/o every arch needing to
do a me too.

I think you need to use CONFIG_TRANSPARENT_HUGEPAGE to guard appropriate tests. I
understand that it only

-Vineet


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-06 18:07       ` Vineet Gupta
@ 2019-11-07  4:44         ` Anshuman Khandual
  2019-11-07 19:05           ` Vineet Gupta
  0 siblings, 1 reply; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-07  4:44 UTC (permalink / raw)
  To: Vineet Gupta, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel



On 11/06/2019 11:37 PM, Vineet Gupta wrote:
> On 11/5/19 7:03 PM, Anshuman Khandual wrote:
>> But should not pfn_pmd() be encapsulated inside HAVE_ARCH_TRANSPARENT_HUGEPAGE
>> at the minimum (but I would say it should be available always, nonetheless) when
>> the platform subscribes to THP irrespective of whether THP is enabled or not.
> 
> For ARC it was only introduced/needed when I added THP support so it is dependent
> in some way.
Right, it is dependent.

> 
>> I could see in the file (arch/arc/include/asm/pgtable.h) that fetching pfn_pmd()
>> and all other basic PMD definitions is conditional on CONFIG_TRANSPARENT_HUGEPAGE.
>>
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> #include <asm/hugepage.h>
>> #endif
>>
>> IIUC, CONFIG_TRANSPARENT_HUGEPAGE should only encapsulate PMD page table helpers
>> which are expected from generic THP code (pmd_trans_huge, pmdp_set_access_flags
>> etc) but not the basic PMD helpers like pmd_pfn, pmd_mkyoung, pmd_mkdirty,
>> pmd_mkclean etc. 
> 
> ARC only has 2 levels of paging, so these don't make any sense in general and
> needed only for THP case.
> I case of arch/arm you see it is only defined in pgtable-3level.h

There is no uniformity for all these across architectures. It has been bit
difficult to get some of these required helpers right (compile and run) on
different platforms.

> 
>> Hence wondering will it be possible to accommodate following
>> code change on arc platform (not even compiled) in order to fix the problem ?
> 
> I'm open to making changes in ARC code but lets do the right thing.
> 
>>   */
>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>>  #include <asm/hugepage.h>
>>  #endif
> 
> This in wrong.  CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE is a just a glue toggle,
> used only in Kconfig files (and not in any "C" code).  It enables generic Kconfig
> code to allow visibility of CONFIG_TRANSPARENT_HUGEPAGE w/o every arch needing to
> do a me too.
> 
> I think you need to use CONFIG_TRANSPARENT_HUGEPAGE to guard appropriate tests. I
> understand that it only

We can probably replace CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE wrapper with
CONFIG_TRANSPARENT_HUGEPAGE. But CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
explicitly depends on CONFIG_TRANSPARENT_HUGEPAGE as a prerequisite. Could
you please confirm if the following change on this test will work on ARC
platform for both THP and !THP cases ? Thank you.

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 621ac09..99ebc7c 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -67,7 +67,7 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
 	WARN_ON(pte_write(pte_wrprotect(pte)));
 }
 
-#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
 {
 	pmd_t pmd = pfn_pmd(pfn, prot);
@@ -85,9 +85,6 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
 	 */
 	WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
 }
-#else
-static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
-#endif
 
 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
 static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
@@ -112,6 +109,10 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
 #else
 static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
 #endif
+#else
+static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
+#endif
 
 static void __init p4d_basic_tests(unsigned long pfn, pgprot_t prot)
 {

> -Vineet
> 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-06  7:34     ` Anshuman Khandual
@ 2019-11-07 12:54       ` Michael Ellerman
  2019-11-07 13:22         ` Anshuman Khandual
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Ellerman @ 2019-11-07 12:54 UTC (permalink / raw)
  To: Anshuman Khandual, Christophe Leroy, linux-mm
  Cc: Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Paul Mackerras, Martin Schwidefsky,
	Heiko Carstens, David S. Miller, Vineet Gupta, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel

Anshuman Khandual <anshuman.khandual@arm.com> writes:
> On 11/06/2019 12:11 PM, Christophe Leroy wrote:
>> Le 06/11/2019 à 04:22, Anshuman Khandual a écrit :
>>> On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
>>>> +    -----------------------
>>>> +    |         arch |status|
>>>> +    -----------------------
>>>> +    |       alpha: | TODO |
>>>> +    |         arc: | TODO |
>>>> +    |         arm: | TODO |
>>>> +    |       arm64: |  ok  |
>>>> +    |         c6x: | TODO |
>>>> +    |        csky: | TODO |
>>>> +    |       h8300: | TODO |
>>>> +    |     hexagon: | TODO |
>>>> +    |        ia64: | TODO |
>>>> +    |        m68k: | TODO |
>>>> +    |  microblaze: | TODO |
>>>> +    |        mips: | TODO |
>>>> +    |       nds32: | TODO |
>>>> +    |       nios2: | TODO |
>>>> +    |    openrisc: | TODO |
>>>> +    |      parisc: | TODO |
>>>> +    |     powerpc: | TODO |
>>>> +    |       ppc32: |  ok  |
>> 
>> Note that ppc32 is a part of powerpc, not a standalone arch.
>
> Right, I understand. But we are yet to hear about how this test
> came about on powerpc server platforms. Will update 'powerpc'
> arch listing above once we get some confirmation. May be once
> this works on all relevant powerpc platforms, we can just merge
> 'powerpc' and 'ppc32' entries here as just 'powerpc'.

On pseries:

  watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [swapper/0:1]
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty #152
  NIP:  c0000000010435a0 LR: c0000000010434b4 CTR: 0000000000000000
  REGS: c00000003a403980 TRAP: 0901   Not tainted  (5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty)
  MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 44000222  XER: 00000000
  CFAR: c0000000010435a8 IRQMASK: 0 
  GPR00: c0000000010434b4 c00000003a403c10 c000000001295000 05210001000000c0 
  GPR04: 8000000000000105 0000000000400dc0 000000003eb00000 0000000000000001 
  GPR08: 0000000000000000 ffffffffffffffff 0000000000000001 0000000000000100 
  GPR12: 0000000000000000 c0000000018f0000 
  NIP [c0000000010435a0] debug_vm_pgtable+0x43c/0x82c
  LR [c0000000010434b4] debug_vm_pgtable+0x350/0x82c
  Call Trace:
  [c00000003a403c10] [c00000000104346c] debug_vm_pgtable+0x308/0x82c (unreliable)
  [c00000003a403ce0] [c000000001004310] kernel_init_freeable+0x1d0/0x39c
  [c00000003a403db0] [c000000000010da0] kernel_init+0x24/0x174
  [c00000003a403e20] [c00000000000bdc4] ret_from_kernel_thread+0x5c/0x78
  Instruction dump:
  7d075078 7ce74b78 7ce0f9ad 40c2fff0 38800000 7f83e378 4b02eee1 60000000 
  48000080 3920ffff 39400001 39000000 <7ea0f8a8> 7ea75039 40c2fff8 7ea74878 

Looking at the asm I think it's stuck in hash__pte_update() waiting for
H_PAGE_BUSY to clear, but not sure why.

That's just using qemu TCG, instructions here if anyone wants to test it
themselves :)

  https://github.com/linuxppc/wiki/wiki/Booting-with-Qemu


If I boot with -cpu power9 (using Radix MMU), I get a plain old BUG:

  debug_vm_pgtable: debug_vm_pgtable: Validating architecture page table helpers
  ------------[ cut here ]------------
  kernel BUG at arch/powerpc/mm/pgtable.c:274!
  Oops: Exception in kernel mode, sig: 5 [#1]
  LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=32 NUMA pSeries
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty #152
  NIP:  c0000000000724e8 LR: c00000000104358c CTR: 0000000000000000
  REGS: c00000003a483980 TRAP: 0700   Not tainted  (5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty)
  MSR:  8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 24000224  XER: 20000000
  CFAR: c000000001043588 IRQMASK: 0 
  GPR00: c00000000104358c c00000003a483c10 c000000001295000 0000000000000009 
  GPR04: 0000000000000000 0000000000000005 0000000000000000 0000000000000009 
  GPR08: 0000000000000001 000000000000000e 0000000000000001 c00000003a5f0000 
  GPR12: 0000000000000000 c0000000018f0000 c000000000010d84 0000000000000000 
  GPR16: 0000000000000000 0000000000000000 c00000003a5f0000 8000000000000105 
  GPR20: c000000001003ab8 0000000000000015 0500613a00000080 0900603a00000080 
  GPR24: 09202e3a00000080 c00000000133bd90 c00000000133bd98 c00000000133bda0 
  GPR28: c00000003a5e0000 c00000003a600af8 c00000003a2e2d48 c00000003a6100a0 
  NIP [c0000000000724e8] assert_pte_locked+0x88/0x190
  LR [c00000000104358c] debug_vm_pgtable+0x428/0x82c
  Call Trace:
  [c00000003a483c10] [c00000000104346c] debug_vm_pgtable+0x308/0x82c (unreliable)
  [c00000003a483ce0] [c000000001004310] kernel_init_freeable+0x1d0/0x39c
  [c00000003a483db0] [c000000000010da0] kernel_init+0x24/0x174
  [c00000003a483e20] [c00000000000bdc4] ret_from_kernel_thread+0x5c/0x78
  Instruction dump:
  7d251a14 39070010 7d463030 7d084a14 38c6ffff 7c884436 7cc607b4 7d083038 
  79081f24 7ccb402a 7cc80074 7908d182 <0b080000> 78cb0022 54c8c03e 7d473830 
  ---[ end trace a694f1bc56529c0e ]---


cheers

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-07 12:54       ` Michael Ellerman
@ 2019-11-07 13:22         ` Anshuman Khandual
  0 siblings, 0 replies; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-07 13:22 UTC (permalink / raw)
  To: Michael Ellerman, Christophe Leroy, linux-mm
  Cc: Andrew Morton, Vlastimil Babka, Greg Kroah-Hartman,
	Thomas Gleixner, Mike Rapoport, Jason Gunthorpe, Dan Williams,
	Peter Zijlstra, Michal Hocko, Mark Rutland, Mark Brown,
	Steven Price, Ard Biesheuvel, Masahiro Yamada, Kees Cook,
	Tetsuo Handa, Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Paul Mackerras, Martin Schwidefsky,
	Heiko Carstens, David S. Miller, Vineet Gupta, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Ingo Molnar, linux-snps-arc, linux-mips, linux-arm-kernel,
	linux-ia64, linuxppc-dev, linux-s390, linux-sh, sparclinux, x86,
	linux-kernel



On 11/07/2019 06:24 PM, Michael Ellerman wrote:
> Anshuman Khandual <anshuman.khandual@arm.com> writes:
>> On 11/06/2019 12:11 PM, Christophe Leroy wrote:
>>> Le 06/11/2019 à 04:22, Anshuman Khandual a écrit :
>>>> On 10/28/2019 10:59 AM, Anshuman Khandual wrote:
>>>>> +    -----------------------
>>>>> +    |         arch |status|
>>>>> +    -----------------------
>>>>> +    |       alpha: | TODO |
>>>>> +    |         arc: | TODO |
>>>>> +    |         arm: | TODO |
>>>>> +    |       arm64: |  ok  |
>>>>> +    |         c6x: | TODO |
>>>>> +    |        csky: | TODO |
>>>>> +    |       h8300: | TODO |
>>>>> +    |     hexagon: | TODO |
>>>>> +    |        ia64: | TODO |
>>>>> +    |        m68k: | TODO |
>>>>> +    |  microblaze: | TODO |
>>>>> +    |        mips: | TODO |
>>>>> +    |       nds32: | TODO |
>>>>> +    |       nios2: | TODO |
>>>>> +    |    openrisc: | TODO |
>>>>> +    |      parisc: | TODO |
>>>>> +    |     powerpc: | TODO |
>>>>> +    |       ppc32: |  ok  |
>>>
>>> Note that ppc32 is a part of powerpc, not a standalone arch.
>>
>> Right, I understand. But we are yet to hear about how this test
>> came about on powerpc server platforms. Will update 'powerpc'
>> arch listing above once we get some confirmation. May be once
>> this works on all relevant powerpc platforms, we can just merge
>> 'powerpc' and 'ppc32' entries here as just 'powerpc'.
> 
> On pseries:
> 
>   watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [swapper/0:1]
>   Modules linked in:
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty #152
>   NIP:  c0000000010435a0 LR: c0000000010434b4 CTR: 0000000000000000
>   REGS: c00000003a403980 TRAP: 0901   Not tainted  (5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty)
>   MSR:  8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 44000222  XER: 00000000
>   CFAR: c0000000010435a8 IRQMASK: 0 
>   GPR00: c0000000010434b4 c00000003a403c10 c000000001295000 05210001000000c0 
>   GPR04: 8000000000000105 0000000000400dc0 000000003eb00000 0000000000000001 
>   GPR08: 0000000000000000 ffffffffffffffff 0000000000000001 0000000000000100 
>   GPR12: 0000000000000000 c0000000018f0000 
>   NIP [c0000000010435a0] debug_vm_pgtable+0x43c/0x82c
>   LR [c0000000010434b4] debug_vm_pgtable+0x350/0x82c
>   Call Trace:
>   [c00000003a403c10] [c00000000104346c] debug_vm_pgtable+0x308/0x82c (unreliable)
>   [c00000003a403ce0] [c000000001004310] kernel_init_freeable+0x1d0/0x39c
>   [c00000003a403db0] [c000000000010da0] kernel_init+0x24/0x174
>   [c00000003a403e20] [c00000000000bdc4] ret_from_kernel_thread+0x5c/0x78
>   Instruction dump:
>   7d075078 7ce74b78 7ce0f9ad 40c2fff0 38800000 7f83e378 4b02eee1 60000000 
>   48000080 3920ffff 39400001 39000000 <7ea0f8a8> 7ea75039 40c2fff8 7ea74878 
> 
> Looking at the asm I think it's stuck in hash__pte_update() waiting for
> H_PAGE_BUSY to clear, but not sure why.
> 
> That's just using qemu TCG, instructions here if anyone wants to test it
> themselves :)
> 
>   https://github.com/linuxppc/wiki/wiki/Booting-with-Qemu
> 
> 
> If I boot with -cpu power9 (using Radix MMU), I get a plain old BUG:
> 
>   debug_vm_pgtable: debug_vm_pgtable: Validating architecture page table helpers
>   ------------[ cut here ]------------
>   kernel BUG at arch/powerpc/mm/pgtable.c:274!
>   Oops: Exception in kernel mode, sig: 5 [#1]
>   LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=32 NUMA pSeries
>   Modules linked in:
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty #152
>   NIP:  c0000000000724e8 LR: c00000000104358c CTR: 0000000000000000
>   REGS: c00000003a483980 TRAP: 0700   Not tainted  (5.4.0-rc6-gcc-8.2.0-next-20191107-00001-g250339d6747b-dirty)
>   MSR:  8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 24000224  XER: 20000000
>   CFAR: c000000001043588 IRQMASK: 0 
>   GPR00: c00000000104358c c00000003a483c10 c000000001295000 0000000000000009 
>   GPR04: 0000000000000000 0000000000000005 0000000000000000 0000000000000009 
>   GPR08: 0000000000000001 000000000000000e 0000000000000001 c00000003a5f0000 
>   GPR12: 0000000000000000 c0000000018f0000 c000000000010d84 0000000000000000 
>   GPR16: 0000000000000000 0000000000000000 c00000003a5f0000 8000000000000105 
>   GPR20: c000000001003ab8 0000000000000015 0500613a00000080 0900603a00000080 
>   GPR24: 09202e3a00000080 c00000000133bd90 c00000000133bd98 c00000000133bda0 
>   GPR28: c00000003a5e0000 c00000003a600af8 c00000003a2e2d48 c00000003a6100a0 
>   NIP [c0000000000724e8] assert_pte_locked+0x88/0x190
>   LR [c00000000104358c] debug_vm_pgtable+0x428/0x82c
>   Call Trace:
>   [c00000003a483c10] [c00000000104346c] debug_vm_pgtable+0x308/0x82c (unreliable)
>   [c00000003a483ce0] [c000000001004310] kernel_init_freeable+0x1d0/0x39c
>   [c00000003a483db0] [c000000000010da0] kernel_init+0x24/0x174
>   [c00000003a483e20] [c00000000000bdc4] ret_from_kernel_thread+0x5c/0x78
>   Instruction dump:
>   7d251a14 39070010 7d463030 7d084a14 38c6ffff 7c884436 7cc607b4 7d083038 
>   79081f24 7ccb402a 7cc80074 7908d182 <0b080000> 78cb0022 54c8c03e 7d473830 
>   ---[ end trace a694f1bc56529c0e ]---

Oops. Does not seem like a quick problem to fix :) Though assert_pte_locked()
gets checked only when DEBUG_VM is enabled. Probably will have to keep this
test disabled on powerpc for now.

> 
> 
> cheers
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-07  4:44         ` Anshuman Khandual
@ 2019-11-07 19:05           ` Vineet Gupta
  2019-11-08  4:27             ` Anshuman Khandual
  0 siblings, 1 reply; 19+ messages in thread
From: Vineet Gupta @ 2019-11-07 19:05 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel

On 11/6/19 8:44 PM, Anshuman Khandual wrote:
>
>>
>>>   */
>>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>>>  #include <asm/hugepage.h>
>>>  #endif
>> This in wrong.  CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE is a just a glue toggle,
>> used only in Kconfig files (and not in any "C" code).  It enables generic Kconfig
>> code to allow visibility of CONFIG_TRANSPARENT_HUGEPAGE w/o every arch needing to
>> do a me too.
>>
>> I think you need to use CONFIG_TRANSPARENT_HUGEPAGE to guard appropriate tests. I
>> understand that it only
> We can probably replace CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE wrapper with
> CONFIG_TRANSPARENT_HUGEPAGE. But CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> explicitly depends on CONFIG_TRANSPARENT_HUGEPAGE as a prerequisite. Could
> you please confirm if the following change on this test will work on ARC
> platform for both THP and !THP cases ? Thank you.
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 621ac09..99ebc7c 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -67,7 +67,7 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>  	WARN_ON(pte_write(pte_wrprotect(pte)));
>  }
>  
> -#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
>  {
>  	pmd_t pmd = pfn_pmd(pfn, prot);
> @@ -85,9 +85,6 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
>  	 */
>  	WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
>  }
> -#else
> -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
> -#endif
>  
>  #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>  static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
> @@ -112,6 +109,10 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
>  #else
>  static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
>  #endif
> +#else
> +static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
> +static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> +#endif

Fails to build for THP case since

CONFIG_TRANSPARENT_HUGEPAGE=y
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD=n

../mm/debug_vm_pgtable.c:112:20: error: redefinition of ‘pmd_basic_tests’

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-07 19:05           ` Vineet Gupta
@ 2019-11-08  4:27             ` Anshuman Khandual
  2019-11-11 19:06               ` Vineet Gupta
  0 siblings, 1 reply; 19+ messages in thread
From: Anshuman Khandual @ 2019-11-08  4:27 UTC (permalink / raw)
  To: Vineet Gupta, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel



On 11/08/2019 12:35 AM, Vineet Gupta wrote:
> On 11/6/19 8:44 PM, Anshuman Khandual wrote:
>>
>>>
>>>>   */
>>>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>>>>  #include <asm/hugepage.h>
>>>>  #endif
>>> This in wrong.  CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE is a just a glue toggle,
>>> used only in Kconfig files (and not in any "C" code).  It enables generic Kconfig
>>> code to allow visibility of CONFIG_TRANSPARENT_HUGEPAGE w/o every arch needing to
>>> do a me too.
>>>
>>> I think you need to use CONFIG_TRANSPARENT_HUGEPAGE to guard appropriate tests. I
>>> understand that it only
>> We can probably replace CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE wrapper with
>> CONFIG_TRANSPARENT_HUGEPAGE. But CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>> explicitly depends on CONFIG_TRANSPARENT_HUGEPAGE as a prerequisite. Could
>> you please confirm if the following change on this test will work on ARC
>> platform for both THP and !THP cases ? Thank you.
>>
>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>> index 621ac09..99ebc7c 100644
>> --- a/mm/debug_vm_pgtable.c
>> +++ b/mm/debug_vm_pgtable.c
>> @@ -67,7 +67,7 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>>  	WARN_ON(pte_write(pte_wrprotect(pte)));
>>  }
>>  
>> -#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>  static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
>>  {
>>  	pmd_t pmd = pfn_pmd(pfn, prot);
>> @@ -85,9 +85,6 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
>>  	 */
>>  	WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
>>  }
>> -#else
>> -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
>> -#endif
>>  
>>  #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>  static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
>> @@ -112,6 +109,10 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
>>  #else
>>  static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>  #endif
>> +#else
>> +static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
>> +static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
>> +#endif
> 
> Fails to build for THP case since
> 
> CONFIG_TRANSPARENT_HUGEPAGE=y
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD=n
> 
> ../mm/debug_vm_pgtable.c:112:20: error: redefinition of ‘pmd_basic_tests’
> 

Hmm, really ? With arm64 defconfig we have the same default combination
where it builds.

CONFIG_TRANSPARENT_HUGEPAGE=y
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD=n	/* It should not even appear */

With the above change, we have now

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
static void __init pmd_basic_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)
{
----
----
}
#else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
#endif
#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) { }
#endif

When !CONFIG_TRANSPARENT_HUGEPAGE

- Dummy definitions for pmd_basic_tests() and pud_basic_tests()

When CONFIG_TRANSPARENT_HUGEPAGE and !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

- Actual pmd_basic_tests() and dummy pud_basic_tests()

When CONFIG_TRANSPARENT_HUGEPAGE and CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

- Actual pmd_basic_tests() and pud_basic_tests()

Tested this on arm64 which does not have CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
for THP and !THP and on x86 which has CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
for THP and !THP which basically covered all combination for these configs.

Is there something I am still missing in plain sight :)

- Anshuman


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
  2019-11-08  4:27             ` Anshuman Khandual
@ 2019-11-11 19:06               ` Vineet Gupta
  0 siblings, 0 replies; 19+ messages in thread
From: Vineet Gupta @ 2019-11-11 19:06 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, Andrew Morton
  Cc: Vlastimil Babka, Greg Kroah-Hartman, Thomas Gleixner,
	Mike Rapoport, Jason Gunthorpe, Dan Williams, Peter Zijlstra,
	Michal Hocko, Mark Rutland, Mark Brown, Steven Price,
	Ard Biesheuvel, Masahiro Yamada, Kees Cook, Tetsuo Handa,
	Matthew Wilcox, Sri Krishna chowdary, Dave Hansen,
	Russell King - ARM Linux, Michael Ellerman, Paul Mackerras,
	Martin Schwidefsky, Heiko Carstens, David S. Miller, James Hogan,
	Paul Burton, Ralf Baechle, Kirill A . Shutemov, Gerald Schaefer,
	Christophe Leroy, Ingo Molnar, linux-snps-arc, linux-mips,
	linux-arm-kernel, linux-ia64, linuxppc-dev, linux-s390, linux-sh,
	sparclinux, x86, linux-kernel

On 11/7/19 8:27 PM, Anshuman Khandual wrote:
>
> On 11/08/2019 12:35 AM, Vineet Gupta wrote:
>> On 11/6/19 8:44 PM, Anshuman Khandual wrote:
>>>>>   */
>>>>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>>>>>  #include <asm/hugepage.h>
>>>>>  #endif
>>>> This in wrong.  CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE is a just a glue toggle,
>>>> used only in Kconfig files (and not in any "C" code).  It enables generic Kconfig
>>>> code to allow visibility of CONFIG_TRANSPARENT_HUGEPAGE w/o every arch needing to
>>>> do a me too.
>>>>
>>>> I think you need to use CONFIG_TRANSPARENT_HUGEPAGE to guard appropriate tests. I
>>>> understand that it only
>>> We can probably replace CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE wrapper with
>>> CONFIG_TRANSPARENT_HUGEPAGE. But CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>> explicitly depends on CONFIG_TRANSPARENT_HUGEPAGE as a prerequisite. Could
>>> you please confirm if the following change on this test will work on ARC
>>> platform for both THP and !THP cases ? Thank you.
>>>
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index 621ac09..99ebc7c 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -67,7 +67,7 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>>>  	WARN_ON(pte_write(pte_wrprotect(pte)));
>>>  }
>>>  
>>> -#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>  static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
>>>  {
>>>  	pmd_t pmd = pfn_pmd(pfn, prot);
>>> @@ -85,9 +85,6 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
>>>  	 */
>>>  	WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
>>>  }
>>> -#else
>>> -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>> -#endif
>>>  
>>>  #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>>  static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
>>> @@ -112,6 +109,10 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot)
>>>  #else
>>>  static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>>  #endif
>>> +#else
>>> +static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>> +static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>> Fails to build for THP case since
>>
>> CONFIG_TRANSPARENT_HUGEPAGE=y
>> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD=n
>>
>> ../mm/debug_vm_pgtable.c:112:20: error: redefinition of ‘pmd_basic_tests’
>>
> Hmm, really ? With arm64 defconfig we have the same default combination
> where it builds.
>
> CONFIG_TRANSPARENT_HUGEPAGE=y
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD=n	/* It should not even appear */
>
> With the above change, we have now
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> static void __init pmd_basic_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)
> {
> ----
> ----
> }
> #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
> #endif
> #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) { }
> #endif
>
> When !CONFIG_TRANSPARENT_HUGEPAGE
>
> - Dummy definitions for pmd_basic_tests() and pud_basic_tests()
>
> When CONFIG_TRANSPARENT_HUGEPAGE and !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>
> - Actual pmd_basic_tests() and dummy pud_basic_tests()
>
> When CONFIG_TRANSPARENT_HUGEPAGE and CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>
> - Actual pmd_basic_tests() and pud_basic_tests()
>
> Tested this on arm64 which does not have CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> for THP and !THP and on x86 which has CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> for THP and !THP which basically covered all combination for these configs.
>
> Is there something I am still missing in plain sight :)

Sorry my bad. I applied your manual hunk mindlessly and missed the nested #else.
So indeed it works. Although the stub for pud_basic_tests() is now defined twice
which makes it a bit ugly. But I'll leave that to you.

Thx,
-Vineet



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-11-11 19:06 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28  5:29 [PATCH V8] mm/debug: Add tests validating architecture page table helpers Anshuman Khandual
2019-10-29 10:31 ` Qian Cai
2019-11-04  2:15   ` Anshuman Khandual
2019-11-05  0:00 ` Anshuman Khandual
2019-11-05 22:30   ` Vineet Gupta
2019-11-06  3:03     ` Anshuman Khandual
2019-11-06 18:07       ` Vineet Gupta
2019-11-07  4:44         ` Anshuman Khandual
2019-11-07 19:05           ` Vineet Gupta
2019-11-08  4:27             ` Anshuman Khandual
2019-11-11 19:06               ` Vineet Gupta
2019-11-05  0:30 ` Anshuman Khandual
2019-11-05 19:36 ` Gerald Schaefer
2019-11-06  3:10   ` Anshuman Khandual
2019-11-06  3:22 ` Anshuman Khandual
2019-11-06  6:41   ` Christophe Leroy
2019-11-06  7:34     ` Anshuman Khandual
2019-11-07 12:54       ` Michael Ellerman
2019-11-07 13:22         ` Anshuman Khandual

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).