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.

> --- 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.

Reply via email to