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

quoted

Ard Biesheuvel [off-list ref] a écrit :

On Mon, 19 Jul 2021 at 17:45, Christophe Leroy
[off-list ref] wrote:
Ard Biesheuvel [off-list ref] a écrit :

(add some folks back to cc:)

On Mon, 19 Jul 2021 at 15:58, Ard Biesheuvel [off-list ref] wrote:
On Mon, 19 Jul 2021 at 12:49, Will Deacon [off-list ref] wrote:
On Sat, Jul 17, 2021 at 06:31:08PM +0200, Christophe Leroy wrote:
Jonathan Marek [off-list ref] 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/ (local)
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 see Will or Catalin cc'ed on that patch, so no, the arm64
maintainers were not in copy. cc'ing a mailing list is not sufficient.

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.
The one that Will just referred to:
https://lore.kernel.org/linux-arm-kernel/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/ (local)
Yes it is that one.

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.
Sure. As long as arm64 gets fixed (and note that 39-bit VA/4k pages is
a very widely used config)

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.
Is this requirement documented somewhere? Having folded PUDs does not
necessarily mean that PUDs don't exist, but simply that PUDs and PGDs
are the same.
If you look at linux/pgtable.h you can see that p4d_set_huge and  
p4d_clear_huge are only for 5 level MMUs. Why shouldn't the same  
principle apply to PUD and PMD ?

Could the problematic arm pud_clear_huge and pud_set_huge be renamed
pgd_clear_huge and pgd_set_huge ?
No, that would break 48-bit VA (4 level paging) configurations, as in
that case, PGDs and PUDs are different levels/sizes.

I meant for 3 level paging only. Of course for 4 level it must apply  
on the PUD.

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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help