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 > _______________________________________________ linux-snps-arc mailing list linux-snps-arc@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/linux-snps-arc