[PATCH mm-hotfixes 0/2] mm/huge_memory: fix huge_zero_pfn race

[PATCH mm-hotfixes 0/2] mm/huge_memory: fix huge_zero_pfn race

[Lorenzo Stoakes (ARM)]

There is a subtle race in the reference-counted huge_zero_folio
implementation.

The fast path atomic logic fails to account for the fact that the
shrinker (which drops the final huge_zero_refcount pin) can overwrite
huge_zero_pfn with the ~0UL sentinel value in shrink_huge_zero_folio_scan()
after a racing get_huge_zero_folio() installed a valid value there.

This results in huge_zero_folio being correctly set but huge_zero_pfn being
set incorrectly and thus is_huge_zero_pfn() and consequently
is_huge_zero_pmd() will misidentify the huge zero folio as being an
ordinary THP folio.

This can result in the huge zero folio being split and otherwise treated
incorrectly.

The solution to this is very subtle as there is an atomic fast path, and
thus ordering in weakly ordered architectures has to be treated very
carefully.

As a result, this series first reworks the
CONFIG_PERSISTENT_HUGE_ZERO_FOLIO logic so it is separated from the
refcounted code in order to make the subsequent fix reasonably
understandable.

The second commit fixes the issue by introducing a spinlock around
huge_zero_[pfn, folio, refcount] write, with careful consideration paid to
load/store ordering in the fast path.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
Lorenzo Stoakes (ARM) (2):
      mm/huge_memory: separate out CONFIG_PERSISTENT_HUGE_ZERO_FOLIO logic
      mm/huge_memory: fix huge_zero_pfn race

 mm/huge_memory.c | 189 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 116 insertions(+), 73 deletions(-)
---
base-commit: 5db85e34ec49beee590676d55f13e8e2b14c742f
change-id: 20260728-fix-refcounted-huge-zero-21cb07b4fa3e

Cheers,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>

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

[Lorenzo Stoakes (ARM)]

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");
+	} else {
+		huge_zero_pfn = folio_pfn(huge_zero_folio);
+		count_vm_event(THP_ZERO_PAGE_ALLOC);
 	}
-	/* Ensure zero folio won't have large_rmappable flag set. */
-	folio_clear_large_rmappable(zero_folio);
+	return 0;
+}
+
+static void __init huge_zero_shrinker_exit(void)
+{
+}
+
+struct folio *mm_get_huge_zero_folio(struct mm_struct *mm)
+{
+	return huge_zero_folio;
+}
+
+void mm_put_huge_zero_folio(struct mm_struct *mm)
+{
+}
+#else
+static bool get_huge_zero_folio(void)
+{
+	struct folio *zero_folio;
+retry:
+	if (likely(atomic_inc_not_zero(&huge_zero_refcount)))
+		return true;
+
+	zero_folio = alloc_huge_zero_folio();
+	if (unlikely(!zero_folio))
+		return false;
+
 	preempt_disable();
 	if (cmpxchg(&huge_zero_folio, NULL, zero_folio)) {
 		preempt_enable();
@@ -258,33 +299,7 @@ static void put_huge_zero_folio(void)
 	 * Counter should never go to zero here. Only shrinker can put
 	 * last reference.
 	 */
-	BUG_ON(atomic_dec_and_test(&huge_zero_refcount));
-}
-
-struct folio *mm_get_huge_zero_folio(struct mm_struct *mm)
-{
-	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO))
-		return huge_zero_folio;
-
-	if (mm_flags_test(MMF_HUGE_ZERO_FOLIO, mm))
-		return READ_ONCE(huge_zero_folio);
-
-	if (!get_huge_zero_folio())
-		return NULL;
-
-	if (mm_flags_test_and_set(MMF_HUGE_ZERO_FOLIO, mm))
-		put_huge_zero_folio();
-
-	return READ_ONCE(huge_zero_folio);
-}
-
-void mm_put_huge_zero_folio(struct mm_struct *mm)
-{
-	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO))
-		return;
-
-	if (mm_flags_test(MMF_HUGE_ZERO_FOLIO, mm))
-		put_huge_zero_folio();
+	WARN_ON_ONCE(atomic_dec_and_test(&huge_zero_refcount));
 }
 
 static unsigned long shrink_huge_zero_folio_count(struct shrinker *shrink,
@@ -300,7 +315,7 @@ static unsigned long shrink_huge_zero_folio_scan(struct shrinker *shrink,
 	if (atomic_cmpxchg(&huge_zero_refcount, 1, 0) == 1) {
 		struct folio *zero_folio = xchg(&huge_zero_folio, NULL);
 		BUG_ON(zero_folio == NULL);
-		WRITE_ONCE(huge_zero_pfn, ~0UL);
+		WRITE_ONCE(huge_zero_pfn, HUGE_ZERO_UNSET_PFN);
 		folio_put(zero_folio);
 		return HPAGE_PMD_NR;
 	}
@@ -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);
+		return -ENOMEM;
+	}
+
+	huge_zero_folio_shrinker->count_objects = shrink_huge_zero_folio_count;
+	huge_zero_folio_shrinker->scan_objects = shrink_huge_zero_folio_scan;
+	shrinker_register(huge_zero_folio_shrinker);
+	return 0;
+}
+
+static void __init huge_zero_shrinker_exit(void)
+{
+	shrinker_free(huge_zero_folio_shrinker);
+}
+
+struct folio *mm_get_huge_zero_folio(struct mm_struct *mm)
+{
+	if (mm_flags_test(MMF_HUGE_ZERO_FOLIO, mm))
+		return READ_ONCE(huge_zero_folio);
+
+	if (!get_huge_zero_folio())
+		return NULL;
+
+	if (mm_flags_test_and_set(MMF_HUGE_ZERO_FOLIO, mm))
+		put_huge_zero_folio();
+
+	return READ_ONCE(huge_zero_folio);
+}
+
+void mm_put_huge_zero_folio(struct mm_struct *mm)
+{
+	if (mm_flags_test(MMF_HUGE_ZERO_FOLIO, mm))
+		put_huge_zero_folio();
+}
+#endif /* CONFIG_PERSISTENT_HUGE_ZERO_FOLIO */
 
 #ifdef CONFIG_SYSFS
 static ssize_t enabled_show(struct kobject *kobj,
@@ -972,39 +1026,14 @@ static int __init thp_shrinker_init(void)
 	deferred_split_shrinker->scan_objects = deferred_split_scan;
 	shrinker_register(deferred_split_shrinker);
 
-	if (IS_ENABLED(CONFIG_PERSISTENT_HUGE_ZERO_FOLIO)) {
-		/*
-		 * Bump the reference of the huge_zero_folio and do not
-		 * initialize the shrinker.
-		 *
-		 * huge_zero_folio will always be NULL on failure. We assume
-		 * that get_huge_zero_folio() will most likely not fail as
-		 * thp_shrinker_init() is invoked early on during boot.
-		 */
-		if (!get_huge_zero_folio())
-			pr_warn("Allocating persistent huge zero folio failed\n");
-		return 0;
-	}
-
-	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);
-		return -ENOMEM;
-	}
-
-	huge_zero_folio_shrinker->count_objects = shrink_huge_zero_folio_count;
-	huge_zero_folio_shrinker->scan_objects = shrink_huge_zero_folio_scan;
-	shrinker_register(huge_zero_folio_shrinker);
-
-	return 0;
+	return huge_zero_init();
 }
 
 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();
 }
 
 static int __init hugepage_init(void)

-- 
2.55.0

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

[Kiryl Shutsemau]

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.

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

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

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

>  }

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

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

[Lorenzo Stoakes (ARM)]

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

[PATCH mm-hotfixes 2/2] mm/huge_memory: fix huge_zero_pfn race

[Lorenzo Stoakes (ARM)]

If !CONFIG_PERSISTENT_HUGE_ZERO_FOLIO, the huge_zero_folio is refcounted by
huge_zero_refcount and returned by mm_get_huge_zero_folio().

When the caller is done with the huge zero page, its reference count is
decremented. Only a shrinker can set the reference count to zero.

A race can unfortunately occur between a shrinker decrementing the
reference count to zero and a concurrent page fault.

This is because shrink_huge_zero_folio_scan() might, if very
unlucky, be preempted between setting huge_zero_refcount to zero
and writing an invalid value.

During this time get_huge_zero_folio() could write to huge_zero_pfn
before shrink_huge_zero_folio_scan() resumes.

In this event the huge zero folio will be persistently
misidentified causing the THP code path to be entered
inappropriately for the huge zero folio:

                CPU 0                                   CPU 1
=======================================|=================================
shrink_huge_zero_folio_scan()          |
   atomic_cmpxchg() sets refcount to 0 |
   xchg() sets huge_zero_folio to NULL | get_huge_zero_folio()
                 |                     |    atomic_inc_not_zero() -> zero
      preempted for a long time        |    Allocate new huge zero folio
                 |                     |    Write valid huge_zero_folio
                 v                     |    Write valid huge_zero_pfn
  Overwrite huge_zero_pfn with ~0UL   <--- Invalid overwrite!

This results in is_huge_zero_pfn() and is_huge_zero_pmd() incorrectly
returning false for a huge zero page which could result in issues like the
huge zero folio being incorrectly split.

Note that the issue is with huge_zero_pfn not huge_zero_folio, as
get_huge_zero_folio() uses cmpxchg() gated on huge_zero_folio being NULL
with a retry loop and shrink_huge_zero_folio_scan() uses xchg() to set
huge_zero_folio.

Fix the issue by introducing a spinlock, huge_zero_lock, to prevent
concurrent write of huge_zero_folio, huge_zero_pfn and huge_zero_refcount.

There needs to be significant care taken here to ensure correctness:

The fast path in get_huge_zero_folio() uses atomic_inc_not_zero(), which is
outside of the critical section when !CONFIG_PERSISTENT_HUGE_ZERO_FOLIO,
and means huge zero allocation is gated on zero huge_zero_refcount.

The fast path doesn't use huge_zero_lock, so the critical section is
irrelevant to it.

So invariants are required - huge_zero_refcount MUST:

* Only be set in the huge_zero_lock critical section to ensure
  serialisation of huge_zero_pfn, huge_zero_folio and huge_zero_refcount
  writes.

* Be set non-zero only AFTER huge_zero_[pfn, folio] are set to valid values
  so installation of the huge zero folio on read page fault ensures
  concurrent is_huge_zero_*() calls correctly identify the huge zero folio.

* Be set zero only BEFORE huge_zero_[pfn, folio] are set to NULL and ~0UL
  respectively, and atomically.

Establish these by:

* Only updating huge_zero_refcount in the huge_zero_lock critical section
  in get_huge_zero_folio() and shrink_huge_zero_folio_scan().

* Using atomic_set_release(&huge_zero_refcount) in get_huge_zero_folio()
  after huge_zero_[pfn, folio] are set. This is paired with
  atomic_inc_not_zero() to ensure atomic_inc_not_zero() only observes a
  non-zero value if huge_zero_[pfn, folio] are set.

* Using atomic_cmpxchg() in shrink_huge_zero_folio_scan() to ensure that it
  is set zero only when equal to 1 and set atomically.

* atomic_cmpxchg() being fully ordered ensures this is done prior to
  huge_zero_[folio, pfn] being set to NULL and ~0UL respectively.

Note that only the huge zero shrinker (via shrink_huge_zero_folio_scan())
can actually set huge_zero_refcount to zero, which is the count of mm's
which have at least one huge zero folio installed plus one shrinker pin.

Additionally convert a BUG_ON() to a VM_WARN_ON_ONCE().

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Reported-by: Hengbin Zhang <uqbarz@gmail.com>
Closes: https://lore.kernel.org/linux-mm/20260727154001.4102341-1-uqbarz@gmail.com/
Fixes: 3b77e8c8cde5 ("mm/thp: make is_huge_zero_pmd() safe and quicker")
Cc: stable@vger.kernel.org # 6.18.x: dependent on prior commit
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/huge_memory.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 0f60bc82e87a..acee2d28b9bb 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -41,6 +41,7 @@
 #include <linux/pgalloc.h>
 #include <linux/pgalloc_tag.h>
 #include <linux/pagewalk.h>
+#include <linux/cleanup.h>
 
 #include <asm/tlb.h>
 #include "internal.h"
@@ -82,6 +83,7 @@ struct folio *huge_zero_folio __read_mostly;
 unsigned long huge_zero_pfn __read_mostly = HUGE_ZERO_UNSET_PFN;
 #ifndef CONFIG_PERSISTENT_HUGE_ZERO_FOLIO
 static atomic_t huge_zero_refcount;
+static DEFINE_SPINLOCK(huge_zero_lock);
 static struct shrinker *huge_zero_folio_shrinker;
 #endif
 
@@ -270,7 +272,8 @@ void mm_put_huge_zero_folio(struct mm_struct *mm)
 static bool get_huge_zero_folio(void)
 {
 	struct folio *zero_folio;
-retry:
+
+	/* Paired with atomic_set_release(). */
 	if (likely(atomic_inc_not_zero(&huge_zero_refcount)))
 		return true;
 
@@ -278,17 +281,21 @@ static bool get_huge_zero_folio(void)
 	if (unlikely(!zero_folio))
 		return false;
 
-	preempt_disable();
-	if (cmpxchg(&huge_zero_folio, NULL, zero_folio)) {
-		preempt_enable();
+	/* Paired with critical section in shrink_huge_zero_folio_scan(). */
+	spin_lock(&huge_zero_lock);
+	if (huge_zero_folio) {
+		/* Somebody else already installed it. */
+		atomic_inc(&huge_zero_refcount);
+		spin_unlock(&huge_zero_lock);
 		folio_put(zero_folio);
-		goto retry;
+		return true;
 	}
+	WRITE_ONCE(huge_zero_folio, zero_folio);
 	WRITE_ONCE(huge_zero_pfn, folio_pfn(zero_folio));
+	/* Paired with atomic_inc_not_zero(). +1 for shrinker pin. */
+	atomic_set_release(&huge_zero_refcount, 2);
+	spin_unlock(&huge_zero_lock);
 
-	/* We take additional reference here. It will be put back by shrinker */
-	atomic_set(&huge_zero_refcount, 2);
-	preempt_enable();
 	count_vm_event(THP_ZERO_PAGE_ALLOC);
 	return true;
 }
@@ -312,15 +319,22 @@ static unsigned long shrink_huge_zero_folio_count(struct shrinker *shrink,
 static unsigned long shrink_huge_zero_folio_scan(struct shrinker *shrink,
 						 struct shrink_control *sc)
 {
-	if (atomic_cmpxchg(&huge_zero_refcount, 1, 0) == 1) {
-		struct folio *zero_folio = xchg(&huge_zero_folio, NULL);
-		BUG_ON(zero_folio == NULL);
+	struct folio *zero_folio;
+
+	/* Paired with critical section in get_huge_zero_folio(). */
+	scoped_guard(spinlock, &huge_zero_lock) {
+		/* Paired with atomic_inc_not_zero() in get_huge_zero_folio(). */
+		if (atomic_cmpxchg(&huge_zero_refcount, 1, 0) != 1)
+			return 0;
+
+		zero_folio = huge_zero_folio;
+		VM_WARN_ON_ONCE(!huge_zero_folio);
+		WRITE_ONCE(huge_zero_folio, NULL);
 		WRITE_ONCE(huge_zero_pfn, HUGE_ZERO_UNSET_PFN);
-		folio_put(zero_folio);
-		return HPAGE_PMD_NR;
 	}
 
-	return 0;
+	folio_put(zero_folio);
+	return HPAGE_PMD_NR;
 }
 
 static int __init huge_zero_init(void)

-- 
2.55.0

Re: [PATCH mm-hotfixes 0/2] mm/huge_memory: fix huge_zero_pfn race

[David Hildenbrand (Arm)]

On 7/28/26 14:05, Lorenzo Stoakes (ARM) wrote:
> There is a subtle race in the reference-counted huge_zero_folio
> implementation.
> 
> The fast path atomic logic fails to account for the fact that the
> shrinker (which drops the final huge_zero_refcount pin) can overwrite
> huge_zero_pfn with the ~0UL sentinel value in shrink_huge_zero_folio_scan()
> after a racing get_huge_zero_folio() installed a valid value there.
> 
> This results in huge_zero_folio being correctly set but huge_zero_pfn being
> set incorrectly and thus is_huge_zero_pfn() and consequently
> is_huge_zero_pmd() will misidentify the huge zero folio as being an
> ordinary THP folio.
> 
> This can result in the huge zero folio being split and otherwise treated
> incorrectly.
> 
> The solution to this is very subtle as there is an atomic fast path, and
> thus ordering in weakly ordered architectures has to be treated very
> carefully.
> 
> As a result, this series first reworks the
> CONFIG_PERSISTENT_HUGE_ZERO_FOLIO logic so it is separated from the
> refcounted code in order to make the subsequent fix reasonably
> understandable.

For at least somewhat easier backports, can we reverse the order?

The spinlock+proper ordering should be possible without #1, or am I missing
something important?

-- 
Cheers,

David