Re: [PATCH v2] arm64: Enable vmalloc-huge with ptdump

[Will Deacon <will@kernel.org>]

On Wed, Jun 11, 2025 at 08:32:51AM +0530, Dev Jain wrote:
> 
> On 11/06/25 12:30 am, Ryan Roberts wrote:
> > On 10/06/2025 17:00, Dev Jain wrote:
> > > arm64 disables vmalloc-huge when kernel page table dumping is enabled,
> > > because an intermediate table may be removed, potentially causing the
> > > ptdump code to dereference an invalid address. We want to be able to
> > > analyze block vs page mappings for kernel mappings with ptdump, so to
> > > enable vmalloc-huge with ptdump, synchronize between page table removal in
> > > pmd_free_pte_page()/pud_free_pmd_page() and ptdump pagetable walking. We
> > > use mmap_read_lock and not write lock because we don't need to synchronize
> > > between two different vm_structs; two vmalloc objects running this same
> > > code path will point to different page tables, hence there is no race.
> > > 
> > > For pud_free_pmd_page(), we isolate the PMD table to avoid taking the lock
> > > 512 times again via pmd_free_pte_page(). Note that there is no need to
> > > move __flush_tlb_kernel_pgtable() to immediately after pud_clear(); the
> > > only argument against this would be that we immediately require a
> > > dsb(ishst) (present in __flush_tlb_kernel_pgtable()) after pud_clear(),
> > > but that is not the case, since the transition is from
> > > valid -> invalid, not vice-versa.
> > > 
> > > No issues were observed with mm-selftests. No issues were observed while
> > > parallelly running test_vmalloc.sh and dumping the kernel pagetable through
> > > sysfs in a loop.
> > > 
> > > v1->v2:
> > >   - Take lock only when CONFIG_PTDUMP_DEBUGFS is on
> > I thought we agreed that we would use a static key and some rcu synchronize
> > magic? What was the reason for taking this approach?
> 
> As I understand it, the RCU magic won't work, I had replied here:
> https://lore.kernel.org/all/6cd41ae9-303d-4eda-8d64-f7dba19eb106@arm.com/

Regardless, it's still not acceptable to penalise the common code because
of a debug option so I'm not going to merge this as-is.

Lemme go reply on the other thread.

Will