Hi Dave,
> On 7/22/26 08:30, Yeoreum Yun wrote:
> > struct pg_state {
> > struct ptdump_state ptdump;
> > + int first_level;
> > int level;
> > pgprotval_t current_prot;
> > pgprotval_t effective_prot;
>
> There is zero reason on x86 to hold this value in a data structure. It
> is 100% determined at boot time or earlier. Pre-5-level-paging it's a
> compile-time constant.
>
> On x86, it never varies per-mm.
Yes once ptdump_pt_level_first() is converted with static inline.
I'll change this.
> > --- a/mm/ptdump.c
> > +++ b/mm/ptdump.c
> > @@ -197,6 +197,17 @@ void ptdump_walk_pgd(struct ptdump_state *st, struct
> > mm_struct *mm, pgd_t *pgd)
> > st->note_page_flush(st);
> > }
> >
> > +int ptdump_pt_level_first(struct mm_struct *mm)
> > +{
> > + if (mm_pmd_folded(mm))
> > + return 3;
> > + if (mm_pud_folded(mm))
> > + return 2;
> > + if (mm_p4d_folded(mm))
> > + return 1;
> > + return 0;
> > +}
>
> Having this in a .c file may not be the best design decision. On a lot
> of architectures and configs, this is 100% a compile-time constant.
> Hiding it in a .c file and forcing a call is kinda silly.
>
> In the worst-case scenario on x86, this ends up being an "ALTERNATIVES"
> patched check. So it's technically variable, but patched at boot-time to
> one possibility or the other.
>
> If something is making the function larger than a few instructions,
> something is wrong and needs to get fixed.
>
> IOW, this should be a static inline in a header, not a full CALL'd
> function in a .c file. *That* will make the value visible to the
> compiler at all call sites and let it do all the smart things compilers
> can do with build time constants.
>
> Also, my original objection was "this function has zero to do with x86".
> Well, it also really has zero to do with ptdump, either. I'd probably
> just stick it in one of the generic page table headers.
Agree. I'll locate in the linux/pgtable.h
--
Sincerely,
Yeoreum Yun
