[PATCH 0/6] mm: fix pfn walker races with memory offline

[PATCH 0/6] mm: fix pfn walker races with memory offline

[Gregory Price (Meta)]

Several pfn walkers check that a struct page is online, then use it
after memory offline has poisoned it (CONFIG_DEBUG_VM) or removal has
freed the vmemmap.  Five of the races crash the kernel; the sixth leaves
an offline spinning with the device hotplug lock held.

Patch 1 adds pfn_walk, an SRCU-fast read section that offline waits for
before poisoning the memmap.  Patches 2 and 5 use it; patches 3 and 6
hold get_online_mems() instead.

  1. compaction scanners
  2. page_owner reads
  3. memory_failure(), soft_offline_page(), unpoison_memory()
  4. alloc_contig_range() undoing another caller's isolation
  5. alloc_contig_pages() isolating a range that went offline
  6. the hwpoison-inject filter

All six are for stable.
  Patches 2 and 5 need patch 1
  patch 5 builds on patch 4
  patch 6 builds on patch 3

Testing
=======

Each race was reproduced on unmodified source in a QEMU guest with three
NUMA nodes and a pc-dimm left offline at boot (DEBUG_VM, KASAN,
lockdep).  A test module holds one task inside the window while the
offline or eject runs, then releases it.

         base                           this series
  patch  outcome                 runs   outcome                 runs
  -----  ----------------------  -----  ----------------------  -----
  1      GPF                     3/3    offline waits, clean    3/3
  2      GPF or #PF              6/6    offline waits, clean    6/6
  3      GPF, or unpoison of a   9/9    blocks, -EIO/-ENXIO     9/9
         poisoned page
  4      offline still looping   4/4    offline completes       3/3
         after 30s
  5      GPF                     3/3    offline waits, clean    6/6
  6      #PF                     3/3    eject waits, -ENXIO     3/3

Without forcing the window, offline/online cycling under compaction
faulted in 8 of 24 runs on base and 0 of 12 with patch 1.

Cost
====
I was unable to measure a discernable cost beyond noise, stddev of
walkers and scanners runtimes on real hardware was wildly inconsistent.
Leads me to believe this change is largely imperceivable.

Gregory Price (Meta) (6):
  mm/compaction: wait for pfn walkers before poisoning offlined memmap
  mm/page_owner: hold off memory offline while a page is read
  mm/memory-failure: keep the page online while handling it
  mm/page_alloc: don't undo another isolation in alloc_contig_range
  mm/page_alloc: keep the range online while alloc_contig isolates it
  mm/hwpoison-inject: keep the page online while filtering it

 mm/compaction.c      |  8 +++++++
 mm/hwpoison-inject.c | 55 +++++++++++++++++++++++++++++---------------
 mm/internal.h        | 41 +++++++++++++++++++++++++++++++++
 mm/memory-failure.c  | 31 +++++++++++++++++--------
 mm/memory_hotplug.c  | 16 +++++++++++--
 mm/mm_init.c         |  4 ++--
 mm/page_alloc.c      | 35 +++++++++++++++++++++++++---
 mm/page_alloc.h      |  7 +++++-
 mm/page_owner.c      | 45 +++++++++++++++++++-----------------
 9 files changed, 186 insertions(+), 56 deletions(-)

-- 
2.53.0-Meta

[PATCH 1/6] mm/compaction: wait for pfn walkers before poisoning offlined memmap

[Gregory Price (Meta)]

Compaction checks that a pageblock is online, then uses its pages across
lock drops and sleeps.  If offline poisons the memmap meanwhile, a
DEBUG_VM kernel faults:

  CPU 1 (compaction)                  CPU 2 (offline_pages)
  pageblock_pfn_to_page()
    block is online
  isolate_migratepages_block()
    cond_resched()
                                      offline_mem_sections()
                                      remove_pfn_range_from_zone()
                                        page_init_poison()
    set_pageblock_skip(valid_page)
      reads the zone of a poisoned page

  Oops: general protection fault, probably for non-canonical address 0xdffffc00000006a2
  RIP: 0010:set_pfnblock_bit+0xa6/0x2b0
   isolate_migratepages_block+0x2456/0x3c80
   compact_zone+0x1638/0x3900
   compact_node+0x12a/0x1f0
   sysctl_compaction_handler+0xec/0x160

The free scanner faults the same way.  A contiguous zone skips the
online check, since offline clears zone->contiguous only after
poisoning.

Hold an SRCU-fast read section (pfn_walk) from the block check to the
last use of its pages, and make remove_pfn_range_from_zone() clear
zone->contiguous and wait for it before poisoning.  Offline pays one
grace period per range.

Scanner positions outlive the section, and offline can shrink the zone
meanwhile, so take the contiguous fast path only inside the current
span, publish zone->contiguous with release, and keep the migrate
scanner inside the zone.

With a kprobe parking the migrate scanner across the offline, base
faults in 3 of 3 runs and this patch in none.  Unforced offline/online
cycling under compaction faults in 8 of 24 runs on base and 0 of 12 with
this patch.

Fixes: d33695b16a9f ("mm/memory_hotplug: poison memmap in remove_pfn_range_from_zone()")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/compaction.c     |  8 ++++++++
 mm/internal.h       | 41 +++++++++++++++++++++++++++++++++++++++++
 mm/memory_hotplug.c | 14 ++++++++++++--
 mm/mm_init.c        |  4 ++--
 mm/page_alloc.h     |  7 ++++++-
 5 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index f1b2060eb2016..2aeb18b6d54dd 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -360,6 +360,7 @@ static void __reset_isolation_suitable(struct zone *zone)
 	if (!zone->compact_blockskip_flush)
 		return;
 
+	guard(pfn_walk)();
 	zone->compact_blockskip_flush = false;
 
 	/*
@@ -716,6 +717,7 @@ isolate_freepages_range(struct compact_control *cc,
 	unsigned long isolated, pfn, block_start_pfn, block_end_pfn;
 	int order;
 
+	guard(pfn_walk)();
 	for (order = 0; order < NR_PAGE_ORDERS; order++)
 		INIT_LIST_HEAD(&cc->freepages[order]);
 
@@ -1340,6 +1342,7 @@ isolate_migratepages_range(struct compact_control *cc, unsigned long start_pfn,
 	unsigned long pfn, block_start_pfn, block_end_pfn;
 	int ret = 0;
 
+	guard(pfn_walk)();
 	/* Scan block by block. First and last block may be incomplete */
 	pfn = start_pfn;
 	block_start_pfn = pageblock_start_pfn(pfn);
@@ -1732,6 +1735,7 @@ static void isolate_freepages(struct compact_control *cc)
 	unsigned long low_pfn;	     /* lowest pfn scanner is able to scan */
 	unsigned int stride;
 
+	guard(pfn_walk)();
 	/* Try a small search of the free lists for a candidate */
 	fast_isolate_freepages(cc);
 	if (cc->nr_freepages)
@@ -2099,6 +2103,7 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
 		(cc->mode != MIGRATE_SYNC ? ISOLATE_ASYNC_MIGRATE : 0);
 	bool fast_find_block;
 
+	guard(pfn_walk)();
 	/*
 	 * Start at where we last stopped, or beginning of the zone as
 	 * initialized by compact_zone(). The first failure will use
@@ -2116,6 +2121,9 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
 	 */
 	fast_find_block = low_pfn != cc->migrate_pfn && !cc->fast_search_fail;
 
+	/* Offline may have shrunk the zone start past an old migrate_pfn */
+	low_pfn = max(low_pfn, block_start_pfn);
+
 	/* Only scan within a pageblock boundary */
 	block_end_pfn = pageblock_end_pfn(low_pfn);
 
diff --git a/mm/internal.h b/mm/internal.h
index d497c35ee7833..8a2faa74403b5 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -18,6 +18,7 @@
 #include <linux/rmap.h>
 #include <linux/swap.h>
 #include <linux/leafops.h>
+#include <linux/srcu.h>
 #include <linux/tracepoint-defs.h>
 
 /* Internal core VMA manipulation functions. */
@@ -1206,6 +1207,46 @@ struct tlbflush_unmap_batch;
  */
 extern struct workqueue_struct *mm_percpu_wq;
 
+/*
+ * Compaction checks that a pageblock is online and then uses its pages,
+ * possibly across sleeps.  remove_pfn_range_from_zone() waits for these
+ * readers before it poisons the memmap of a range taken offline.
+ */
+#ifdef CONFIG_MEMORY_HOTREMOVE
+extern struct srcu_struct pfn_walk_srcu;
+
+static inline struct srcu_ctr __percpu *pfn_walk_lock(void)
+{
+	return srcu_read_lock_fast(&pfn_walk_srcu);
+}
+
+static inline void pfn_walk_unlock(struct srcu_ctr __percpu *scp)
+{
+	srcu_read_unlock_fast(&pfn_walk_srcu, scp);
+}
+
+static inline void pfn_walk_synchronize(void)
+{
+	synchronize_srcu(&pfn_walk_srcu);
+}
+#else
+static inline struct srcu_ctr __percpu *pfn_walk_lock(void)
+{
+	return NULL;
+}
+
+static inline void pfn_walk_unlock(struct srcu_ctr __percpu *scp)
+{
+}
+
+static inline void pfn_walk_synchronize(void)
+{
+}
+#endif
+
+DEFINE_LOCK_GUARD_0(pfn_walk, _T->scp = pfn_walk_lock(),
+		    pfn_walk_unlock(_T->scp), struct srcu_ctr __percpu *scp)
+
 #ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
 void try_to_unmap_flush(void);
 void try_to_unmap_flush_dirty(void);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 796af1028ee23..b5d9e367b893d 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -543,6 +543,16 @@ void remove_pfn_range_from_zone(struct zone *zone,
 	struct pglist_data *pgdat = zone->zone_pgdat;
 	unsigned long pfn, cur_nr_pages;
 
+	/*
+	 * The range is offline, but compaction may still use pages it found
+	 * online.  Make new scans check the online state instead of trusting
+	 * zone->contiguous, and wait for the old ones before poisoning.
+	 */
+	if (!zone_is_zone_device(zone)) {
+		clear_zone_contiguous(zone);
+		pfn_walk_synchronize();
+	}
+
 	/* Poison struct pages because they are now uninitialized again. */
 	for (pfn = start_pfn; pfn < end_pfn; pfn += cur_nr_pages) {
 		cond_resched();
@@ -563,8 +573,6 @@ void remove_pfn_range_from_zone(struct zone *zone,
 	if (zone_is_zone_device(zone))
 		return;
 
-	clear_zone_contiguous(zone);
-
 	shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
 	update_pgdat_span(pgdat);
 
@@ -1783,6 +1791,8 @@ bool mhp_range_allowed(u64 start, u64 size, bool need_mapping)
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
 
+DEFINE_SRCU_FAST(pfn_walk_srcu);
+
 static int check_no_memblock_for_node_cb(struct memory_block *mem, void *arg)
 {
 	int nid = *(int *)arg;
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 01ced0553ce9b..db610ddc37398 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2270,8 +2270,8 @@ void set_zone_contiguous(struct zone *zone)
 		cond_resched();
 	}
 
-	/* We confirm that there is no hole */
-	zone->contiguous = true;
+	/* We confirm that there is no hole; pairs with pageblock_pfn_to_page() */
+	smp_store_release(&zone->contiguous, true);
 }
 
 /*
diff --git a/mm/page_alloc.h b/mm/page_alloc.h
index 53df6b8a0c00a..7e185326b80ab 100644
--- a/mm/page_alloc.h
+++ b/mm/page_alloc.h
@@ -216,7 +216,12 @@ extern struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
 static inline struct page *pageblock_pfn_to_page(unsigned long start_pfn,
 				unsigned long end_pfn, struct zone *zone)
 {
-	if (zone->contiguous)
+	/*
+	 * Offline can shrink the zone under a caller's old pfns and then set
+	 * zone->contiguous again, so only trust it inside the current span.
+	 */
+	if (smp_load_acquire(&zone->contiguous) &&
+	    zone_spans_pfn(zone, start_pfn) && zone_spans_pfn(zone, end_pfn - 1))
 		return pfn_to_page(start_pfn);
 
 	return __pageblock_pfn_to_page(start_pfn, end_pfn, zone);
-- 
2.53.0-Meta

Re: [PATCH 1/6] mm/compaction: wait for pfn walkers before poisoning offlined memmap

[David Hildenbrand (Arm)]

On 10/27/26 05:21, Gregory Price (Meta) wrote:
> Compaction checks that a pageblock is online, then uses its pages across
> lock drops and sleeps.  If offline poisons the memmap meanwhile, a
> DEBUG_VM kernel faults:
> 
>   CPU 1 (compaction)                  CPU 2 (offline_pages)
>   pageblock_pfn_to_page()
>     block is online
>   isolate_migratepages_block()
>     cond_resched()
>                                       offline_mem_sections()
>                                       remove_pfn_range_from_zone()
>                                         page_init_poison()
>     set_pageblock_skip(valid_page)
>       reads the zone of a poisoned page
> 
>   Oops: general protection fault, probably for non-canonical address 0xdffffc00000006a2
>   RIP: 0010:set_pfnblock_bit+0xa6/0x2b0
>    isolate_migratepages_block+0x2456/0x3c80
>    compact_zone+0x1638/0x3900
>    compact_node+0x12a/0x1f0
>    sysctl_compaction_handler+0xec/0x160
> 
> The free scanner faults the same way.  A contiguous zone skips the
> online check, since offline clears zone->contiguous only after
> poisoning.
> 
> Hold an SRCU-fast read section (pfn_walk) from the block check to the
> last use of its pages, and make remove_pfn_range_from_zone() clear
> zone->contiguous and wait for it before poisoning.  Offline pays one
> grace period per range.
> 
> Scanner positions outlive the section, and offline can shrink the zone
> meanwhile, so take the contiguous fast path only inside the current
> span, publish zone->contiguous with release, and keep the migrate
> scanner inside the zone.
> 
> With a kprobe parking the migrate scanner across the offline, base
> faults in 3 of 3 runs and this patch in none.  Unforced offline/online
> cycling under compaction faults in 8 of 24 runs on base and 0 of 12 with
> this patch.
> 
> Fixes: d33695b16a9f ("mm/memory_hotplug: poison memmap in remove_pfn_range_from_zone()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
> ---

BTW, the order of these patches is all wrong.

There are much more important fixes hiding in this series. And this change here
would certainly not be the first thing we'd backport.

Please create a proper series that only addresses the pfn_to_online_page()
situation, without different, more urgent fixes.

Because there is no way we will fast track anything like this here into stable
kernels.

-- 
Cheers,

David

[PATCH 2/6] mm/page_owner: hold off memory offline while a page is read

[Gregory Price (Meta)]

Reading /sys/kernel/debug/page_owner while memory is removed can oops.
read_page_owner() checks a pfn with pfn_valid(), and print_page_owner()
can sleep in kmalloc() before it reads the struct page:

  CPU 1 (read page_owner)             CPU 2 (remove the memory block)
  pfn_valid(pfn)
  page = pfn_to_page(pfn)
  page_ext_get()
  page_ext_put()
  print_page_owner()
    kmalloc()
                                      offline_pages()
                                        remove_pfn_range_from_zone()
                                      arch_remove_memory()
                                        frees the vmemmap
    get_pageblock_migratetype(page)

  BUG: unable to handle page fault for address: ffd4000006000000
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  Oops: Oops: 0000 [#1] SMP KASAN
  RIP: 0010:__get_pfnblock_flags_mask+0x3e/0x2b0
  Call Trace:
   <TASK>
   get_pfnblock_migratetype+0xf/0x20
   read_page_owner+0x629/0xf00
   full_proxy_read+0x102/0x190
   vfs_read+0x153/0xcd0
   ksys_read+0xe4/0x1b0
   do_syscall_64+0xbd/0x630
   entry_SYSCALL_64_after_hwframe+0x4b/0x53

Offline alone faults the same way on DEBUG_VM kernels.  The page_ext is
safe; offline_page_ext() frees it after an RCU grace period.

Check the pfn with pfn_to_online_page() inside a pfn_walk read section,
which offline waits for, and move kmalloc() and copy_to_user() out of
it, since offline waits with the hotplug locks held.  A full read of
page_owner in a 5G guest takes 1.36 s before and 1.34 s after.

With a kprobe sleeping the reader on a DIMM pfn across an offline or a
removal, base faults in 3 of 3 runs of each and this patch in none.

Fixes: 48c96a368579 ("mm/page_owner: keep track of page owners")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_owner.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index cfc31c92d7657..d2c0b831fcd3b 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -13,6 +13,7 @@
 #include <linux/memcontrol.h>
 #include <linux/sched/clock.h>
 
+#include "internal.h"
 #include "page_alloc.h"
 
 /*
@@ -600,20 +601,14 @@ static inline int print_page_owner_memcg(char *kbuf, size_t count, int ret,
 #endif
 
 static ssize_t
-print_page_owner(char __user *buf, size_t count, unsigned long pfn,
+print_page_owner(char *kbuf, size_t count, unsigned long pfn,
 		struct page *page, struct page_owner *page_owner,
 		depot_stack_handle_t handle,
 		struct page_owner_filter_state *state)
 {
 	int ret, pageblock_mt, page_mt;
-	char *kbuf;
 	enum page_owner_print_mode print_mode;
 
-	count = min_t(size_t, count, PAGE_SIZE);
-	kbuf = kmalloc(count, GFP_KERNEL);
-	if (!kbuf)
-		return -ENOMEM;
-
 	print_mode = state->print_mode;
 
 	ret = scnprintf(kbuf, count,
@@ -637,14 +632,14 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 	if (print_mode != PAGE_OWNER_PRINT_HANDLE) {
 		ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0);
 		if (ret >= count)
-			goto err;
+			return -ENOMEM;
 	}
 
 	if (print_mode != PAGE_OWNER_PRINT_STACK) {
 		ret += scnprintf(kbuf + ret, count - ret, "handle: %u\n",
 				 handle);
 		if (ret >= count)
-			goto err;
+			return -ENOMEM;
 	}
 
 	if (page_owner->last_migrate_reason != MR_NEVER) {
@@ -657,17 +652,9 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 
 	ret += snprintf(kbuf + ret, count - ret, "\n");
 	if (ret >= count)
-		goto err;
-
-	if (copy_to_user(buf, kbuf, ret))
-		ret = -EFAULT;
+		return -ENOMEM;
 
-	kfree(kbuf);
 	return ret;
-
-err:
-	kfree(kbuf);
-	return -ENOMEM;
 }
 
 void __dump_page_owner(const struct page *page)
@@ -734,10 +721,17 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	struct page_owner *page_owner;
 	depot_stack_handle_t handle;
 	struct page_owner_filter_state *state = file->private_data;
+	ssize_t ret = 0;
+	char *kbuf;
 
 	if (!static_branch_unlikely(&page_owner_inited))
 		return -EINVAL;
 
+	count = min_t(size_t, count, PAGE_SIZE);
+	kbuf = kmalloc(count, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
 	page = NULL;
 	if (*ppos == 0)
 		pfn = min_low_pfn;
@@ -766,7 +760,12 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 			continue;
 		}
 
-		page = pfn_to_page(pfn);
+		/* Offline waits for us before it poisons the memmap */
+		guard(pfn_walk)();
+		page = pfn_to_online_page(pfn);
+		if (!page)
+			continue;
+
 		if (skip_buddy_pages(&pfn, page))
 			continue;
 
@@ -825,14 +824,18 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 
 		page_owner_tmp = *page_owner;
 		page_ext_put(page_ext);
-		return print_page_owner(buf, count, pfn, page,
+		ret = print_page_owner(kbuf, count, pfn, page,
 				&page_owner_tmp, handle, state);
+		break;
 ext_put_continue:
 		page_ext_put(page_ext);
 		cond_resched();
 	}
 
-	return 0;
+	if (ret > 0 && copy_to_user(buf, kbuf, ret))
+		ret = -EFAULT;
+	kfree(kbuf);
+	return ret;
 }
 
 static loff_t lseek_page_owner(struct file *file, loff_t offset, int orig)
-- 
2.53.0-Meta

[PATCH 3/6] mm/memory-failure: keep the page online while handling it

[Gregory Price (Meta)]

memory_failure(), soft_offline_page() and unpoison_memory() check that a
pfn is online, then sleep on mf_mutex before using its page.  If the
block goes offline meanwhile, a DEBUG_VM kernel faults on the poisoned
memmap, and removal frees it on any kernel:

  CPU 1 (soft_offline_page)           CPU 2 (offline_pages)
  pfn_to_online_page()
    page is online
  mutex_lock(&mf_mutex)
                                      offline_mem_sections()
                                      remove_pfn_range_from_zone()
                                        page_init_poison()
  get_hwpoison_page()
    zone_pcp_disable(page_zone(page))
      zone of a poisoned page

  Oops: general protection fault, probably for non-canonical address 0xdffffc000000069e: 0000 [#1] SMP KASAN
  RIP: 0010:__zone_set_pageset_high_and_batch+0x89/0x270
   zone_pcp_disable+0x28/0x40
   get_hwpoison_page+0xc2/0x1160
   soft_offline_page.part.0+0xe7/0xbf0

memory_failure() faults the same way.  unpoison_memory() does not fault,
but it clears flags on the poisoned page and returns success.

Hold get_online_mems() from before the online check to the last use of
the page.  soft_offline_page() took it only around get_hwpoison_page();
it now takes it once, earlier.

With a kprobe parking each entry point across the offline, base faults,
or for unpoison_memory() succeeds, in 3 of 3 runs each.  With this patch
all 9 runs wait and fail with -EIO or -ENXIO, and lockdep stays clean
over offline/online cycles mixed with error injection.

Fixes: 96c804a6ae8c ("mm/memory-failure.c: don't access uninitialized memmaps in memory_failure()")
Fixes: feec24a6139d ("mm, soft-offline: convert parameter to pfn")
Fixes: d613f53c83ec ("mm/memory-failure: fix VM_BUG_ON_PAGE(PagePoisoned(page)) when unpoison memory")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/memory-failure.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 0c96eb5119976..cdae2a1988bb8 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2394,6 +2394,8 @@ int memory_failure(unsigned long pfn, int flags)
 	if (!sysctl_memory_failure_recovery)
 		panic("Memory failure on page %lx", pfn);
 
+	/* Keep the page online from the check below until we are done */
+	get_online_mems();
 	mutex_lock(&mf_mutex);
 
 	if (!(flags & MF_SW_SIMULATED))
@@ -2608,11 +2610,13 @@ int memory_failure(unsigned long pfn, int flags)
 identify_page_state:
 	res = identify_page_state(pfn, p, page_flags);
 	mutex_unlock(&mf_mutex);
+	put_online_mems();
 	return res;
 unlock_page:
 	folio_unlock(folio);
 unlock_mutex:
 	mutex_unlock(&mf_mutex);
+	put_online_mems();
 	return res;
 }
 EXPORT_SYMBOL_GPL(memory_failure);
@@ -2742,9 +2746,12 @@ int unpoison_memory(unsigned long pfn)
 	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
 					DEFAULT_RATELIMIT_BURST);
 
+	get_online_mems();
 	p = pfn_to_online_page(pfn);
-	if (!p)
+	if (!p) {
+		put_online_mems();
 		return -EIO;
+	}
 	folio = page_folio(p);
 
 	mutex_lock(&mf_mutex);
@@ -2833,6 +2840,7 @@ int unpoison_memory(unsigned long pfn)
 		unpoison_pr_info("%#lx: software-unpoisoned page\n",
 				 page_to_pfn(p), &unpoison_rs);
 	}
+	put_online_mems();
 	return ret;
 }
 EXPORT_SYMBOL(unpoison_memory);
@@ -2974,38 +2982,42 @@ int soft_offline_page(unsigned long pfn, int flags)
 		return -ENXIO;
 	}
 
+	if (!sysctl_enable_soft_offline) {
+		pr_info_once("disabled by /proc/sys/vm/enable_soft_offline\n");
+		put_ref_page(pfn, flags);
+		return -EOPNOTSUPP;
+	}
+
+	/* Keep the page online from the check below until we are done */
+	get_online_mems();
+
 	/* Only online pages can be soft-offlined (esp., not ZONE_DEVICE). */
 	page = pfn_to_online_page(pfn);
 	if (!page) {
+		put_online_mems();
 		put_ref_page(pfn, flags);
 		return -EIO;
 	}
 
-	if (!sysctl_enable_soft_offline) {
-		pr_info_once("disabled by /proc/sys/vm/enable_soft_offline\n");
-		put_ref_page(pfn, flags);
-		return -EOPNOTSUPP;
-	}
-
 	mutex_lock(&mf_mutex);
 
 	if (PageHWPoison(page)) {
 		pr_info("%#lx: page already poisoned\n", pfn);
 		put_ref_page(pfn, flags);
 		mutex_unlock(&mf_mutex);
+		put_online_mems();
 		return 0;
 	}
 
 retry:
-	get_online_mems();
 	ret = get_hwpoison_page(page, flags | MF_SOFT_OFFLINE);
-	put_online_mems();
 
 	if (hwpoison_filter(page)) {
 		if (ret > 0)
 			put_page(page);
 
 		mutex_unlock(&mf_mutex);
+		put_online_mems();
 		return -EOPNOTSUPP;
 	}
 
@@ -3023,6 +3035,7 @@ int soft_offline_page(unsigned long pfn, int flags)
 	}
 
 	mutex_unlock(&mf_mutex);
+	put_online_mems();
 
 	return ret;
 }
-- 
2.53.0-Meta

[PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[Gregory Price (Meta)]

An alloc_contig_range() that overlaps a block going offline can leave
the offline spinning until it gets a signal, holding the device hotplug
lock.  When isolation fails, alloc_contig_frozen_range() undoes it over
the whole range, clearing the offline's MIGRATE_ISOLATE too:

  CPU 1 (offline_pages)               CPU 2 (alloc_contig_range)
  start_isolate_page_range()
    blocks are MIGRATE_ISOLATE
                                      start_isolate_page_range()
                                        -EBUSY on CPU 1's blocks
                                      undo_isolate_page_range()
                                        blocks are MIGRATE_MOVABLE
  test_pages_isolated()
    never succeeds again

start_isolate_page_range() fails only on a block someone else isolated,
after undoing its own, so this undo only ever clears another caller's.
Return directly, as before alloc_contig_range() went pageblock granular.

With offline_pages() parked after isolating a DIMM block and a 1G
hugetlb allocation made over it, the offline on base still loops 30s
later in 4 of 4 runs:

  offline_pages+0x5b2/0x1570
  memory_subsys_offline+0x17e/0x470
  device_offline+0x250/0x340
  state_store+0x88/0x110

With this patch the allocation still fails, and the offline completes in
3 of 3 runs.

Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 55d3fc4b307f5..bb3c7e95f2ed4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7396,9 +7396,13 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
 	 * put back to page allocator so that buddy can use them.
 	 */
 
+	/*
+	 * On failure this has undone its own isolation, and any other
+	 * isolated blocks in the range belong to someone else, e.g. offline.
+	 */
 	ret = start_isolate_page_range(start, end, mode);
 	if (ret)
-		goto done;
+		return ret;
 
 	drain_all_pages(cc.zone);
 
-- 
2.53.0-Meta

Re: [PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[David Hildenbrand (Arm)]

On 10/27/26 05:22, Gregory Price (Meta) wrote:
> An alloc_contig_range() that overlaps a block going offline can leave
> the offline spinning until it gets a signal, holding the device hotplug
> lock.  When isolation fails, alloc_contig_frozen_range() undoes it over
> the whole range, clearing the offline's MIGRATE_ISOLATE too:
> 
>   CPU 1 (offline_pages)               CPU 2 (alloc_contig_range)
>   start_isolate_page_range()
>     blocks are MIGRATE_ISOLATE
>                                       start_isolate_page_range()
>                                         -EBUSY on CPU 1's blocks
>                                       undo_isolate_page_range()
>                                         blocks are MIGRATE_MOVABLE
>   test_pages_isolated()
>     never succeeds again
> 
> start_isolate_page_range() fails only on a block someone else isolated,
> after undoing its own, so this undo only ever clears another caller's.
> Return directly, as before alloc_contig_range() went pageblock granular.
> 
> With offline_pages() parked after isolating a DIMM block and a 1G
> hugetlb allocation made over it, the offline on base still loops 30s
> later in 4 of 4 runs:
> 
>   offline_pages+0x5b2/0x1570
>   memory_subsys_offline+0x17e/0x470
>   device_offline+0x250/0x340
>   state_store+0x88/0x110
> 
> With this patch the allocation still fails, and the offline completes in
> 3 of 3 runs.
> 
> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
> ---

We have an existing AI fix for that as well already handled by Vlasta.

I think we'll have to establish some model of "ask David first before posting"
because otherwise we just create duplicate work for everyone.

-- 
Cheers,

David

Re: [PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[Gregory Price (Meta)]

On 2026-10-27 14:37 +0100, David Hildenbrand (Arm) wrote:
> On 10/27/26 05:22, Gregory Price (Meta) wrote:
> 
> We have an existing AI fix for that as well already handled by Vlasta.
> 
> I think we'll have to establish some model of "ask David first before posting"
> because otherwise we just create duplicate work for everyone.
> 

Yeah, this is going to happen more.

I've been sitting in these patches for over a month while they soaked on
hardware getting performance tests.  

Maybe worth a private tree we can rebase on regularly for these kinds of
un-vetted/posted fixes to see what's coming?

-- 
~Gregory

Re: [PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[David Hildenbrand (Arm)]

On 10/27/26 15:06, Gregory Price (Meta) wrote:
> On 2026-10-27 14:37 +0100, David Hildenbrand (Arm) wrote:
>> On 10/27/26 05:22, Gregory Price (Meta) wrote:
>>
>> We have an existing AI fix for that as well already handled by Vlasta.
>>
>> I think we'll have to establish some model of "ask David first before posting"
>> because otherwise we just create duplicate work for everyone.
>>
> 
> Yeah, this is going to happen more.
> 
> I've been sitting in these patches for over a month while they soaked on
> hardware getting performance tests.  

I mean this fix here is really just independent and shouldn't wait that long :)

> 
> Maybe worth a private tree we can rebase on regularly for these kinds of
> un-vetted/posted fixes to see what's coming?

Maybe. But if it's just us core people we can just talk on IRC etc (like Zi did
two days ago when he was about to post something that I already had on my queue).

-- 
Cheers,

David

Re: [PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[Zi Yan]

On 27 Oct 2026, at 10:13, David Hildenbrand (Arm) wrote:

> On 10/27/26 15:06, Gregory Price (Meta) wrote:
>> On 2026-10-27 14:37 +0100, David Hildenbrand (Arm) wrote:
>>> On 10/27/26 05:22, Gregory Price (Meta) wrote:
>>>
>>> We have an existing AI fix for that as well already handled by Vlasta.
>>>
>>> I think we'll have to establish some model of "ask David first before posting"
>>> because otherwise we just create duplicate work for everyone.
>>>
>>
>> Yeah, this is going to happen more.
>>
>> I've been sitting in these patches for over a month while they soaked on
>> hardware getting performance tests.
>
> I mean this fix here is really just independent and shouldn't wait that long :)
>
>>
>> Maybe worth a private tree we can rebase on regularly for these kinds of
>> un-vetted/posted fixes to see what's coming?
>
> Maybe. But if it's just us core people we can just talk on IRC etc (like Zi did
> two days ago when he was about to post something that I already had on my queue).

One idea is that maybe Sashiko is able to identify patches fixing
the same issue and does some grouping or tagging on the website.
Or send an email about it with a lore link.

Best Regards,
Yan, Zi

Re: [PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[David Hildenbrand (Arm)]

On 10/27/26 19:53, Zi Yan wrote:
> On 27 Oct 2026, at 10:13, David Hildenbrand (Arm) wrote:
> 
>> On 10/27/26 15:06, Gregory Price (Meta) wrote:
>>>
>>> Yeah, this is going to happen more.
>>>
>>> I've been sitting in these patches for over a month while they soaked on
>>> hardware getting performance tests.
>>
>> I mean this fix here is really just independent and shouldn't wait that long :)
>>
>>>
>>> Maybe worth a private tree we can rebase on regularly for these kinds of
>>> un-vetted/posted fixes to see what's coming?
>>
>> Maybe. But if it's just us core people we can just talk on IRC etc (like Zi did
>> two days ago when he was about to post something that I already had on my queue).
> 
> One idea is that maybe Sashiko is able to identify patches fixing
> the same issue and does some grouping or tagging on the website.
> Or send an email about it with a lore link.

I think that's too late. For us core devs we just need to sync on "has someone
already a fix for that or is it worth cleaning the AI shite up and sending it out".

-- 
Cheers,

David

Re: [PATCH 4/6] mm/page_alloc: don't undo another isolation in alloc_contig_range

[Gregory Price]

On 2026-10-27 19:58 +0100, David Hildenbrand (Arm) wrote:
> On 10/27/26 19:53, Zi Yan wrote:
> > On 27 Oct 2026, at 10:13, David Hildenbrand (Arm) wrote:
> > 
> >> Maybe. But if it's just us core people we can just talk on IRC etc (like Zi did
> >> two days ago when he was about to post something that I already had on my queue).
> > 
> > One idea is that maybe Sashiko is able to identify patches fixing
> > the same issue and does some grouping or tagging on the website.
> > Or send an email about it with a lore link.
> 
> I think that's too late. For us core devs we just need to sync on "has someone
> already a fix for that or is it worth cleaning the AI shite up and sending it out".
> 

It might be worth just having a maintainer Fixes:-only review branch
that automates pulling these things in and responding to conflicts.

I've been working on some b4 workbench automation to make triaging the
list a little more sustainable. It's getting there, but it needs time.

~Gregory

-- 
Gregory Price <gourry@gourry.net>

[PATCH 5/6] mm/page_alloc: keep the range online while alloc_contig isolates it

[Gregory Price (Meta)]

A gigantic hugetlb allocation racing with memory offline can crash.
alloc_contig_pages() checks that a range is online under zone->lock,
drops the lock, and alloc_contig_range() then reads the zone of its
pages.  If the range goes offline in between, a DEBUG_VM kernel faults
on the poisoned memmap, and removal frees it on any kernel:

  CPU 1 (alloc_contig_pages)          CPU 2 (offline_pages)
  pfn_range_valid_contig()
    range is online
  spin_unlock(&zone->lock)
                                      offline_mem_sections()
                                      remove_pfn_range_from_zone()
                                        page_init_poison()
  alloc_contig_range()
    start_isolate_page_range()
      isolate_single_pageblock()
        zone of a poisoned page

  Oops: general protection fault, probably for non-canonical address 0xdffffc00000006a2: 0000 [#1] SMP KASAN
  RIP: 0010:isolate_single_pageblock+0xe5/0x820
   start_isolate_page_range+0x74/0x1a0
   alloc_contig_frozen_range_noprof+0x21c/0xcc0
   alloc_contig_frozen_pages_noprof+0x4cf/0x700

A range that goes offline during isolation is no better, since
start_isolate_page_range() skips offline blocks.

Check that the range is online, isolate it and check again under
pfn_walk, and look up the zone only then; an isolated range cannot go
offline.  This adds one online lookup per pageblock on each side of
isolation.

With a kprobe holding a 1G allocation at start_isolate_page_range()
across the offline, base faults in 3 of 3 runs.  With this patch the
offline waits for the allocation in 6 of 6, and both finish.

Fixes: 5e27a2df03b8 ("mm/page_alloc: add alloc_contig_pages()")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bb3c7e95f2ed4..82f35816a0711 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7319,6 +7319,16 @@ static void __free_contig_frozen_range(unsigned long pfn, unsigned long nr_pages
 	__free_contig_range_common(pfn, nr_pages, /* is_frozen= */ true);
 }
 
+static bool pfn_range_online(unsigned long start, unsigned long end)
+{
+	unsigned long pfn;
+
+	for (pfn = start; pfn < end; pfn += pageblock_nr_pages)
+		if (!pfn_to_online_page(pfn))
+			return false;
+	return true;
+}
+
 /**
  * alloc_contig_frozen_range() -- tries to allocate given range of frozen pages
  * @start:	start PFN to allocate
@@ -7352,7 +7362,6 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
 	struct compact_control cc = {
 		.nr_migratepages = 0,
 		.order = -1,
-		.zone = page_zone(pfn_to_page(start)),
 		.mode = MIGRATE_SYNC,
 		.ignore_skip_hint = true,
 		.no_set_skip_hint = true,
@@ -7397,13 +7406,29 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
 	 */
 
 	/*
-	 * On failure this has undone its own isolation, and any other
-	 * isolated blocks in the range belong to someone else, e.g. offline.
+	 * Part of the range may go offline under us.  Offline cannot poison
+	 * the memmap while we hold pfn_walk, but isolation silently skips
+	 * blocks that are already offline, so check before and after.  On
+	 * failure, start_isolate_page_range() has undone its own isolation,
+	 * and any other isolated blocks belong to someone else, e.g. offline.
 	 */
-	ret = start_isolate_page_range(start, end, mode);
+	scoped_guard(pfn_walk) {
+		unsigned long first = pageblock_start_pfn(start);
+		unsigned long last = pageblock_align(end);
+
+		if (!pfn_range_online(first, last))
+			return -EBUSY;
+		ret = start_isolate_page_range(start, end, mode);
+		if (!ret && !pfn_range_online(first, last)) {
+			/* Every online isolated block in the range is ours */
+			undo_isolate_page_range(start, end);
+			ret = -EBUSY;
+		}
+	}
 	if (ret)
 		return ret;
 
+	cc.zone = page_zone(pfn_to_page(start));
 	drain_all_pages(cc.zone);
 
 	/*
-- 
2.53.0-Meta

[PATCH 6/6] mm/hwpoison-inject: keep the page online while filtering it

[Gregory Price (Meta)]

Injecting a memory error with the filter enabled while the memory is
hot-removed can crash.  hwpoison_inject() checks the pfn with
pfn_valid(), then runs shake_folio() and the filter on its page, which
can sleep in lru_add_drain_all():

  CPU 1 (hwpoison_inject)             CPU 2 (ACPI eject)
  pfn_valid()
  folio = page_folio(pfn_to_page())
                                      offline_pages()
                                      arch_remove_memory()
                                        vmemmap freed
  shake_folio(folio)
    reads folio->flags

  BUG: unable to handle page fault for address: ffd4000006040033
  #PF: error_code(0x0000) - not-present page
  RIP: 0010:shake_folio+0x2a/0x50
   hwpoison_inject+0x232/0x4b0 [hwpoison_inject]
   simple_attr_write_xsigned.constprop.0.isra.0+0x179/0x220
   debugfs_attr_write+0x63/0x90

Until the filter stopped pinning the page, that reference held off
offline.

Hold get_online_mems() around the filter, check the pfn with
pfn_to_online_page() under it, and drop the hold before
memory_failure(), which takes it again.  Offline and ZONE_DEVICE pages
never passed the filter anyway.  Export get/put_online_mems() to the
module.

With the injector held at shake_folio() on a free DIMM page across an
eject, base faults in 3 of 3 runs.  With this patch the eject waits, and
the injection then fails with -ENXIO, in 3 of 3.

Fixes: fd476720c9ba ("mm,hwpoison-inject: don't pin for hwpoison_filter")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/hwpoison-inject.c | 55 +++++++++++++++++++++++++++++---------------
 mm/memory_hotplug.c  |  2 ++
 2 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/mm/hwpoison-inject.c b/mm/hwpoison-inject.c
index a11222572f978..f6cc967b3a468 100644
--- a/mm/hwpoison-inject.c
+++ b/mm/hwpoison-inject.c
@@ -9,6 +9,7 @@
 #include <linux/hugetlb.h>
 #include <linux/page-flags.h>
 #include <linux/memcontrol.h>
+#include <linux/memory_hotplug.h>
 #include "internal.h"
 
 static u32 hwpoison_filter_enable;
@@ -99,43 +100,61 @@ static int hwpoison_filter(struct page *p)
 
 static struct dentry *hwpoison_dir;
 
-static int hwpoison_inject(void *data, u64 val)
+/* Returns true if the filter rejects the page at @pfn */
+static bool hwpoison_inject_filtered(unsigned long pfn)
 {
-	unsigned long pfn = val;
 	struct page *p;
 	struct folio *folio;
-	int err;
-
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
-	if (!pfn_valid(pfn))
-		return -ENXIO;
 
-	p = pfn_to_page(pfn);
+	/*
+	 * Offline and ZONE_DEVICE pages are not LRU, hugetlb or free buddy
+	 * pages, so the checks below never let them through.
+	 */
+	p = pfn_to_online_page(pfn);
+	if (!p)
+		return true;
 	folio = page_folio(p);
 
-	if (!hwpoison_filter_enable)
-		goto inject;
-
 	shake_folio(folio);
 	/*
 	 * This implies unable to support non-LRU pages except free page.
 	 */
 	if (!folio_test_lru(folio) && !folio_test_hugetlb(folio) &&
 	    !is_free_buddy_page(p))
-		return 0;
+		return true;
 
 	/*
 	 * do a racy check to make sure PG_hwpoison will only be set for
 	 * the targeted owner (or on a free page).
 	 * memory_failure() will redo the check reliably inside page lock.
 	 */
-	err = hwpoison_filter(&folio->page);
-	if (err)
-		return 0;
+	return hwpoison_filter(&folio->page);
+}
+
+static int hwpoison_inject(void *data, u64 val)
+{
+	unsigned long pfn = val;
+	bool filtered;
+	int err;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (!pfn_valid(pfn))
+		return -ENXIO;
+
+	if (hwpoison_filter_enable) {
+		/*
+		 * Keep the page online while the filter looks at it.
+		 * memory_failure() takes this again, so drop it first.
+		 */
+		get_online_mems();
+		filtered = hwpoison_inject_filtered(pfn);
+		put_online_mems();
+		if (filtered)
+			return 0;
+	}
 
-inject:
 	pr_info("Injecting memory failure at pfn %#lx\n", pfn);
 	err = memory_failure(pfn, MF_SW_SIMULATED);
 	return (err == -EOPNOTSUPP) ? 0 : err;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b5d9e367b893d..fe2ed0894d49d 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -215,11 +215,13 @@ void get_online_mems(void)
 {
 	percpu_down_read(&mem_hotplug_lock);
 }
+EXPORT_SYMBOL_FOR_MODULES(get_online_mems, "hwpoison-inject");
 
 void put_online_mems(void)
 {
 	percpu_up_read(&mem_hotplug_lock);
 }
+EXPORT_SYMBOL_FOR_MODULES(put_online_mems, "hwpoison-inject");
 
 bool movable_node_enabled = false;
 
-- 
2.53.0-Meta

Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[David Hildenbrand (Arm)]

On 10/27/26 05:21, Gregory Price (Meta) wrote:
> Several pfn walkers check that a struct page is online, then use it
> after memory offline has poisoned it (CONFIG_DEBUG_VM) or removal has
> freed the vmemmap.  Five of the races crash the kernel; the sixth leaves
> an offline spinning with the device hotplug lock held.

I have roughly the exact same list of AI garbage fixes here from a certain company.

And I consider this all extremely low priority.

So this is something to discuss throughout the next months and do it properly.

-- 
Cheers,

David

Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[Gregory Price (Meta)]

On 2026-10-27 09:57 +0100, David Hildenbrand (Arm) wrote:
> On 10/27/26 05:21, Gregory Price (Meta) wrote:
> > Several pfn walkers check that a struct page is online, then use it
> > after memory offline has poisoned it (CONFIG_DEBUG_VM) or removal has
> > freed the vmemmap.  Five of the races crash the kernel; the sixth leaves
> > an offline spinning with the device hotplug lock held.
> 
> I have roughly the exact same list of AI garbage fixes here from a certain company.
> 
> And I consider this all extremely low priority.
> 
> So this is something to discuss throughout the next months and do it properly.
> 

I actually have a reason for these fixes other than "bot said bad".

We leverage ballooning and hotplugging as VM memory overcommit tools
and are finding that some of these race conditions crash them with
large MTBF.

While most of these are insanely narrow, they are not as unlikely to
happen as you think. A vCPU can take a vmexit in these windows much
more easily than a host can take an NMI or other interrupt.

Thankfully none of these (or the other series) appear to be obvious
security issues, but they are stability issues.

I have a reproducer for every single one of these (and the other
series) - but a handful are unreachable due to a 2-statement read
being emitted as a single read by the compiler (which can be undone
by a targeted compiler flag). Those were included for completeness.

--

On properly - I have a follow up series that fixes the zonelist walks
structurally, but it uses SRCU-fast on the fast path. The catch is that
PowerPC (and i think others?) don't SRCU-fast without atomics, so they
take a performance hit.  (It also doesn't help us for backports)

I've been weighing that solution over never freeing and/or prepopulating
zones - but this also results in performance impacts (leaving dead zones
in the list forces the allocator to walk them for no value).

-- 
~Gregory

Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[David Hildenbrand (Arm)]

On 10/27/26 12:31, Gregory Price (Meta) wrote:
> On 2026-10-27 09:57 +0100, David Hildenbrand (Arm) wrote:
>> On 10/27/26 05:21, Gregory Price (Meta) wrote:
>>> Several pfn walkers check that a struct page is online, then use it
>>> after memory offline has poisoned it (CONFIG_DEBUG_VM) or removal has
>>> freed the vmemmap.  Five of the races crash the kernel; the sixth leaves
>>> an offline spinning with the device hotplug lock held.
>>
>> I have roughly the exact same list of AI garbage fixes here from a certain company.
>>
>> And I consider this all extremely low priority.
>>
>> So this is something to discuss throughout the next months and do it properly.
>>
> 
> I actually have a reason for these fixes other than "bot said bad".

So you found them on production kernels?

We did not have a single real report of that for probably a decade or
even longer in production.

Recently, we had the single first report about the compaction case from a
stress test (no production):

https://lore.kernel.org/all/20260903095543.3752338-1-yuan1.liu@intel.com/

"We are not sure whether this race is reachable under realistic
workloads or only under this synthetic stress test."

Where I motivated that we need something clean.

> 
> We leverage ballooning and hotplugging as VM memory overcommit tools
> and are finding that some of these race conditions crash them with
> large MTBF.

yeah, that makes sense.

> 
> While most of these are insanely narrow, they are not as unlikely to
> happen as you think. A vCPU can take a vmexit in these windows much
> more easily than a host can take an NMI or other interrupt.

Jup, that's what I would expect.

> 
> Thankfully none of these (or the other series) appear to be obvious
> security issues, but they are stability issues.

Right. And DEBUG_VM reports are irrelevant in production.

The thing is that the race between offlining and actual memory removal
is extremely long. Poisoning is just a mechanism to find these theoretical
issues that could cause issues faster.

And with all the other things going on, this has lower priority, and I
wanted to get back to the AI patches I have once the other thing is sorted.

> 
> I have a reproducer for every single one of these (and the other
> series) - but a handful are unreachable due to a 2-statement read
> being emitted as a single read by the compiler (which can be undone
> by a targeted compiler flag). Those were included for completeness.


In general, I think we want something that will protect all
pfn_to_online_page() + zone->contig thingies, not just selected ones.
We discussed in the past using some sort of rcu for that.

get_online_mems is usually the wrong approach.

The infrastructure for that should be added in a clean separate patch, not
mangled into a patch that fixes a DEBUG_VM splat.

This is nothing to rush in or to schlop around.

The AI patch for compaction I have here does something similar to your first
patch, with a bit less churn, and converts a handful more users.

E.g.,


+/*
+ * Nothing keeps a section online once pfn_to_online_page() has returned. A pfn
+ * walker that holds no reference to the page does the check and looks at the
+ * memmap under this guard: remove_pfn_range_from_zone() waits for it before the
+ * memmap is poisoned and can be freed. That wait cannot be interrupted, and
+ * whatever a walker sleeps on under the guard, memory offlining waits for as
+ * well: leave the guard before copying to user space. mem_hotplug_lock is held
+ * during the wait: never take it under the guard.
+ */
+extern struct srcu_struct memmap_srcu;
+#define memmap_walk_guard()    guard(srcu)(&memmap_srcu)


and then


diff --git a/fs/proc/page.c b/fs/proc/page.c
index 260772b20bd99..c5f6d7fe12d9a 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -58,13 +58,39 @@ static u64 get_kpage_count(const struct page *page)
        return ret;
 }

+static u64 kpage_info(unsigned long pfn, enum kpage_operation op)
+{
+       struct page *page;
+
+       /* The page is not ours: keep its memmap until we are done with it. */
+       memmap_walk_guard();
+
+       /*
+        * TODO: ZONE_DEVICE support requires to identify
+        * memmaps that were actually initialized.
+        */
+       page = pfn_to_online_page(pfn);
+       if (!page)
+               return 0;
+
+       switch (op) {
+       case KPAGE_FLAGS:
+               return stable_page_flags(page);
+       case KPAGE_COUNT:
+               return get_kpage_count(page);
+       case KPAGE_CGROUP:
+               return page_cgroup_ino(page);
+       default:
+               return 0;
+       }
+}
+
 static ssize_t kpage_read(struct file *file, char __user *buf,
                size_t count, loff_t *ppos,
                enum kpage_operation op)
 {
        const unsigned long max_dump_pfn = get_max_dump_pfn();
        u64 __user *out = (u64 __user *)buf;
-       struct page *page;
        unsigned long src = *ppos;
        unsigned long pfn;
        ssize_t ret = 0;
@@ -78,29 +104,7 @@ static ssize_t kpage_read(struct file *file, char __user *buf,
        count = min_t(unsigned long, count, (max_dump_pfn * KPMSIZE) - src);

        while (count > 0) {
-               /*
-                * TODO: ZONE_DEVICE support requires to identify
-                * memmaps that were actually initialized.
-                */
-               page = pfn_to_online_page(pfn);
-
-               if (page) {
-                       switch (op) {
-                       case KPAGE_FLAGS:
-                               info = stable_page_flags(page);
-                               break;
-                       case KPAGE_COUNT:
-                               info = get_kpage_count(page);
-                               break;
-                       case KPAGE_CGROUP:
-                               info = page_cgroup_ino(page);
-                               break;
-                       default:
-                               info = 0;
-                               break;
-                       }
-               } else
-                       info = 0;
+               info = kpage_info(pfn, op);

                if (put_user(info, out)) {


protecting individual pfn_to_online_page is just whack a mole. Likely we want a
way to check whether pfn_to_online_page() is called from the wrong context, and
maybe we need a variant that can be called from "wrong context" for a good reason.

-- 
Cheers,

David

Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[Gregory Price (Meta)]

On 2026-10-27 13:51 +0100, David Hildenbrand (Arm) wrote:
> On 10/27/26 12:31, Gregory Price (Meta) wrote:
> > On 2026-10-27 09:57 +0100, David Hildenbrand (Arm) wrote:
> > 
> > I actually have a reason for these fixes other than "bot said bad".
> 
> So you found them on production kernels?
> 
> We did not have a single real report of that for probably a decade or
> even longer in production.
> 
> Recently, we had the single first report about the compaction case from a
> stress test (no production):
> 
> https://lore.kernel.org/all/20260903095543.3752338-1-yuan1.liu@intel.com/
> 
> "We are not sure whether this race is reachable under realistic
> workloads or only under this synthetic stress test."
> 
> Where I motivated that we need something clean.
> 

tl;dr:

while these are ai-generated, I'm not posting untested, unreviewed
schlopp for the sake of getting patches upstream.

I actually care about this issue.

---

Some of these don't show up as crashes but instead as hung offline
attempts (GFP_KERNEL landing on movable).  That shows up as our
hypervisor just failing to overcommit / guests hanging.

The compaction/alloc_contig bug is one I've seen in production.

I've seen a handful of NULL derefs in production i suspect are the
zonelist derefs, but it's hard to say (they're rare enough that we
lose the dumps before someone has a thought to pull them).

I expect this will scale up as we increase the number of VMs and
the amount of hotplug we're doing.

For clarity, we use ballooning + hotplug as a combo:
 - ballooning to do cooperative overcommit
 - hotplug to defend host stability when the overcommit ratio is extreme
   (basically deflate_on_oom because a host stability issue)

We're pretty aggressive about ballooning guests to squeeze them and
dropping the balloon to hot-unplug that memory when a guest is cold.


skippable side note on virtio-balloon/mem:

I have some improvements to virtio-balloon and virtio-mem that allow for
handoff from balloon directly to mem for hot-unplug without having to
drop the balloon, but I haven't posted those (and we don't run those in
production, so this is not the nexus of crashes).

Similarly I have another extension that lets the host set a "NO_ALLOC"
bit on the zone so a balloon-only system can provide the host a way to
defend against mass deflate_on_oom events (we prefer guests to OOM and
have the host to kill them due to PSI, rather than threaten host
stability). Also not in production and not posted, but there have been
a lot of experiments on overcommit here.

> The thing is that the race between offlining and actual memory removal
> is extremely long. Poisoning is just a mechanism to find these theoretical
> issues that could cause issues faster.
> 
> And with all the other things going on, this has lower priority, and I
> wanted to get back to the AI patches I have once the other thing is sorted.
> 

Sure, I realize I'm pushing this out during the merge window, but I
felt it reasonable to get some thoughts on whether these fixes are
worth considering for the window.

If the answer is no just put em on the back burner. We can carry
fixes as-needed.

> > 
> > I have a reproducer for every single one of these (and the other
> > series) - but a handful are unreachable due to a 2-statement read
> > being emitted as a single read by the compiler (which can be undone
> > by a targeted compiler flag). Those were included for completeness.
> 
> 
> In general, I think we want something that will protect all
> pfn_to_online_page() + zone->contig thingies, not just selected ones.
> We discussed in the past using some sort of rcu for that.
>
> get_online_mems is usually the wrong approach.
> 

Agreed, although the PFN walks seem genuinely harder to wrangle into an
good rcu shape.

get_online_mems seems like the backportable solution, but i can see not
wanting to have to go clean it back up later.

> protecting individual pfn_to_online_page is just whack a mole. Likely we want a
> way to check whether pfn_to_online_page() is called from the wrong context, and
> maybe we need a variant that can be called from "wrong context" for a good reason.
> 

Agreed.  Although I fear the structural fix for this will have uglier
performance issue than we think.  Then again, maybe it doesn't matter
(if you're doing a PFN walk, then you're likely already doing something
stupidly expensive).

Where i'm largely concerned is the page allocator and the
compaction/contig code not taking a regression.

For zonelists i've ended up with something like this:

@@ -1461,7 +1466,7 @@ typedef struct pglist_data {
         * Generally the first zones will be references to this node's
         * node_zones.
         */
-       struct zonelist node_zonelists[MAX_ZONELISTS];
+       struct zonelist __rcu *node_zonelists[MAX_ZONELISTS];

...

static inline struct srcu_ctr __percpu *zonelist_read_lock(void)
{
       return srcu_read_lock_fast(&zonelist_srcu);
}

static inline void zonelist_read_unlock(struct srcu_ctr __percpu *scp)
{
       srcu_read_unlock_fast(&zonelist_srcu, scp);
}

DEFINE_LOCK_GUARD_0(zonelist_read, _T->scp = zonelist_read_lock(),
                   zonelist_read_unlock(_T->scp),
                   struct srcu_ctr __percpu *scp)

static inline struct zonelist *pgdat_zonelist(pg_data_t *pgdat, int type)
{
       return zonelist_dereference(pgdat->node_zonelists[type]);
}

With this the zonelist cookie checks evaporate, but you still end up
with guard() at every walk location - not great, but better i suppose.

I was hoping this could be sunken into the zonelist walk macros, but
I haven't seen that pan out.  I've been spending a lot of time auditing
that code for private nodes as well.


On performance:

I've been hammering this with will-it-scale and a bunch of workload
tests on a variety of hosts and have not seen any obvious regressions
outside of powerpc (which i can't test directly, but SRCU-fast requires
atomics for powerpc from what i can tell - so it will eat overhead
without a fallback... if there is actually any hotplug ppc hardware out
there to care about?)

-- 
~Gregory

Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[David Hildenbrand (Arm)]

On 10/27/26 14:49, Gregory Price (Meta) wrote:
> On 2026-10-27 13:51 +0100, David Hildenbrand (Arm) wrote:
>> On 10/27/26 12:31, Gregory Price (Meta) wrote:
>>>
>>> I actually have a reason for these fixes other than "bot said bad".
>>
>> So you found them on production kernels?
>>
>> We did not have a single real report of that for probably a decade or
>> even longer in production.
>>
>> Recently, we had the single first report about the compaction case from a
>> stress test (no production):
>>
>> https://lore.kernel.org/all/20260903095543.3752338-1-yuan1.liu@intel.com/
>>
>> "We are not sure whether this race is reachable under realistic
>> workloads or only under this synthetic stress test."
>>
>> Where I motivated that we need something clean.
>>
> 
> tl;dr:
> 
> while these are ai-generated, I'm not posting untested, unreviewed
> schlopp for the sake of getting patches upstream.

Appreciated.

For this long-standing considered low-priority thing in particular it is
reasonable to go back in time, see how often it was raised (quite often by me)
and how often it was actually reported (I found one case).

Then discuss the high level design in more than a single sentence :)

Discuss why we are not using it for other pfn_to_online_page() users, how it
interacts with zone->contig etc etc.

There really isn't a lot on the design in the cover letter.

> 
> I actually care about this issue.
> 
> ---
> 
> Some of these don't show up as crashes but instead as hung offline
> attempts (GFP_KERNEL landing on movable).  That shows up as our
> hypervisor just failing to overcommit / guests hanging.
> 
> The compaction/alloc_contig bug is one I've seen in production.

Right, but I suspect that's "[PATCH 4/6] mm/page_alloc: don't undo another
isolation in alloc_contig_range" which is *not at all* related to "mm: fix pfn
walker races with memory offline".

> 
> I've seen a handful of NULL derefs in production i suspect are the
> zonelist derefs, but it's hard to say (they're rare enough that we
> lose the dumps before someone has a thought to pull them).

That would be interesting to know.

> 
> I expect this will scale up as we increase the number of VMs and
> the amount of hotplug we're doing.
> 
> For clarity, we use ballooning + hotplug as a combo:
>  - ballooning to do cooperative overcommit
>  - hotplug to defend host stability when the overcommit ratio is extreme
>    (basically deflate_on_oom because a host stability issue)
> 
> We're pretty aggressive about ballooning guests to squeeze them and
> dropping the balloon to hot-unplug that memory when a guest is cold.

Yeah, that makes sense.

> 
> 
> skippable side note on virtio-balloon/mem:
> 
> I have some improvements to virtio-balloon and virtio-mem that allow for
> handoff from balloon directly to mem for hot-unplug without having to
> drop the balloon, but I haven't posted those (and we don't run those in
> production, so this is not the nexus of crashes).

Do you mean free-page reporting or actual balloon inflation/deflation paired
with virtio-mem. I hope not the latter, because that is just disgusting.

> 
> Similarly I have another extension that lets the host set a "NO_ALLOC"
> bit on the zone so a balloon-only system can provide the host a way to
> defend against mass deflate_on_oom events (we prefer guests to OOM and
> have the host to kill them due to PSI, rather than threaten host
> stability). Also not in production and not posted, but there have been
> a lot of experiments on overcommit here.
> 
>> The thing is that the race between offlining and actual memory removal
>> is extremely long. Poisoning is just a mechanism to find these theoretical
>> issues that could cause issues faster.
>>
>> And with all the other things going on, this has lower priority, and I
>> wanted to get back to the AI patches I have once the other thing is sorted.
>>
> 
> Sure, I realize I'm pushing this out during the merge window, but I
> felt it reasonable to get some thoughts on whether these fixes are
> worth considering for the window.

Just a matter of priority. I think this should be tackled in clean way indeed.

> 
> If the answer is no just put em on the back burner. We can carry
> fixes as-needed.
> 
>>>
>>> I have a reproducer for every single one of these (and the other
>>> series) - but a handful are unreachable due to a 2-statement read
>>> being emitted as a single read by the compiler (which can be undone
>>> by a targeted compiler flag). Those were included for completeness.
>>
>>
>> In general, I think we want something that will protect all
>> pfn_to_online_page() + zone->contig thingies, not just selected ones.
>> We discussed in the past using some sort of rcu for that.
>>
>> get_online_mems is usually the wrong approach.
>>
> 
> Agreed, although the PFN walks seem genuinely harder to wrangle into an
> good rcu shape.

And that's where the overall design needs to discuss this and how to resolve it.

:)

> 
> get_online_mems seems like the backportable solution, but i can see not
> wanting to have to go clean it back up later.
> 
>> protecting individual pfn_to_online_page is just whack a mole. Likely we want a
>> way to check whether pfn_to_online_page() is called from the wrong context, and
>> maybe we need a variant that can be called from "wrong context" for a good reason.
>>
> 
> Agreed.  Although I fear the structural fix for this will have uglier
> performance issue than we think.  Then again, maybe it doesn't matter
> (if you're doing a PFN walk, then you're likely already doing something
> stupidly expensive).

Which areas are you mostly concerned about and from which piece do you expect
the overhead?

> 
> Where i'm largely concerned is the page allocator and the
> compaction/contig code not taking a regression.
> 
> For zonelists i've ended up with something like this:
> 
> @@ -1461,7 +1466,7 @@ typedef struct pglist_data {
>          * Generally the first zones will be references to this node's
>          * node_zones.
>          */
> -       struct zonelist node_zonelists[MAX_ZONELISTS];
> +       struct zonelist __rcu *node_zonelists[MAX_ZONELISTS];
> 
> ...
> 
> static inline struct srcu_ctr __percpu *zonelist_read_lock(void)
> {
>        return srcu_read_lock_fast(&zonelist_srcu);
> }
> 
> static inline void zonelist_read_unlock(struct srcu_ctr __percpu *scp)
> {
>        srcu_read_unlock_fast(&zonelist_srcu, scp);
> }
> 
> DEFINE_LOCK_GUARD_0(zonelist_read, _T->scp = zonelist_read_lock(),
>                    zonelist_read_unlock(_T->scp),
>                    struct srcu_ctr __percpu *scp)
> 
> static inline struct zonelist *pgdat_zonelist(pg_data_t *pgdat, int type)
> {
>        return zonelist_dereference(pgdat->node_zonelists[type]);
> }
> 
> With this the zonelist cookie checks evaporate, but you still end up
> with guard() at every walk location - not great, but better i suppose.


For "broader" things that are not just a simple pfn_to_online_page() user, we
could also have memory online/offline sync in more efficient ways using the
memory notifier.

It's also tricky, but maybe for zone->contig + compaction the better solution?

> 
> I was hoping this could be sunken into the zonelist walk macros, but
> I haven't seen that pan out.  I've been spending a lot of time auditing
> that code for private nodes as well.
> 
> 
> On performance:
> 
> I've been hammering this with will-it-scale and a bunch of workload
> tests on a variety of hosts and have not seen any obvious regressions
> outside of powerpc (which i can't test directly, but SRCU-fast requires
> atomics for powerpc from what i can tell - so it will eat overhead
> without a fallback... if there is actually any hotplug ppc hardware out
> there to care about?)

So the big problem is the overhead on the read side for srcu?

If we can find something else to suspend compaction, we could maybe switch to
rcu for the remaining cases.

Now that I think about it, I recall why I didn't fix this earlier :)

-- 
Cheers,

David

Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[Gregory Price (Meta)]

On 2026-10-27 15:25 +0100, David Hildenbrand (Arm) wrote:
> On 10/27/26 14:49, Gregory Price (Meta) wrote:
> > 
> > while these are ai-generated, I'm not posting untested, unreviewed
> > schlopp for the sake of getting patches upstream.
> 
> Appreciated.
> 
> For this long-standing considered low-priority thing in particular it is
> reasonable to go back in time, see how often it was raised (quite often by me)
> and how often it was actually reported (I found one case).
> 
> Then discuss the high level design in more than a single sentence :)
> 
> Discuss why we are not using it for other pfn_to_online_page() users, how it
> interacts with zone->contig etc etc.
> 
> There really isn't a lot on the design in the cover letter.
>

Fair, there's a mix of design vs firefighting tension going on here.

I'm trying to get out ahead of a rapidly evolving scaling issue before
it becomes big.  But I will admit it's sometimes hard to even figure
out what questions to ask without posting the wrong patches first.

(Cunningham's law :P)

> > Some of these don't show up as crashes but instead as hung offline
> > attempts (GFP_KERNEL landing on movable).  That shows up as our
> > hypervisor just failing to overcommit / guests hanging.
> > 
> > The compaction/alloc_contig bug is one I've seen in production.
> 
> Right, but I suspect that's "[PATCH 4/6] mm/page_alloc: don't undo another
> isolation in alloc_contig_range" which is *not at all* related to "mm: fix pfn
> walker races with memory offline".
>

Ehhh... i'd argue it's the same shape.  But i'll concede some of these
could be pulled out ahead as you've said elsewhere.

> > skippable side note on virtio-balloon/mem:
> > 
> > I have some improvements to virtio-balloon and virtio-mem that allow for
> > handoff from balloon directly to mem for hot-unplug without having to
> > drop the balloon, but I haven't posted those (and we don't run those in
> > production, so this is not the nexus of crashes).
> 
> Do you mean free-page reporting or actual balloon inflation/deflation paired
> with virtio-mem. I hope not the latter, because that is just disgusting.
> 

Actual balloon inflation/deflation.  It is disgusting, but it's
effective.  Right now we use hotplug at boot to minimize the size of
cold machines to avoid thundering herd issues, and then add it when
they actually get allocated for work.

At runtime we use ballooning to reduce a VM to its actual RSS based
on PSI feedback (see: that virtio stats extension i sent you some while
ago), but would like to use hotplug to give the host a defense against
a storm of deflate_on_oom scenarios across a large number of guests.

Basically using PSI stats we can make guests do somewhat aggressive
proactive reclaim on without having to build any new tooling, and
while being able to back-off aggressively.

Free-page reporting is also on by default, but this only helps you if
your guests are being extra nice about things.

(spoiler: they're not! they're greedly little imps! :D)

My experiments made balloon prefer to try to reclaim movable regions
in contiguous blocks, and then just hand that contiguous block off to
virtio-mem for offline without having to drop the balloon.

tl;dr: This seems effective and works quite well.

I've become a little confused as to why virtio balloon and mem are
separate entities when they could work together this way.

Haven't had the time to read up on the development history.

> > 
> > Agreed.  Although I fear the structural fix for this will have uglier
> > performance issue than we think.  Then again, maybe it doesn't matter
> > (if you're doing a PFN walk, then you're likely already doing something
> > stupidly expensive).
> 
> Which areas are you mostly concerned about and from which piece do you expect
> the overhead?
> 

largely the zonelist iteration - which looking back through history 
has already been "optimized" multiple times, only for those things
to introduce more bugs :\.

The zonelist races seem more ammenable to RCU solutions than the pfn
walks.  I'm not sure how to safely quiesce every possible PFN walker
without atomics in their fast path.

> For "broader" things that are not just a simple pfn_to_online_page() user, we
> could also have memory online/offline sync in more efficient ways using the
> memory notifier.
> 
> It's also tricky, but maybe for zone->contig + compaction the better solution?
> 

This seems reasonable, I was trying not to schlopp you with a half-baked
architectural fix and instead use the available tools.

But maybe we should chat (after the window).

I'll try to be on IRC more often.

> > On performance:
> > 
> > I've been hammering this with will-it-scale and a bunch of workload
> > tests on a variety of hosts and have not seen any obvious regressions
> > outside of powerpc (which i can't test directly, but SRCU-fast requires
> > atomics for powerpc from what i can tell - so it will eat overhead
> > without a fallback... if there is actually any hotplug ppc hardware out
> > there to care about?)
> 
> So the big problem is the overhead on the read side for srcu?
>

Yes - but specifically this affects PowerPC because it:
 - has NMI
 - does NOT have NMI-safe per-CPU operations
 - supports memory hotplug
 - uses atomics for SRCU-fast

So using SRCU fast in these contexts is not cheap.

For what I could measure in QEMU, the atomics on PPC seem like they
could be significant (15-20% overhead) in microbenchmarks.

Hard to say what that turns into on real hardware.

I implemented a fallback solution for those architectures to just not
free the zonelists in this case.  They have to iterate the dead zones,
but it's a much less overhead comparatively (<1%, but measurable).

For everyone else SRCU-fast is imperceptible overhead.

> If we can find something else to suspend compaction, we could maybe switch to
> rcu for the remaining cases.
> 

Is long-term compaction suspension even safe? I had to make COMPACTION a
requirement for N_MEMORY_USER_NUMA in order to not generate spurious
failures due to fragmentation.

~Gregory

-- 
~Gregory