Re: [PATCH] Revert "ptdesc: remove references to folios from __pagetable_ctor() and pagetable_dtor()"

[David Hildenbrand (Arm)]

On 2/25/26 01:24, Axel Rasmussen wrote:
> This change swapped out mod_node_page_state for lruvec_stat_add_folio.
> But, these two APIs are not interchangeable: the lruvec version also
> increments memcg stats, in addition to "global" pgdat stats.
> 
> So after this change, the "pagetables" memcg stat in memory.stat always
> yields "0", which is a userspace visible regression.

Argh, missed that these page tables are charged.

> 
> I tried to look for a refactor where we add a variant of
> lruvec_stat_mod_folio which takes a pgdat and a memcg instead of a
> folio, to try to adhere to the spirit of the original patch. But at the
> end of the day this just means we have to call
> folio_memcg(ptdesc_folio(ptdesc)) anyway, which doesn't really
> accomplish much.

The goal of the original patch is obviously to get rid of any folio
references in ptdesc code.

But if these ptdesc are charged, then we'd also need long-term a
ptdesc->memcg_data.

For now we could still go through page->memcg_data, to avoid going
through the folio.

But that requires some work, so likely we should just directly enlighten
ptdesc to have memcg_data.

Agreed that, as a quick fix, reverting the commit might be best.

But let's wait a bit whether Willy has a comment.

> 
> This regression is visible in master as well as 6.18 stable, so CC
> stable too.
> 
> Fixes: f0c92726e89f ("ptdesc: remove references to folios from __pagetable_ctor() and pagetable_dtor()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Axel Rasmussen <axelrasmussen@xxxxxxxxxx>
> ---
>  include/linux/mm.h | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5be3d8a8f806..abb4963c1f06 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3514,26 +3514,21 @@ static inline bool ptlock_init(struct ptdesc *ptdesc) { return true; }
>  static inline void ptlock_free(struct ptdesc *ptdesc) {}
>  #endif /* defined(CONFIG_SPLIT_PTE_PTLOCKS) */
>  
> -static inline unsigned long ptdesc_nr_pages(const struct ptdesc *ptdesc)
> -{
> -	return compound_nr(ptdesc_page(ptdesc));
> -}
> -
>  static inline void __pagetable_ctor(struct ptdesc *ptdesc)
>  {
> -	pg_data_t *pgdat = NODE_DATA(memdesc_nid(ptdesc->pt_flags));
> +	struct folio *folio = ptdesc_folio(ptdesc);
>  
> -	__SetPageTable(ptdesc_page(ptdesc));
> -	mod_node_page_state(pgdat, NR_PAGETABLE, ptdesc_nr_pages(ptdesc));
> +	__folio_set_pgtable(folio);
> +	lruvec_stat_add_folio(folio, NR_PAGETABLE);
>  }
>  
>  static inline void pagetable_dtor(struct ptdesc *ptdesc)
>  {
> -	pg_data_t *pgdat = NODE_DATA(memdesc_nid(ptdesc->pt_flags));
> +	struct folio *folio = ptdesc_folio(ptdesc);
>  
>  	ptlock_free(ptdesc);
> -	__ClearPageTable(ptdesc_page(ptdesc));
> -	mod_node_page_state(pgdat, NR_PAGETABLE, -ptdesc_nr_pages(ptdesc));
> +	__folio_clear_pgtable(folio);
> +	lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>  }
>  
>  static inline void pagetable_dtor_free(struct ptdesc *ptdesc)


-- 
Cheers,

David