Re: [PATCH v3 06/22] mm: Always use page table accessor functions
From: David Hildenbrand (Red Hat)
Date: Thu Nov 27 2025 - 03:35:46 EST
On 11/27/25 09:26, Christophe Leroy (CS GROUP) wrote:
Le 26/11/2025 à 15:22, Ryan Roberts a écrit :
On 26/11/2025 13:47, Wei Yang wrote:
On Wed, Nov 26, 2025 at 01:03:42PM +0000, Ryan Roberts wrote:
On 26/11/2025 12:35, David Hildenbrand (Red Hat) wrote:[...]
Hi,
I've just come across this patch and wanted to mention that we could also
benefit from this improved absraction for some features we are looking at for
arm64. As you mention, Anshuman had a go but hit some roadblocks.
The main issue is that the compiler was unable to optimize away the
READ_ONCE()s
for the case where certain levels of the pgtable are folded. But it can
optimize
the plain C dereferences. There were complaints the the generated code for arm
(32) and powerpc was significantly impacted due to having many more
(redundant)
loads.
We do have mm_pmd_folded()/p4d_folded() etc, could that help to sort
this out internally?
Just stumbled over the reply from Christope:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F0019d675-ce3d-4a5c-89ed-f126c45145c9%40kernel.org&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C22d0a028b1ec4a8b678108de2cf73769%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638997637481119954%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=ocR6usVgRHfue0MrtbQnDO8whINvy%2FDMAfNE3caiY8c%3D&reserved=0
And wonder if we could handle that somehow directly in the pgdp_get() etc.
I certainly don't like the suggestion of doing the is_folded() test outside the
helper, but if we can push that logic down into pXdp_get() that would be pretty
neat. Anshuman and I did briefly play with the idea of doing a C dereference if
the level is folded and a READ_ONCE() otherwise, all inside each pXdp_get()
helper. Although we never proved it to be correct. I struggle with the model for
folding. Do you want to optimize out all-but-the-highest level's access or
all-but-the-lowest level's access? Makes my head hurt...
You mean sth like:
static inline pmd_t pmdp_get(pmd_t *pmdp)
{
#ifdef __PAGETABLE_PMD_FOLDED
return *pmdp;
#else
return READ_ONCE(*pmdp);
#endif
}
Yes. But I'm not convinced it's correct.
I *think* (but please correct me if I'm wrong) if the PMD is folded, the PUD and
P4D must also be folded, and you effectively have a 2 level pgtable consisting
of the PGD table and the PTE table. p4dp_get(), pudp_get() and pmdp_get() are
all effectively duplicating the load of the pgd entry? So assuming pgdp_get()
was already called and used READ_ONCE(), you might hope the compiler will just
drop the other loads and just use the value returned by READ_ONCE(). But I doubt
there is any guarantee of that and you might be in a situation where pgdp_get()
never even got called (perhaps you already have the pmd pointer).
I think you can't assume pgdp_get() was already called, because some
parts of code will directly descend to PMD level using pmd_off() or
pmd_off_k()
static inline pmd_t *pmd_off(struct mm_struct *mm, unsigned long va)
{
return pmd_offset(pud_offset(p4d_offset(pgd_offset(mm, va), va), va), va);
}
static inline pmd_t *pmd_off_k(unsigned long va)
{
return pmd_offset(pud_offset(p4d_offset(pgd_offset_k(va), va), va), va);
}
I'll note that these (nesty) helpers only work when you know that you have folded page tables.
And that's why I am arguing that the pmdp_get() must actually be kept as is.
--
Cheers
David