Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
> #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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
#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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
On 11/6/19 8:44 PM, Anshuman Khandual wrote:
>
>>
>>> */
>>> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
>>> #include
>>> #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’
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
On 11/07/2019 06:24 PM, Michael Ellerman wrote: > Anshuman Khandual 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-1-g250339d6747b-dirty #152 > NIP: c10435a0 LR: c10434b4 CTR: > REGS: c0003a403980 TRAP: 0901 Not tainted > (5.4.0-rc6-gcc-8.2.0-next-20191107-1-g250339d6747b-dirty) > MSR: 82009033 CR: 44000222 XER: > > CFAR: c10435a8 IRQMASK: 0 > GPR00: c10434b4 c0003a403c10 c1295000 0521000100c0 > GPR04: 8105 00400dc0 3eb0 0001 > GPR08: 0001 0100 > GPR12: c18f > NIP [c10435a0] debug_vm_pgtable+0x43c/0x82c > LR [c10434b4] debug_vm_pgtable+0x350/0x82c > Call Trace: > [c0003a403c10] [c104346c] debug_vm_pgtable+0x308/0x82c > (unreliable) > [c0003a403ce0] [c1004310] kernel_init_freeable+0x1d0/0x39c > [c0003a403db0] [c0010da0] kernel_init+0x24/0x174 > [c0003a403e20] [c000bdc4] ret_from_kernel_thread+0x5c/0x78 > Instruction dump: > 7d075078 7ce74b78 7ce0f9ad 40c2fff0 3880 7f83e378 4b02eee1 6000 > 4880 3920 3941 3900 <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-1-g250339d6747b-dirty #152 > NIP: c00724e8 LR: c104358c CTR: > REGS: c0003a483980 TRAP: 0700 Not tainted > (5.4.0-rc6-gcc-8.2.0-next-20191107-1-g250339d6747b-dirty) > MSR: 82029033 CR: 24000224 XER: > 2000 > CFAR: c1043588 IRQMASK: 0 > GPR00: c104358c c0003a483c10 c1295000 0009 > GPR04: 0005 0009 > GPR08: 0001 000e 0001 c0003a5f > GPR12: c18f c0010d84 > GPR16: c0003a5f 8105 > GPR20: c1003ab8 0015 0500613a0080 0900603a0080 > GPR24: 09202e3a0080 c133bd90 c133bd98 c133bda0 > GPR28: c0003a5e c0003a600af8 c0003a2e2d48 c0003a6100a0 > NIP [c00724e8] assert_pte_locked+0x88/0x190 > LR [c104358c] debug_vm_pgtable+0x428/0x82c > Call Trace: > [c0003a483c10] [c104346c] debug_vm_pgtable+0x308/0x82c > (unreliable) > [c0003a483ce0] [c1004310] kernel_init_freeable+0x1d0/0x39c > [c0003a483db0] [c0010da0] kernel_init+0x24/0x174 > [c0003a483e20] [c000bdc4] ret_from_kernel_thread+0x5c/0x78 > Instruction dump: > 7d251a14 39070010 7d463030 7d084a14 38c6 7c884436 7cc607b4
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
Anshuman Khandual 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-1-g250339d6747b-dirty #152 NIP: c10435a0 LR: c10434b4 CTR: REGS: c0003a403980 TRAP: 0901 Not tainted (5.4.0-rc6-gcc-8.2.0-next-20191107-1-g250339d6747b-dirty) MSR: 82009033 CR: 44000222 XER: CFAR: c10435a8 IRQMASK: 0 GPR00: c10434b4 c0003a403c10 c1295000 0521000100c0 GPR04: 8105 00400dc0 3eb0 0001 GPR08: 0001 0100 GPR12: c18f NIP [c10435a0] debug_vm_pgtable+0x43c/0x82c LR [c10434b4] debug_vm_pgtable+0x350/0x82c Call Trace: [c0003a403c10] [c104346c] debug_vm_pgtable+0x308/0x82c (unreliable) [c0003a403ce0] [c1004310] kernel_init_freeable+0x1d0/0x39c [c0003a403db0] [c0010da0] kernel_init+0x24/0x174 [c0003a403e20] [c000bdc4] ret_from_kernel_thread+0x5c/0x78 Instruction dump: 7d075078 7ce74b78 7ce0f9ad 40c2fff0 3880 7f83e378 4b02eee1 6000 4880 3920 3941 3900 <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-1-g250339d6747b-dirty #152 NIP: c00724e8 LR: c104358c CTR: REGS: c0003a483980 TRAP: 0700 Not tainted (5.4.0-rc6-gcc-8.2.0-next-20191107-1-g250339d6747b-dirty) MSR: 82029033 CR: 24000224 XER: 2000 CFAR: c1043588 IRQMASK: 0 GPR00: c104358c c0003a483c10 c1295000 0009 GPR04: 0005 0009 GPR08: 0001 000e 0001 c0003a5f GPR12: c18f c0010d84 GPR16: c0003a5f 8105 GPR20: c1003ab8 0015 0500613a0080 0900603a0080 GPR24: 09202e3a0080 c133bd90 c133bd98 c133bda0 GPR28: c0003a5e c0003a600af8 c0003a2e2d48 c0003a6100a0 NIP [c00724e8] assert_pte_locked+0x88/0x190 LR [c104358c] debug_vm_pgtable+0x428/0x82c Call Trace: [c0003a483c10] [c104346c] debug_vm_pgtable+0x308/0x82c (unreliable) [c0003a483ce0] [c1004310] kernel_init_freeable+0x1d0/0x39c [c0003a483db0] [c0010da0] kernel_init+0x24/0x174 [c0003a483e20] [c000bdc4] ret_from_kernel_thread+0x5c/0x78 Instruction dump: 7d251a14 39070010 7d463030 7d084a14 38c6 7c884436 7cc607b4 7d083038 79081f24 7ccb402a 7cc80074 7908d182 <0b08> 78cb0022 54c8c03e 7d473830 ---[ end trace a694f1bc56529c0e ]--- cheers
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
>> #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
>> #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
>
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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 > #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 > #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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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 >> >
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
On 11/06/2019 01:06 AM, Gerald Schaefer wrote: > On Mon, 28 Oct 2019 10:59:22 +0530 > 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. > > 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 > >
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
#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
#endif
>
> Other than that the tests pass for !THP and THP too. So once fixed, you could
Glad that it
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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 > Cc: Vlastimil Babka > Cc: Greg Kroah-Hartman > Cc: Thomas Gleixner > Cc: Mike Rapoport > Cc: Jason Gunthorpe > Cc: Dan Williams > Cc: Peter Zijlstra > Cc: Michal Hocko > Cc: Mark Rutland > Cc: Mark Brown > Cc: Steven Price > Cc: Ard Biesheuvel > Cc: Masahiro Yamada > Cc: Kees Cook > Cc: Tetsuo Handa > Cc: Matthew Wilcox > Cc: Sri Krishna chowdary > Cc: Dave Hansen > Cc: Russell King - ARM Linux > Cc: Michael Ellerman > Cc: Paul Mackerras > Cc: Martin Schwidefsky > Cc: Heiko Carstens > Cc: "David S. Miller" > Cc: Vineet Gupta > Cc: James Hogan > Cc: Paul Burton > Cc: Ralf Baechle > Cc: Kirill A. Shutemov > Cc: Gerald Schaefer > Cc: Christophe Leroy > Cc: Ingo Molnar > Cc: linux-snps-...@lists.infradead.org > Cc: linux-m...@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-i...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-s...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: sparcli...@vger.kernel.org > Cc: x...@kernel.org > Cc: linux-ker...@vger.kernel.org > > Tested-by: Christophe Leroy #PPC32 > Suggested-by: Catalin Marinas > Signed-off-by: Andrew Morton > Signed-off-by: Christophe Leroy > Signed-off-by: Anshuman Khandual > --- > > 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.ga56...@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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
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 > Cc: Vlastimil Babka > Cc: Greg Kroah-Hartman > Cc: Thomas Gleixner > Cc: Mike Rapoport > Cc: Jason Gunthorpe > Cc: Dan Williams > Cc: Peter Zijlstra > Cc: Michal Hocko > Cc: Mark Rutland > Cc: Mark Brown > Cc: Steven Price > Cc: Ard Biesheuvel > Cc: Masahiro Yamada > Cc: Kees Cook > Cc: Tetsuo Handa > Cc: Matthew Wilcox > Cc: Sri Krishna chowdary > Cc: Dave Hansen > Cc: Russell King - ARM Linux > Cc: Michael Ellerman > Cc: Paul Mackerras > Cc: Martin Schwidefsky > Cc: Heiko Carstens > Cc: "David S. Miller" > Cc: Vineet Gupta > Cc: James Hogan > Cc: Paul Burton > Cc: Ralf Baechle > Cc: Kirill A. Shutemov > Cc: Gerald Schaefer > Cc: Christophe Leroy > Cc: Ingo Molnar > Cc: linux-snps-...@lists.infradead.org > Cc: linux-m...@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-i...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-s...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: sparcli...@vger.kernel.org > Cc: x...@kernel.org > Cc: linux-ker...@vger.kernel.org > > Tested-by: Christophe Leroy #PPC32 > Suggested-by: Catalin Marinas > Signed-off-by: Andrew Morton > Signed-off-by: Christophe Leroy > Signed-off-by: Anshuman Khandual > --- > > 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.ga56...@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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
On Mon, 28 Oct 2019 10:59:22 +0530 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. 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
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
On 10/29/2019 04:01 PM, Qian Cai wrote: > > >> On Oct 28, 2019, at 1:29 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. > > 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.
Re: [PATCH V8] mm/debug: Add tests validating architecture page table helpers
> On Oct 28, 2019, at 1:29 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. This looks like a perfect candidate to streamline with the new kunit framework, no?
