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

Hi Anshuman,

Hello Vineet,

On 11/4/19 4:00 PM, Anshuman Khandual wrote:

quoted

On 10/28/2019 10:59 AM, Anshuman Khandual wrote:

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel