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

[Christophe Leroy]

Ard Biesheuvel <ardb at kernel.org> a écrit :

> (add some folks back to cc:)
>
> On Mon, 19 Jul 2021 at 15:58, Ard Biesheuvel <ardb at kernel.org> wrote:
>>
>> On Mon, 19 Jul 2021 at 12:49, Will Deacon <will at kernel.org> wrote:
>> >
>> > On Sat, Jul 17, 2021 at 06:31:08PM +0200, Christophe Leroy wrote:
>> > > Jonathan Marek <jonathan at 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.

arm maintainers were in copy, see  
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/73ec95f40cafbbb69bdfb43a7f53876fd845b0ce.1620990479.git.christophe.leroy@csgroup.eu/

I don't know what report you mention, I m only aware of one that I got  
after the start of my holidays and that I plan to handle first week of  
August when I'm back at work.

As I answered to Will, now that we are aware of an issue on Arm64 we  
need to work together and find a fix that works for all.


>>
>> >
>> > 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.

And that's probably the source of the misunderstanding, because a 3  
level architecture is supposed to only have pgd, pmd and pud.

Could the problematic arm pud_clear_huge and pud_set_huge be renamed  
pgd_clear_huge and pgd_set_huge ?

Christophe