Re: [PATCH RFC v2 13/20] x86: mm: carve out the generic compile-time folded pgtable case in effective_prot()

[Yeoreum Yun <yeoreum.yun@arm.com>]

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