Re: [PATCH v3 06/22] mm: Always use page table accessor functions
From: David Hildenbrand (Red Hat)
Date: Wed Nov 26 2025 - 09:56:21 EST
On 11/26/25 15:52, Lorenzo Stoakes wrote:
On Wed, Nov 26, 2025 at 03:46:40PM +0100, David Hildenbrand (Red Hat) wrote:
On 11/26/25 15:22, Ryan Roberts wrote:
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://lkml.kernel.org/r/0019d675-ce3d-4a5c-89ed-f126c45145c9@xxxxxxxxxx
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.
Yeah, I'm also still trying to understand how it could work.
With __PAGETABLE_PMD_FOLDED we treat the PUD to be fake-present, like
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).
static inline int pud_present(pud_t pud) { return 1; }
And obtaining the pmd_t* is essentially cast of the pud_t*
static inline pmd_t * pmd_offset(pud_t * pud, unsigned long address)
{
return (pmd_t *)pud;
}
So in that case we might want to have the READ_ONCE() remove from the
pudp_get(), not the pmdp_get()?
Would the pmdp_get() never get invoked then? Or otherwise wouldn't that end up
requiring a READ_ONCE() further up the stack?
See my other reply, I think the pmdp_get() is required because all pud_* functions are just simple stubs.
IOW, push the READ_ONCE() down to the lowest level so the previous ones
(that will get essentially ignore?) will get folded into the last
READ_ONCE()?
But my head still hurts and I am focusing on something else concurrently :)
Even if we could make this work, I don't love that there's some implicit
assumption there that could easily break later on.
I'd rather we kept it as stupid/obvious as possible...
Looking at include/asm-generic/pgtable-nopmd.h I am not sure we are talking about implicit assumptions here. It's kind-of the design that the pud_t values are dummies, so why shoul the pudp_get() give you any guarantees.
At least that's my current understanding, which might be very flawed :)
--
Cheers
David