Re: [PATCH mm-hotfixes 1/2] mm/huge_memory: separate out CONFIG_PERSISTENT_HUGE_ZERO_FOLIO logic

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Tue, Jul 28, 2026 at 04:08:35PM +0100, Kiryl Shutsemau wrote:
> On Tue, Jul 28, 2026 at 01:05:44PM +0100, Lorenzo Stoakes (ARM) wrote:
> > Rather than mixing the refcounted and non-refcounted
> > CONFIG_PERSISTENT_HUGE_ZERO_FOLIO logic, separate the two out cleanly
> > so it is clear what happens when this configuration option is set and what
> > happens when it is not.
> >
> > Introduce HUGE_ZERO_UNSET_PFN to abstract the ~0UL assignment, only
> > introduce the refcount and shrinker if !CONFIG_PERSISTENT_HUGE_ZERO_FOLIO,
> > abstract initialisation and teardown, abstract the huge zero folio
> > allocation from refcounting.
> >
> > Also change a BUG_ON() to WARN_ON_ONCE() while we're at it.
> >
> > Without this change, the subsequent fix for a subtle race is harder to
> > understand thus this is a dependency of it.
> >
> > Cc: stable@vger.kernel.org # 6.18.x: dependency of subsequent fix
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  mm/huge_memory.c | 159 ++++++++++++++++++++++++++++++++-----------------------
> >  1 file changed, 94 insertions(+), 65 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 032702a4637b..0f60bc82e87a 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -77,9 +77,14 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
> >  					 struct shrink_control *sc);
> >  static bool split_underused_thp = true;
> >
> > -static atomic_t huge_zero_refcount;
> > +#define HUGE_ZERO_UNSET_PFN (~0UL)
> >  struct folio *huge_zero_folio __read_mostly;
> > -unsigned long huge_zero_pfn __read_mostly = ~0UL;
> > +unsigned long huge_zero_pfn __read_mostly = HUGE_ZERO_UNSET_PFN;
> > +#ifndef CONFIG_PERSISTENT_HUGE_ZERO_FOLIO
> > +static atomic_t huge_zero_refcount;
> > +static struct shrinker *huge_zero_folio_shrinker;
> > +#endif
> > +
> >  unsigned long huge_anon_orders_always __read_mostly;
> >  unsigned long huge_anon_orders_madvise __read_mostly;
> >  unsigned long huge_anon_orders_inherit __read_mostly;
> > @@ -221,22 +226,58 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
> >  	return orders;
> >  }
> >
> > -static bool get_huge_zero_folio(void)
> > +static struct folio *alloc_huge_zero_folio(void)
> >  {
> >  	struct folio *zero_folio;
> > -retry:
> > -	if (likely(atomic_inc_not_zero(&huge_zero_refcount)))
> > -		return true;
> >
> >  	zero_folio = folio_alloc((GFP_TRANSHUGE | __GFP_ZERO | __GFP_ZEROTAGS) &
> >  				 ~__GFP_MOVABLE,
> >  			HPAGE_PMD_ORDER);
> >  	if (!zero_folio) {
> >  		count_vm_event(THP_ZERO_PAGE_ALLOC_FAILED);
> > -		return false;
> > +		return NULL;
> > +	}
> > +	folio_clear_large_rmappable(zero_folio); /* Explicitly not rmappable. */
> > +	return zero_folio;
> > +}
> > +
> > +#ifdef CONFIG_PERSISTENT_HUGE_ZERO_FOLIO
> > +static int __init huge_zero_init(void)
> > +{
> > +	huge_zero_folio = alloc_huge_zero_folio();
> > +	if (!huge_zero_folio) {
> > +		pr_warn("Allocating persistent huge zero folio failed\n");
>
> I am not sure the warn is enough. mm_get_huge_zero_folio() will produce
> NULL pointer now without any attempts to allocate again.

Well firstly this was existing behaviour :) and is to be backported also, so
anything changing that would need to be a separate change.

Secondly this failure really shouldn't happen in reality, it's very early in the
boot (see my RFC...) and the config has to set CONFIG_THP too.

And finally, more importantly perhaps, everywhere deals with it - anon read
fault in do_huge_pmd_anonymous_page() for e.g.

	if (!(vmf->flags & FAULT_FLAG_WRITE) &&
			!mm_forbids_zeropage(vma->vm_mm) &&
			transparent_hugepage_use_zero_page()) {
		...
		zero_folio = mm_get_huge_zero_folio(vma->vm_mm); <- returns NULL
		if (unlikely(!zero_folio)) {
			pte_free(vma->vm_mm, pgtable);
			count_vm_event(THP_FAULT_FALLBACK);
			return VM_FAULT_FALLBACK;
		}
		...
	}

DAX:

static vm_fault_t dax_pmd_load_hole(struct xa_state *xas, struct vm_fault *vmf,
		const struct iomap_iter *iter, void **entry)
{
	...
	zero_folio = mm_get_huge_zero_folio(vmf->vma->vm_mm);

	if (unlikely(!zero_folio)) {
		trace_dax_pmd_load_hole_fallback(inode, vmf, zero_folio, *entry);
		return VM_FAULT_FALLBACK;
	}

	...
}

etc. etc.

Where:

struct folio *mm_get_huge_zero_folio(struct mm_struct *mm)
{
	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO))
		return huge_zero_folio; <-- returns NULL
}

So it's all fine.

>
> Have you considered moving huge_zero_folio to BSS for
> CONFIG_PERSISTENT_HUGE_ZERO_FOLIO=y?

Interesting :)

That'd be huge for larger page size and why would we expect a failure at init
like this I guess I'd say?

It'd be nice to guarantee it though.

IN general you have to have CONFIG_THP enabled to get this at all, and surely
the micro embedded systems will never set that... (see my RFC to just make the
persistent huge zero folio permanently how we do this and remove the damn
refcounted nonsense altogether).

But one for a follow up anyway!

>
> > @@ -308,7 +323,46 @@ static unsigned long shrink_huge_zero_folio_scan(struct shrinker *shrink,
> >  	return 0;
> >  }
> >
> > -static struct shrinker *huge_zero_folio_shrinker;
> > +static int __init huge_zero_init(void)
> > +{
> > +	huge_zero_folio_shrinker = shrinker_alloc(0, "thp-zero");
> > +	if (!huge_zero_folio_shrinker) {
> > +		shrinker_free(deferred_split_shrinker);
> > +		list_lru_destroy(&deferred_split_lru);
>
> Hm. What? Why does huge_zero_init() touches deferred_*?
> That's caller business.

Again existing code :)

But yeah that's really broken as subsys_initcall() apparently discards the
error...

I guess the thinking was because I think because it's doing stuff after the
deferred stuff is initialised and bailling.

In practice it's probably an impossibly small allocation to fail. But should do
something about this, if only oops-ing on failure.

But that's for a follow up :) existing code, backported fix etc. etc.

This patch is rearranging stuff so the code is much clearer for the race stuff
taking into account CONFIG_PERSISTENT_HUGE_ZERO_FOLIO.

>
> >  static void __init thp_shrinker_exit(void)
> >  {
> > -	shrinker_free(huge_zero_folio_shrinker);
> >  	shrinker_free(deferred_split_shrinker);
> >  	list_lru_destroy(&deferred_split_lru);
> > +	huge_zero_shrinker_exit();
>
> Any reason behind the reorder?

Well 'do everything else first then call the external function' but really no
solid reason. It makes no difference though.

Can re-reorder if a respin required :)

>
> >  }
>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

Cheers, Lorenzo