Re: [PATCH 04/11] mm: Rework compound_head() for power-of-2 sizeof(struct page)

Kiryl Shutsemau
2025-12-06T16:29:48Z

On Sat, Dec 06, 2025 at 12:25:12AM +0000, Usama Arif wrote:
>
>
> 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/

Ack.

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

Okay.

> >
> > 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@kernel.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?

There's no change for non-power-of-2 sizeof(struct page) as there's no
fake heads because there's no HVO for such cases.

See hugetlb_vmemmap_optimizable_size() as I mentioned in the comment.

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

But this code is about to be deleted. Is it really worth it?

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

Okay, I will try to explain it better.

>
> > + }
> > }
> > 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

Ack.

>
> > + * 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"

Ack.

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

Ack.

>
> > + * 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);
> }

Okay.

>
> > ps->idx = folio_page_idx(foliop, page);
> > }
> >
>

--
Kiryl Shutsemau / Kirill A. Shutemov