On 7/13/26 23:40, Dave Hansen wrote:
> On 7/13/26 14:17, David Hildenbrand (Arm) wrote:
>>> To me, the double READ_ONCE() is a non-issue.
>> To other arch maintainers, it is!
>
> Ahh, so it seems like Christophe in ppc32 land was looking at this more
> like a regression that needed to get fixed.
Yes, exactly. :)
And I agree that it is something we should be optimizing.
>
> Christophe, just out of curiosity, was this something that was causing
> you practical problems like measurable performance regressions, or was
> it really just insane code generation that seems unacceptably suboptimal?
I'd be curious about that as well. I mean, looking at the generated code
it's clear that it is suboptimal.
The solution space I see:
(1) Pass the result from e.g., pgdp_get() into p4d_get(), so it can just return
the value with folded p4ds.
That requires extreme amounts of churn in core-mm AFAIKS, so I don't see that
as feasible.
(2) Rewrite folding code to make p4d_present() be a dummy instead of
pgd_present(), and make p4dp_get() return a dummy value.
... a lot of churn across architectures. I'm sure we'll learn soon why it was
done
ike the way it is today in the first place. Something interesting to look at,
certainly, but a bit of a stretch just to optimize reads.
(3) Make folded pgdp_get() use an ordinary read instead of a READ_ONCE / dummy.
I don't like that, because we couldn't catch easily if the value is then
actually used some old/new code.
If pgd_present() etc are supposed to ignore the value, I'd rather have a
mechanism
that enforces that the values are actually ignore in pgd_offset() etc as well.
(4) What we do in this series, but instead of forbidding set_pgd(), make it
only
complain when we are passing in a dummy value.
The expectation is that it would avoid touching most architecture code in patch
12 -> 27 and still prevent mistakes in the future.
@Yeoreum can you play with that and see what the end result would be?
The problem of how to handle ptdump effective_prot remains. Which is
unfortunately an
x86 32-bit only problem ;) ... and effective_prot is essentially an x86-only
thing.
There are various ways we could handle that, my preferred one would be
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fb298e2191792..3d56e48fe4151 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -96,7 +96,7 @@ config X86
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
select ARCH_HAS_PMEM_API if X86_64
select ARCH_HAS_PREEMPT_LAZY
- select ARCH_HAS_PTDUMP
+ select ARCH_HAS_PTDUMP if X86_64
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_HW_PTE_YOUNG
select ARCH_HAS_NONLEAF_PMD_YOUNG if PGTABLE_LEVELS > 2
Another option would be to keep the behavior unchanged:
diff --git a/mm/ptdump.c b/mm/ptdump.c
index 5851096e6f656..ce2cf5e07ac0a 100644
--- a/mm/ptdump.c
+++ b/mm/ptdump.c
@@ -39,6 +39,9 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
return note_kasan_page_table(walk, addr);
#endif
+ if (mm_p4d_folded(walk->mm))
+ return 0;
+
if (st->effective_prot_pgd)
st->effective_prot_pgd(st, val);
@@ -62,6 +65,13 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
return note_kasan_page_table(walk, addr);
#endif
+ if (mm_pud_folded(walk->mm))
+ return 0;
+
+ /* Simulate old behavior: all page table levels exist with folded
values. */
+ if (mm_p4d_folded(walk->mm) && (st->effective_prot_pgd))
+ st->effective_prot_pgd(st, __pgd(p4d_val(val)));
+
if (st->effective_prot_p4d)
st->effective_prot_p4d(st, val);
@@ -85,6 +95,15 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
return note_kasan_page_table(walk, addr);
#endif
+ if (mm_pmd_folded(walk->mm))
+ return 0;
+
+ /* Simulate old behavior: all page table levels exist with folded
values. */
+ if (mm_pud_folded(walk->mm) && st->effective_prot_pgd)
+ st->effective_prot_pgd(st, __pgd(pud_val(val)));
+ if (mm_p4d_folded(walk->mm) && st->effective_prot_p4d)
+ st->effective_prot_p4d(st, __p4d(pud_val(val)));
+
if (st->effective_prot_pud)
st->effective_prot_pud(st, val);
@@ -107,6 +126,14 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
return note_kasan_page_table(walk, addr);
#endif
+ /* Simulate old behavior: all page table levels exist with folded
values. */
+ if (mm_pud_folded(walk->mm) && st->effective_prot_pgd)
+ st->effective_prot_pgd(st, __pgd(pmd_val(val)));
+ if (mm_p4d_folded(walk->mm) && st->effective_prot_p4d)
+ st->effective_prot_p4d(st, __p4d(pmd_val(val)));
+ if (mm_pmd_folded(walk->mm) && st->effective_prot_pud)
+ st->effective_prot_pud(st, __pud(pmd_val(val)));
+
if (st->effective_prot_pmd)
st->effective_prot_pmd(st, val);
if (pmd_leaf(val)) {
Then there would be another option of making x86 simply remembering the first
level it got called for.
--
Cheers,
David
