Re: [patch 1/1] mm/debug: add tests validating architecture page table helpers

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 04/12/2020 10:03 PM, Linus Torvalds wrote:
> On Sun, Apr 12, 2020 at 12:42 AM Andrew Morton
> <akpm@linux-foundation.org> wrote:
>>
>> 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
>> inside kernel_init() right after async_synchronize_full().
> 
> This last part makes absolutely zero sense to me.
> 
> Why would it be done before the machine is properly up? Why isn't this
> just a module with a module_init? What makes this test so important
> and magical that it has to be done during early boot?

This test first started with being a module_init(), then a late_initcall()
before getting called inside kernel_init() in it's current form. During our
early review process (https://patchwork.kernel.org/patch/11176831/) Ingo had
suggested moving this test earlier during boot to catch potential page table
helper problems (if any), which made sense.

This test does not particularly need to run during early boot but it might
just be preferable to detect potential page table helper problems (if any)
earlier before they start getting used some where else. Though I guess it
is not absolutely necessary either.

> 
> So not applied, particularly since this apparently caused problems. We
> don't randomly break the boot, which is fragile anyway, just for a
> test that doesn't look like it should be boot-specific to me.

Fair enough, will change this to late_initcall().

Just to give some more historical context..

This test has been validated on all explicitly supported platforms (i.e via
enabling ARCH_HAS_DEBUG_VM_PGTABLE) i.e arm64, s390, arc, powerpc (32 and 64),
x86 (!X86_PAE). But this can also be enabled via CONFIG_EXPERT on platforms
that dont subscribe ARCH_HAS_DEBUG_VM_PGTABLE either because there were no
volunteers to run the test or the test might just fail to run. The idea behind
allowing this test get enabled via CONFIG_EXPERT was to just facilitate easy
experimentation (without really changing Kconfig) on platforms that dont support
it just yet through ARCH_HAS_DEBUG_VM_PGTABLE. There was a recent bug report for
one such scenarios [1]. But we do expect such scenarios to exist on platforms
without ARCH_HAS_DEBUG_VM_PGTABLE and CONFIG_EXPERT method was precisely provided
for the very same reason.

[1] https://lore.kernel.org/lkml/fb14237e-f973-bc34-4981-280ba5e1eb92@arm.com/

> 
> If it wants a new clean mm, it should just do a fork. And do so
> _looong_ long after boot.
> 
>                 Linus
>