Re: [RFC PATCH v5 01/15] mm: Consolidate freeing of typed folios on final folio_put()

[Elliot Berman <elliot.berman@oss.qualcomm.com>]

On Fri, Jan 17, 2025 at 04:29:47PM +0000, Fuad Tabba wrote:
> Some folio types, such as hugetlb, handle freeing their own
> folios. Moreover, guest_memfd will require being notified once a
> folio's reference count reaches 0 to facilitate shared to private
> folio conversion, without the folio actually being freed at that
> point.
> 
> As a first step towards that, this patch consolidates freeing
> folios that have a type. The first user is hugetlb folios. Later
> in this patch series, guest_memfd will become the second user of
> this.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  include/linux/page-flags.h | 15 +++++++++++++++
>  mm/swap.c                  | 24 +++++++++++++++++++-----
>  2 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 691506bdf2c5..6615f2f59144 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -962,6 +962,21 @@ static inline bool page_has_type(const struct page *page)
>  	return page_mapcount_is_type(data_race(page->page_type));
>  }
>  
> +static inline int page_get_type(const struct page *page)
> +{
> +	return page->page_type >> 24;
> +}
> +
> +static inline bool folio_has_type(const struct folio *folio)
> +{
> +	return page_has_type(&folio->page);
> +}
> +
> +static inline int folio_get_type(const struct folio *folio)
> +{
> +	return page_get_type(&folio->page);
> +}
> +
>  #define FOLIO_TYPE_OPS(lname, fname)					\
>  static __always_inline bool folio_test_##fname(const struct folio *folio) \
>  {									\
> diff --git a/mm/swap.c b/mm/swap.c
> index 10decd9dffa1..6f01b56bce13 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -94,6 +94,20 @@ static void page_cache_release(struct folio *folio)
>  		unlock_page_lruvec_irqrestore(lruvec, flags);
>  }
>  
> +static void free_typed_folio(struct folio *folio)
> +{
> +	switch (folio_get_type(folio)) {

I think you need:

+#if IS_ENABLED(CONFIG_HUGETLBFS)
> +	case PGTY_hugetlb:
> +		free_huge_folio(folio);
> +		return;
+#endif

I think this worked before because folio_test_hugetlb was defined by:
FOLIO_TEST_FLAG_FALSE(hugetlb)
and evidently compiler optimizes out the free_huge_folio(folio) before
linking.

You'll probably want to do the same for the PGTY_guestmem in the later
patch!

> +	case PGTY_offline:
> +		/* Nothing to do, it's offline. */
> +		return;
> +	default:
> +		WARN_ON_ONCE(1);
> +	}
> +}
> +
>  void __folio_put(struct folio *folio)
>  {
>  	if (unlikely(folio_is_zone_device(folio))) {
> @@ -101,8 +115,8 @@ void __folio_put(struct folio *folio)
>  		return;
>  	}
>  
> -	if (folio_test_hugetlb(folio)) {
> -		free_huge_folio(folio);
> +	if (unlikely(folio_has_type(folio))) {
> +		free_typed_folio(folio);
>  		return;
>  	}
>  
> @@ -934,13 +948,13 @@ void folios_put_refs(struct folio_batch *folios, unsigned int *refs)
>  		if (!folio_ref_sub_and_test(folio, nr_refs))
>  			continue;
>  
> -		/* hugetlb has its own memcg */
> -		if (folio_test_hugetlb(folio)) {
> +		if (unlikely(folio_has_type(folio))) {
> +			/* typed folios have their own memcg, if any */
>  			if (lruvec) {
>  				unlock_page_lruvec_irqrestore(lruvec, flags);
>  				lruvec = NULL;
>  			}
> -			free_huge_folio(folio);
> +			free_typed_folio(folio);
>  			continue;
>  		}
>  		folio_unqueue_deferred_split(folio);
> -- 
> 2.48.0.rc2.279.g1de40edade-goog
>