Re: [PATCH 3/9] mm: Call free_transhuge_folio() directly from destroy_large_folio()

[David Hildenbrand <david@redhat.com>]

On 15.08.23 05:26, Matthew Wilcox (Oracle) wrote:
> Indirect calls are expensive, thanks to Spectre.  Convert this one to
> a direct call, and pass a folio instead of the head page for type safety.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/huge_mm.h | 2 +-
>   mm/huge_memory.c        | 5 ++---
>   mm/page_alloc.c         | 8 +++++---
>   3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 20284387b841..24aee49a581a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -144,7 +144,7 @@ unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
>   		unsigned long len, unsigned long pgoff, unsigned long flags);
>   
>   void prep_transhuge_page(struct page *page);
> -void free_transhuge_page(struct page *page);
> +void free_transhuge_folio(struct folio *folio);
>   
>   bool can_split_folio(struct folio *folio, int *pextra_pins);
>   int split_huge_page_to_list(struct page *page, struct list_head *list);
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 8480728fa220..516fe3c26ef3 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2779,9 +2779,8 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>   	return ret;
>   }
>   
> -void free_transhuge_page(struct page *page)
> +void free_transhuge_folio(struct folio *folio)
>   {
> -	struct folio *folio = (struct folio *)page;
>   	struct deferred_split *ds_queue = get_deferred_split_queue(folio);
>   	unsigned long flags;
>   
> @@ -2798,7 +2797,7 @@ void free_transhuge_page(struct page *page)
>   		}
>   		spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>   	}
> -	free_compound_page(page);
> +	free_compound_page(&folio->page);
>   }
>   
>   void deferred_split_folio(struct folio *folio)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1f67d4968590..feb2e95cf021 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -287,9 +287,6 @@ const char * const migratetype_names[MIGRATE_TYPES] = {
>   static compound_page_dtor * const compound_page_dtors[NR_COMPOUND_DTORS] = {
>   	[NULL_COMPOUND_DTOR] = NULL,
>   	[COMPOUND_PAGE_DTOR] = free_compound_page,
> -#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -	[TRANSHUGE_PAGE_DTOR] = free_transhuge_page,
> -#endif
>   };
>   
>   int min_free_kbytes = 1024;
> @@ -624,6 +621,11 @@ void destroy_large_folio(struct folio *folio)
>   		return;
>   	}
>   
> +	if (folio_test_transhuge(folio) && dtor == TRANSHUGE_PAGE_DTOR) {
> +		free_transhuge_folio(folio);

I really wonder if folio_test_transhuge() should be written similar to 
folio_test_hugetlb() instead, such that the dtor check is implicit.

Any good reasons not to do that?

-- 
Cheers,

David / dhildenb