Re: [PATCH] Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"

From: Ard Biesheuvel <ardb@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Jonathan Marek <jonathan@marek.ca>,
	 Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	 Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
Date: Mon, 19 Jul 2021 15:58:46 +0200	[thread overview]
Message-ID: <CAMj1kXH5eBitt4max6dAOUbWsjy5XYqaMFV-L8HLWJmkbxUXAg@mail.gmail.com> (raw)
In-Reply-To: <20210719104918.GA6440@willie-the-truck>

On Mon, 19 Jul 2021 at 12:49, Will Deacon <will@kernel.org> wrote:
>
> On Sat, Jul 17, 2021 at 06:31:08PM +0200, Christophe Leroy wrote:
> > Jonathan Marek <jonathan@marek.ca> a écrit :
> >
> > > c742199a breaks arm64 for some configs because it stubs out functions which
> > > should not have been stubbed out.
> > >
> > > With 4K pages and ARM64_VA_BITS_39=y, the kernel crashes early on unmapped
> > > 1G pages in the linear map caused by pud_set_huge() in alloc_init_pud()
> > > being stubbed out. Reverting c742199a fixes the crash.
> >
> > This patch is there for a reason. Reverting it will break some other config.
>
> Which config? Not reverting it breaks arm64.
>
> > The fix needs to allow it work with all platforms.
>
> Hmm, there was already a report that selftests were failing with this
> change:
>
> https://lore.kernel.org/linux-arm-kernel/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
>
> That was a week ago and doesn't seem to have progressed, so I'm inclined to
> revert this if it's not resolved this week as we usually use -rc3 as a base
> for queuing patches for the next release.
>
> > I don't understand, why does arm64 needs these PUD helpers when it defines
> > __PAGETABLE_PUD_FOLDED ?

So if I am not mistaken, you modified arch/arm64 MMU code without
understanding it and without cc'ing any of the arm64 maintainers,
ignored reports that they were causing problems, and now you are
objecting to them being reverted because it break 'some other config'?
I don't think that is entirely reasonable, and the fact that the
maintainers weren't even cc'ed on the patch is enough justification to
simply revert it. IMHO.

>
> Probably for the huge vmap code; see arch_vmap_pXd_supported(). That also
> lines up with the failing selftests afaict.
>

The patch actually explains it: alloc_init_pud() in the swapper init
code uses it to lay down 1 GB block mappings for the linear map. That
code could quite easily be updated to work around this, but I guess it
would be better to fix this more comprehensively.

> For 4k pages with 3 levels of walk, we want to be able to use 1GB mappings
> at level 1 (pgd), 2MB mappings at level 2 (pmd) as well as the usual 4k
> page mappings at level 3 (pte).
>

Yes, we appear to use PUDs for 4k pages kernel regardless of whether
they are folded into PGDs to refer to level 1/1GB block mappings.

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