Message-ID: <22609798-e84b-46ca-9cb5-649ffba4a2a4@gmail.com>
Date: Sat, 6 Dec 2025 00:25:12 +0000
From: Usama Arif <usamaarif642@...il.com>
To: Kiryl Shutsemau <kas@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Muchun Song <muchun.song@...ux.dev>
Cc: David Hildenbrand <david@...nel.org>, Oscar Salvador <osalvador@...e.de>,
 Mike Rapoport <rppt@...nel.org>, Vlastimil Babka <vbabka@...e.cz>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Matthew Wilcox <willy@...radead.org>, Zi Yan <ziy@...dia.com>,
 Baoquan He <bhe@...hat.com>, Michal Hocko <mhocko@...e.com>,
 Johannes Weiner <hannes@...xchg.org>, Jonathan Corbet <corbet@....net>,
 kernel-team@...a.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH 04/11] mm: Rework compound_head() for power-of-2
 sizeof(struct page)



On 05/12/2025 19:43, Kiryl Shutsemau wrote:
> For tail pages, the kernel uses the 'compound_info' field to get to the
> head page. The bit 0 of the field indicates whether the page is a
> tail page, and if set, the remaining bits represent a pointer to the
> head page.
> 
> For cases when size of struct page is power-of-2, change the encoding of
> compound_info to store a mask that can be applied to the virtual address
> of the tail page in order to access the head page. It is possible
> because sturct page of the head page is naturally aligned with regards

nit: s/sturct/struct/

> to order of the page.

Might be good to add to state here that no change expected if the struct page
is not a power of 2.

> 
> The significant impact of this modification is that all tail pages of
> the same order will now have identical 'compound_info', regardless of
> the compound page they are associated with. This paves the way for
> eliminating fake heads.
> 
> Signed-off-by: Kiryl Shutsemau <kas@...nel.org>
> ---
>  include/linux/page-flags.h | 61 +++++++++++++++++++++++++++++++++-----
>  mm/util.c                  | 15 +++++++---
>  2 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 11d9499e5ced..eef02fbbb40f 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -210,6 +210,13 @@ static __always_inline const struct page *page_fixed_fake_head(const struct page
>  	if (!static_branch_unlikely(&hugetlb_optimize_vmemmap_key))
>  		return page;
>  
> +	/*
> +	 * Fake heads only exists if size of struct page is power-of-2.
> +	 * See hugetlb_vmemmap_optimizable_size().
> +	 */
> +	if (!is_power_of_2(sizeof(struct page)))
> +		return page;
> +


hmm my understanding reviewing up until this patch of the series is that everything works
the same as old code when struct page is not a power of 2. Returning page here means you dont
fix page head when sizeof(struct page) is not a power of 2?

>  	/*
>  	 * Only addresses aligned with PAGE_SIZE of struct page may be fake head
>  	 * struct page. The alignment check aims to avoid access the fields (
> @@ -223,10 +230,13 @@ static __always_inline const struct page *page_fixed_fake_head(const struct page
>  		 * because the @page is a compound page composed with at least
>  		 * two contiguous pages.
>  		 */
> -		unsigned long head = READ_ONCE(page[1].compound_info);
> +		unsigned long info = READ_ONCE(page[1].compound_info);
>  
> -		if (likely(head & 1))
> -			return (const struct page *)(head - 1);
> +		if (likely(info & 1)) {
> +			unsigned long p = (unsigned long)page;
> +
> +			return (const struct page *)(p & info);

Would it be worth writing a comment over here similar to what you have in set_compound_head
to explain why this works? i.e. compound_info contains the mask derived from folio order that
can be applied to the virtual address to get the head page.

Also, it takes a few minutes to wrap your head around the fact that this works because the struct
page of the head page is aligned wrt to the order. Maybe it might be good to add that somewhere as
a comment somewhere? I dont see it documented in this patch, if its in a future patch, please ignore
this comment. 

> +		}
>  	}
>  	return page;
>  }
> @@ -281,11 +291,27 @@ static __always_inline int page_is_fake_head(const struct page *page)
>  
>  static __always_inline unsigned long _compound_head(const struct page *page)
>  {
> -	unsigned long head = READ_ONCE(page->compound_info);
> +	unsigned long info = READ_ONCE(page->compound_info);
>  
> -	if (unlikely(head & 1))
> -		return head - 1;
> -	return (unsigned long)page_fixed_fake_head(page);
> +	/* Bit 0 encodes PageTail() */
> +	if (!(info & 1))
> +		return (unsigned long)page_fixed_fake_head(page);
> +
> +	/*
> +	 * If the size of struct page is not power-of-2, the rest if

nit: s/if/of

> +	 * compound_info is the pointer to the head page.
> +	 */
> +	if (!is_power_of_2(sizeof(struct page)))
> +		return info - 1;
> +
> +	/*
> +	 * If the size of struct page is power-of-2 it is set the rest of

nit: remove "it is set"

> +	 * the info encodes the mask that converts the address of the tail
> +	 * page to the head page.
> +	 *
> +	 * No need to clear bit 0 in the mask as 'page' always has it clear.
> +	 */
> +	return (unsigned long)page & info;
>  }
>  
>  #define compound_head(page)	((typeof(page))_compound_head(page))
> @@ -294,7 +320,26 @@ static __always_inline void set_compound_head(struct page *page,
>  					      struct page *head,
>  					      unsigned int order)
>  {
> -	WRITE_ONCE(page->compound_info, (unsigned long)head + 1);
> +	unsigned int shift;
> +	unsigned long mask;
> +
> +	if (!is_power_of_2(sizeof(struct page))) {
> +		WRITE_ONCE(page->compound_info, (unsigned long)head | 1);
> +		return;
> +	}
> +
> +	/*
> +	 * If the size of struct page is power-of-2, bits [shift:0] of the
> +	 * virtual address of compound head are zero.
> +	 *
> +	 * Calculate mask that can be applied the virtual address of the

nit: applied to the ..

> +	 * tail page to get address of the head page.
> +	 */
> +	shift = order + order_base_2(sizeof(struct page));
> +	mask = GENMASK(BITS_PER_LONG - 1, shift);
> +
> +	/* Bit 0 encodes PageTail() */
> +	WRITE_ONCE(page->compound_info, mask | 1);
>  }
>  
>  static __always_inline void clear_compound_head(struct page *page)
> diff --git a/mm/util.c b/mm/util.c
> index cbf93cf3223a..6723d2bb7f1e 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -1234,7 +1234,7 @@ static void set_ps_flags(struct page_snapshot *ps, const struct folio *folio,
>   */
>  void snapshot_page(struct page_snapshot *ps, const struct page *page)
>  {
> -	unsigned long head, nr_pages = 1;
> +	unsigned long info, nr_pages = 1;
>  	struct folio *foliop;
>  	int loops = 5;
>  
> @@ -1244,8 +1244,8 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
>  again:
>  	memset(&ps->folio_snapshot, 0, sizeof(struct folio));
>  	memcpy(&ps->page_snapshot, page, sizeof(*page));
> -	head = ps->page_snapshot.compound_info;
> -	if ((head & 1) == 0) {
> +	info = ps->page_snapshot.compound_info;
> +	if ((info & 1) == 0) {
>  		ps->idx = 0;
>  		foliop = (struct folio *)&ps->page_snapshot;
>  		if (!folio_test_large(foliop)) {
> @@ -1256,7 +1256,14 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
>  		}
>  		foliop = (struct folio *)page;
>  	} else {
> -		foliop = (struct folio *)(head - 1);
> +		unsigned long p = (unsigned long)page;
> +
> +		/* See compound_head() */
> +		if (is_power_of_2(sizeof(struct page)))
> +			foliop = (struct folio *)(p & info);
> +		else
> +			foliop = (struct folio *)(info - 1);
> +

Would it be better to do below, as you dont need to than declare p if sizeof(struct page) is not
a power of 2?

if (!is_power_of_2(sizeof(struct page)))
	foliop = (struct folio *)(info - 1);
else {
	unsigned long p = (unsigned long)page;
	foliop = (struct folio *)(p & info);
}
	
>  		ps->idx = folio_page_idx(foliop, page);
>  	}
>  

Powered by blists - more mailing lists